| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | shihao zhong <zhong950419(at)gmail(dot)com> |
| Cc: | Himanshu Upadhyaya <upadhyaya(dot)himanshu(at)gmail(dot)com>, 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:48:47 |
| Message-ID: | 147106.1788742127@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
shihao zhong <zhong950419(at)gmail(dot)com> writes:
>>> 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.
What does this test, other than the test module itself? The hook
call in itself has no interesting behavior, and I don't think that
we need to spin up a whole database session just to pass through
that code line.
In general, I think people have become far too enamored of tests
for the sake of tests, and are not thinking about the downstream
costs in CI and buildfarm cycles, developer time spent waiting for
tests to run, and for that matter the nonzero ongoing maintenance
effort that every test script has. Sure, those costs are tiny
on a one-time basis, but they add up fast.
It's a good idea to write something like this to convince yourself
that the hook is usable for something useful, but that doesn't mean
we need to carry that proof in-tree forevermore.
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | shihao zhong | 2026-09-07 00:13:13 | Re: Fire create_upper_paths_hook for UPPERREL_PARTIAL_GROUP_AGG |