Re: Skip a redundant singleton GROUP BY node - Mailing list pgsql-hackers

Hi ChenHui Mo,

Thanks for the detailed tests and the clear plan output. Both observations are valid, and I have reorganized the series
asv2 to address them while keeping the proof narrow. 

This version is a two-patch series based on PostgreSQL commit fb60892f40:

Patch 1/2: Move unique-index GROUP BY matching into indxpath

This is a behavior-preserving refactor. It moves the shared unique-index/GROUP BY matcher into indxpath.c, keeps
GroupByColInfoprivate to that file, and exposes only a higher-level function that returns the GROUP BY columns made
redundantby a suitable unique index. It also clarifies that notnullattnums contains heap attribute numbers.
Centralizingthe NOT NULL, NULLS NOT DISTINCT, opfamily, and collation checks avoids maintaining a second matcher in the
singletoncode. 

Patch 2/2: Skip a redundant singleton GROUP BY node

This adds the planner optimization, documentation, and regression coverage. It adds direct complete and partial paths
asalternatives to ordinary grouping paths instead of replacing ordinary grouping path generation. 

The proof is still intentionally narrow.

The input must be a single ordinary base relation or partitioned table.

The query must be a plain GROUP BY with no aggregates, HAVING, window functions, set operations, DISTINCT, SRFs, or row
locking.

There must be an immediate, non-partial, non-expression unique index whose key columns are covered by simple grouping
Vars.

A NULLS DISTINCT unique index also requires every key column to be NOT NULL; NULLS NOT DISTINCT removes that
requirement.

The index and grouping keys must agree on equality semantics, including opfamily and collation.

Additional grouping expressions are safe because they can only subdivide singleton groups.

Projection below Gather

Yes. v1 populated only grouped_rel->pathlist, so a parallel-safe SELECT-list expression could remain above Gather.

v2/v3 now also adds direct partial paths to grouped_rel->partial_pathlist when the projection is parallel-safe.
create_ordered_paths()and normal Gather generation can then place a Result below Gather, allowing workers to evaluate
thetarget expression. Parallel-restricted expressions are still handled on the leader. 

For your expensive parallel-safe projection case, the direct partial path can produce the following shape:

Gather
-> Result
-> Parallel Append
-> Parallel Seq Scan

I agree that the comparison with the manually rewritten query shows remaining optimization headroom rather than a
regressionagainst master: master evaluates the expression above Gather and also pays for partial and final aggregation.
Inmy paired release-build A/B on a smaller 100k-row, eight-partition synthetic table, disabling competing grouping
methodsto isolate the path placement took this case from 521.850 ms to 165.189 ms median. I do not generalize that
numberto your production-like workload; the useful point is the placement change and that the planner now has both
alternatives.

Partial paths for ORDER BY

Also fixed. The singleton branch now constructs direct partial paths, so create_ordered_paths() can consider
worker-localsorting followed by Gather Merge. With partitionwise aggregation enabled, ordinary partitionwise grouping
pathsremain available and compete with the singleton paths. In particular, the planner can now consider: 

Gather Merge
-> Sort
-> Result
-> Parallel Append
-> Parallel Seq Scan

rather than being forced into:

Sort
-> Gather
-> Parallel Append
-> Parallel Seq Scan

As you noted, this is not necessarily a performance win by itself. My ordered test also showed that Gather Merge
overheadcan offset the benefit of moving the sort downward. Therefore I am treating this as a plan-space fix: the
plannernow has the opportunity to choose worker-local sorting, but the cost model still decides among it, direct
completepaths, and ordinary partitionwise grouping. 

Volatile output columns under partial retrieval

While addressing your parallel-safe volatile observation, I also made the partial-fetch case conservative. When LIMIT
isneeded, or tuple_fraction indicates partial retrieval, and the query target contains a volatile function, the planner
keepsthe ordinary grouping paths and does not add the direct singleton paths. This avoids changing observable
evaluationcounts or moving side effects into workers for cases where the old grouping plan might stop early. 

Full-consumption volatile grouping-expression tests retain their expected counts. New tests also cover volatile output
columnswith LIMIT and an exact sequence-call check. 

Set-operation children

A UNION parent still asks for sorted children, so it rejects the optimization. A UNION ALL child does not require
orderedinput and may use its own local singleton proof. I corrected the comment to state this boundary precisely and
addedcoverage for both UNION and UNION ALL plan shapes. 

Testing

The focused singleton_grouping regression passes, as does the full core regression suite (240/240). Coverage includes
opfamily/collationmismatches, prepared-plan invalidation, partitioned tables, partitionwise aggregate retention,
worker-localordering, parallel-safe and parallel-restricted targets, volatile output columns with LIMIT, UNION and
UNIONALL children, inheritance, nullable keys, and implicit/degenerate grouping boundaries. 

Your timings and mine are individual observations or limited synthetic medians, so I am not claiming a universal
performancewin. The structural goal of v3 is that the cost model has the right alternatives: ordinary grouping,
partitionwisegrouping, direct complete paths, and direct partial paths. 

Sorry about the literal " " strings in the archived v1 email. This message is plain text and should not contain them.

Thanks again for catching both points; they materially improved the path design.

Regards,
Xiangxin