| From: | shihao zhong <zhong950419(at)gmail(dot)com> |
|---|---|
| To: | Himanshu Upadhyaya <upadhyaya(dot)himanshu(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, Andrew Dunstan <amdunstan(at)gmail(dot)com> |
| Subject: | Re: Fire create_upper_paths_hook for UPPERREL_PARTIAL_GROUP_AGG |
| Date: | 2026-09-07 00:13:13 |
| Message-ID: | CAGRkXqRZBMUKQXtKiVbHXWR9obG-pufE-i5a5jgcg44GhHezhA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> create_upper_paths_hook has no test coverage anywhere in the tree
I agree this patch needs test coverage, so I have attached 0002. It
reuses test_extensible, which already installs a planner hook: a new
GUC makes it add a partial aggregate path from create_upper_paths_hook,
and the test checks that the planner gathers it and builds Finalize
Aggregate on top. It fails without 0001.
Nit on the code comment: the other five call sites use a single line,
so maybe just "/* Let extensions possibly add some more partial paths
*/" and move the rest to the commit message. "Heap and other AMs are
unaffected" also looks like a leftover from v2.
I will leave it to the author to decide whether to take these.
Thanks,
Shihao
| Attachment | Content-Type | Size |
|---|---|---|
| 0002-Test-create_upper_paths_hook-for-UPPERREL_PARTIAL_GR.patch | application/x-patch | 5.0 KB |
| v3-0001-Fire-create_upper_paths_hook-for-UPPERREL_PARTIAL.patch | application/x-patch | 2.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-09-07 00:48:47 | Re: Fire create_upper_paths_hook for UPPERREL_PARTIAL_GROUP_AGG |
| Previous Message | Michael Paquier | 2026-09-06 23:47:33 | Re: Rename of varatt_external to varatt_external_oid |