| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | pgsql: Delay qp_callback processing till after join removal. |
| Date: | 2026-10-13 21:53:49 |
| Message-ID: | E1xGkRR-00000001UvW-0HWb@gemulon.postgresql.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
Delay qp_callback processing till after join removal.
Commit 2ebf25e7d left query_planner's qp_callback call where
it had been historically, but that turns out to be a bad idea.
It means that the code reached via qp_callback is subject to the
must-be-able-to-restart rule created by looping back after join
removal. We found one place that fails that requirement:
adjust_group_pathkeys_for_groupagg sets Aggref.aggpresorted flags,
and there's nothing that will reset those in case of a query_planner
restart, yet the selected set of Aggrefs can change. There might be
other places with similar issues.
Fortunately, qp_callback's main charter is to compute/update pathkey
values, and that is not something that join removal depends on or
should depend on. (Whether join removal is valid is surely not
dependent on what order the rel might get scanned in.) Nothing else
within the restart loop depends on it either --- in other words,
the existing placement was a bit random to begin with. So we can
fix this problem just by moving that call to below the restart loop.
That costs nothing, indeed it saves some cycles by not having to do
that processing more than once. And removing an unstated requirement
from code that's not directly called by query_planner should help
forestall future bugs.
While the known bug only exists in v18 and up (older branches lack the
aggpresorted optimization), I've chosen to back-patch the code change
to v16 like 2ebf25e7d was. This is the sort of API requirement that
we don't really want to be different across branches.
Reported-by: shihao zhong <zhong950419(at)gmail(dot)com>
Author: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Reviewed-by: shihao zhong <zhong950419(at)gmail(dot)com>
Discussion: https://postgr.es/m/CAGRkXqSOAT1yop44B8jwacL_91rxK=QuoU8Of3NLMb9NgAO2NA@mail.gmail.com
Backpatch-through: 16
Branch
------
REL_16_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/38fe5a93665a064bb93a83915098df557be52a8c
Modified Files
--------------
src/backend/optimizer/plan/planmain.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-10-14 01:17:42 | pgsql: Release memory allocated by ndistinct_for_combination() |
| Previous Message | Tom Lane | 2026-10-13 21:53:48 | pgsql: Delay qp_callback processing till after join removal. |