Hi ChenHui Mo,

Thanks for the detailed tests and the clear plan output. Both observations are 
valid, and I have reorganized the series as v2 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 GroupByColInfo private to that file, and 
exposes only a higher-level function that returns the GROUP BY columns made 
redundant by a suitable unique index. It also clarifies that notnullattnums 
contains heap attribute numbers. Centralizing the NOT NULL, NULLS NOT DISTINCT, 
opfamily, and collation checks avoids maintaining a second matcher in the 
singleton code.

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 as alternatives 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 
the target 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 regression against master: master evaluates 
the expression above Gather and also pays for partial and final aggregation. In 
my paired release-build A/B on a smaller 100k-row, eight-partition synthetic 
table, disabling competing grouping methods to isolate the path placement took 
this case from 521.850 ms to 165.189 ms median. I do not generalize that number 
to 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-local sorting followed by Gather 
Merge. With partitionwise aggregation enabled, ordinary partitionwise grouping 
paths remain 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 overhead can offset the benefit of moving 
the sort downward. Therefore I am treating this as a plan-space fix: the 
planner now has the opportunity to choose worker-local sorting, but the cost 
model still decides among it, direct complete paths, 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 is needed, or tuple_fraction 
indicates partial retrieval, and the query target contains a volatile function, 
the planner keeps the ordinary grouping paths and does not add the direct 
singleton paths. This avoids changing observable evaluation counts 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 columns with 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 ordered input and may use its own local 
singleton proof. I corrected the comment to state this boundary precisely and 
added coverage 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/collation mismatches, 
prepared-plan invalidation, partitioned tables, partitionwise aggregate 
retention, worker-local ordering, parallel-safe and parallel-restricted 
targets, volatile output columns with LIMIT, UNION and UNION ALL 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 performance win. The structural goal of v3 is 
that the cost model has the right alternatives: ordinary grouping, 
partitionwise grouping, 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

Attachment: v2-0001-Move-unique-index-GROUP-BY-matching-into-indxpath.patch
Description: Binary data

Attachment: v2-0002-Skip-a-redundant-singleton-GROUP-BY-node.patch
Description: Binary data

Reply via email to