| From: | Robert Haas <rhaas(at)postgresql(dot)org> |
|---|---|
| To: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | pgsql: pg_plan_advice: Fix defects in JOIN_ORDER advice feedback. |
| Date: | 2026-09-17 15:41:06 |
| Message-ID: | E1x7EEU-000000008Cf-0IIA@gemulon.postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
pg_plan_advice: Fix defects in JOIN_ORDER advice feedback.
This commit fixes several related problems. All of these problems
escaped testing for the same reason: they do not occur when only
generated advice is supplied, and therefore test_plan_advice was
incapable of detecting these shortcomings.
First, prior to this commit, the advice feedback code never regarded
an unordered sublist of a JOIN_ORDER specification as matching an
unrolled join. As a result, when JOIN_ORDER() advice contained
unordered sublists, the advice feedback tended to be "matched, failed"
even when the advice worked exactly as intended. Only the case where
an unordered sublist was implemented by a plan shape not subject to
unrolling, such as a partitionwise join, worked properly.
Second, prior to this commit, a join order sublist in the initial
position wasn't properly handled. Note that this overlaps with the
problem described in the previous paragraph; JOIN_ORDER({a b} c) was
broken both because of the unordered sublist and because of the
sublist being in the initial position. However, there's more to this
case: JOIN_ORDER((a b) c) means the same as JOIN_ORDER(a b c), but the
advice feedback code didn't know that, and would generate "matched,
failed" for the former case even when everything was working.
Finally, prior to this commit, advice feedback didn't correctly handle
single-element sublists. Those are pretty nonsensical, since by
definition a join involves at least 2 tables, so we could just ban
that case. Instead, at least for now, I've chosen to make advice
feedback handle such cases in the same way that advice enforcement
already does: the extra grouping levels are simply disregarded, so
that JOIN_ORDER({a} ((b))) is enforced in the same way as, and also
gets the same advice feedback as, JOIN_ORDER(a b).
Reported-by: Noah Misch <noah(at)leadboat(dot)com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+TgmoYmXy-jiP5qDhqNEiYFEBzQsArO6O2d9E8szNZqi1bePQ@mail.gmail.com
Reviewed-by: Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/e1d8f81d4961ca6c4228eb9a3ab5d36168008e42
Modified Files
--------------
contrib/pg_plan_advice/expected/join_order.out | 205 ++++++++++++++-
contrib/pg_plan_advice/expected/partitionwise.out | 58 +++++
contrib/pg_plan_advice/pgpa_walker.c | 296 +++++++++++++++++-----
contrib/pg_plan_advice/sql/join_order.sql | 51 ++++
contrib/pg_plan_advice/sql/partitionwise.sql | 8 +
5 files changed, 554 insertions(+), 64 deletions(-)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Daniel Gustafsson | 2026-09-17 16:53:20 | Re: pgsql: Revert online data checksum transitions |
| Previous Message | Robert Haas | 2026-09-17 13:47:33 | pgsql: Change GEQO fitness comparisons to consider disabled_nodes. |