]> granicus.if.org Git - postgresql/commit
Fix a PlaceHolderVar-related oversight in star-schema planning patch.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 4 Aug 2015 18:55:32 +0000 (14:55 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 4 Aug 2015 18:55:50 +0000 (14:55 -0400)
commit85e5e222b1dd02f135a8c3bf387d0d6d88e669bd
tree9584cd1be1e24d7128cd4a9024db75b448d9e509
parent369342cf70972a81e6be99b31593f31b73479d7f
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.
src/backend/optimizer/path/joinpath.c
src/test/regress/expected/join.out
src/test/regress/sql/join.sql