pgsql: Fix a PlaceHolderVar-related oversight in star-schema planning p - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Fix a PlaceHolderVar-related oversight in star-schema planning p
Date
Msg-id E1ZMhNf-0002HX-5z@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Fix a PlaceHolderVar-related oversight in star-schema planning patch.

In commit b514a7460d9127ddda6598307272c701cbb133b7, I changed the planner
so that it would allow nestloop paths to remain partially parameterized,
ie the inner relation might need parameters from both the current outer
relation and some upper-level outer relation.  That's fine so long as we're
talking about distinct parameters; but the patch also allowed creation of
nestloop paths for cases where the inner relation's parameter was a
PlaceHolderVar whose eval_at set included the current outer relation and
some upper-level one.  That does *not* work.

In principle we could allow such a PlaceHolderVar to be evaluated at the
lower join node using values passed down from the upper relation along with
values from the join's own outer relation.  However, nodeNestloop.c only
supports simple Vars not arbitrary expressions as nestloop parameters.
createplan.c is also a few bricks shy of being able to handle such cases;
it misplaces the PlaceHolderVar parameters in the plan tree, which is why
the visible symptoms of this bug are "plan should not reference subplan's
variable" and "failed to assign all NestLoopParams to plan nodes" planner
errors.

Adding the necessary complexity to make this work doesn't seem like it
would be repaid in significantly better plans, because in cases where such
a PHV exists, there is probably a corresponding join order constraint that
would allow a good plan to be found without using the star-schema exception.
Furthermore, adding complexity to nodeNestloop.c would create a run-time
penalty even for plans where this whole consideration is irrelevant.
So let's just reject such paths instead.

Per fuzz testing by Andreas Seltenreich; the added regression test is based
on his example query.  Back-patch to 9.2, like the previous patch.

Branch
------
REL9_4_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/b58e8caf0cdd5e2657771b32a84464d7ff5ecebc

Modified Files
--------------
src/backend/optimizer/path/joinpath.c |  102 ++++++++++++++++++++++++---------
src/test/regress/expected/join.out    |   51 +++++++++++++++++
src/test/regress/sql/join.sql         |   31 ++++++++++
3 files changed, 158 insertions(+), 26 deletions(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Fix a PlaceHolderVar-related oversight in star-schema planning p
Next
From: Tom Lane
Date:
Subject: pgsql: Fix a PlaceHolderVar-related oversight in star-schema planning p