| From: | ChenhuiMo <chenhuimo(dot)mch(at)qq(dot)com> |
|---|---|
| To: | zengxx <xiangxin_zeng(at)qq(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Skip a redundant singleton GROUP BY node |
| Date: | 2026-09-21 14:15:34 |
| Message-ID: | tencent_8354D56F81E7150ABC6D17D3A035A8CA9907@qq.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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 ` ` 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 |
|---|---|---|
| benchmark.sql | application/octet-stream | 18.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2026-09-21 14:24:21 | Re: [Patch] New pg_stat_tablespace view |
| Previous Message | shihao zhong | 2026-09-21 14:11:09 | Re: [Patch] New pg_stat_tablespace view |