| From: | zengxx <xiangxin_zeng(at)qq(dot)com> |
|---|---|
| To: | ChenhuiMo <chenhuimo(dot)mch(at)qq(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | 回复: Skip a redundant singleton GROUP BY node |
| Date: | 2026-09-24 01:26:46 |
| Message-ID: | tencent_4E3A37FD5452D9A03C53D24047A8B302F80A@qq.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | 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 as v3 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.
1. 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.
2. 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.
3. 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.
4. 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 "&nbsp;" 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
------------------ 原始邮件 ------------------
发件人: "ChenhuiMo" <chenhuimo(dot)mch(at)qq(dot)com>;
发送时间: 2026年9月21日(星期一) 晚上10:15
收件人: "zengxx"<xiangxin_zeng(at)qq(dot)com>;"pgsql-hackers"<pgsql-hackers(at)lists(dot)postgresql(dot)org>;
主题: Re: Skip a redundant singleton GROUP BY node
Hi Xiangxin,
Thanks for the patch. I tested v1 on PostgreSQL 20devel, including unique-key grouping,
expensive target expressions, volatile expressions, partition pruning, and parallel plans.
The elimination works well in the cases I tested. In particular, queries with ORDER BY
and LIMIT can avoid both grouping and sorting by retaining an ordered index path.
The full-consumption sequence tests also preserved the expected evaluation counts.
I noticed two areas that might deserve further consideration.
Projection below Gather
After grouping is eliminated, a parallel-safe expression that appears only in the
SELECT list can still be evaluated by the leader.
For example, my partitioned table has 300,000 rows across eight partitions, a
primary key on (bucket, id), and an approximately 1 KB payload column:
SELECT bucket, id, sg_review.slow_i(payload)
FROM sg_review.part_heap
GROUP BY bucket, id;
Here, slow_i is an expensive PL/pgSQL function declared IMMUTABLE, STRICT,
PARALLEL SAFE, and COST 100. With the patch, the relevant plan output is:
Gather
Output: bucket, id, slow_i(payload)
-> Parallel Append
-> Parallel Seq Scan
Output: bucket, id, payload
Removing the redundant GROUP BY manually produces a plan that evaluates
slow_i in the partition scans below Gather. Both plans launch two workers,
but the grouped query took about 3962 ms, versus 1302 ms without GROUP BY.
I observed the same behavior with an otherwise identical function declared
VOLATILE and PARALLEL SAFE, and with an ordinary non-partitioned table.
This is an optimization opportunity rather than a demonstrated regression
against master: master also evaluated the function above Gather, and additionally
performed partial and final aggregation.
It looks like create_singleton_grouping_paths() applies the final target to existing
complete paths through create_projection_path(), without pushing that target
below an existing Gather. Would it be worth considering paths that evaluate the
parallel-safe target in workers?
Partial paths for subsequent ORDER BY
With partitionwise aggregation enabled, I also tested:
SELECT bucket, id, rank_key
FROM sg_review.part_heap
GROUP BY bucket, id, rank_key
ORDER BY rank_key + 1;
Master chose Gather Merge with worker-local Sort nodes above a Parallel
Append of per-partition HashAggregate nodes. With the patch, grouping
disappeared, but the plan became a single Sort above Gather.
The singleton branch appears to populate only grouped_rel->pathlist.
Could it also preserve or construct suitable partial paths, so that create_ordered_paths()
can consider worker-local sorting followed by Gather Merge?
To be clear, this particular query was faster with the patch: approximately 44.5 ms versus
67.1 ms on master. Thus, this test demonstrates the change in sorting placement,
not a performance regression.
For these parallel tests, both builds used:
SET jit = off;
SET work_mem = '32MB';
SET enable_partitionwise_aggregate = on;
SET enable_parallel_append = on;
SET max_parallel_workers_per_gather = 2;
SET min_parallel_table_scan_size = 0;
SET min_parallel_index_scan_size = 0;
SET parallel_setup_cost = 0;
SET parallel_tuple_cost = 0;
The timings above are individual observations, not benchmark medians. So far, I have
not found incorrect results or a reproducible performance regression in the tested cases.
I've attached the SQL script, including the table and function definitions, data generation,
and test queries, along with the EXPLAIN ANALYZE output from master and the patched build.
One minor formatting note: the archived message contains several literal `&nbsp;` strings.
Could you remove those in the next version of the email? That would make the text easier to read.
Regards,
ChenHui Mo
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Move-unique-index-GROUP-BY-matching-into-indxpath.patch | application/octet-stream | 17.0 KB |
| v3-0002-Skip-a-redundant-singleton-GROUP-BY-node.patch | application/octet-stream | 68.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shihao zhong | 2026-09-24 01:30:19 | Reset waitStart when a lock wait fails |
| Previous Message | zengxx | 2026-09-24 01:16:34 | Skip a redundant singleton GROUP BY node |