diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2013-08-17 20:22:41 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2013-08-17 20:22:41 -0400 |
commit | 517db4945560358a82b9152d01cfad3bbd2af17e (patch) | |
tree | ebc6a2807696fdb860aec87e19b806430500a4b3 /src/backend/nodes | |
parent | 2505aaed7be290018f999f6e9250c24b26db97d0 (diff) | |
download | postgresql-517db4945560358a82b9152d01cfad3bbd2af17e.tar.gz postgresql-517db4945560358a82b9152d01cfad3bbd2af17e.zip |
Fix planner problems with LATERAL references in PlaceHolderVars.
The planner largely failed to consider the possibility that a
PlaceHolderVar's expression might contain a lateral reference to a Var
coming from somewhere outside the PHV's syntactic scope. We had a previous
report of a problem in this area, which I tried to fix in a quick-hack way
in commit 4da6439bd8553059766011e2a42c6e39df08717f, but Antonin Houska
pointed out that there were still some problems, and investigation turned
up other issues. This patch largely reverts that commit in favor of a more
thoroughly thought-through solution. The new theory is that a PHV's
ph_eval_at level cannot be higher than its original syntactic level. If it
contains lateral references, those don't change the ph_eval_at level, but
rather they create a lateral-reference requirement for the ph_eval_at join
relation. The code in joinpath.c needs to handle that.
Another issue is that createplan.c wasn't handling nested PlaceHolderVars
properly.
In passing, push knowledge of lateral-reference checks for join clauses
into join_clause_is_movable_to. This is mainly so that FDWs don't need
to deal with it.
This patch doesn't fix the original join-qual-placement problem reported by
Jeremy Evans (and indeed, one of the new regression test cases shows the
wrong answer because of that). But the PlaceHolderVar problems need to be
fixed before that issue can be addressed, so committing this separately
seems reasonable.
Diffstat (limited to 'src/backend/nodes')
-rw-r--r-- | src/backend/nodes/copyfuncs.c | 3 | ||||
-rw-r--r-- | src/backend/nodes/equalfuncs.c | 21 | ||||
-rw-r--r-- | src/backend/nodes/outfuncs.c | 4 |
3 files changed, 18 insertions, 10 deletions
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 525e3fa2d19..6b20e317323 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -1917,8 +1917,8 @@ _copyLateralJoinInfo(const LateralJoinInfo *from) { LateralJoinInfo *newnode = makeNode(LateralJoinInfo); - COPY_SCALAR_FIELD(lateral_rhs); COPY_BITMAPSET_FIELD(lateral_lhs); + COPY_BITMAPSET_FIELD(lateral_rhs); return newnode; } @@ -1952,6 +1952,7 @@ _copyPlaceHolderInfo(const PlaceHolderInfo *from) COPY_SCALAR_FIELD(phid); COPY_NODE_FIELD(ph_var); COPY_BITMAPSET_FIELD(ph_eval_at); + COPY_BITMAPSET_FIELD(ph_lateral); COPY_BITMAPSET_FIELD(ph_needed); COPY_SCALAR_FIELD(ph_width); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 379cbc0c203..b49e1e731de 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -761,15 +761,19 @@ _equalPlaceHolderVar(const PlaceHolderVar *a, const PlaceHolderVar *b) /* * We intentionally do not compare phexpr. Two PlaceHolderVars with the * same ID and levelsup should be considered equal even if the contained - * expressions have managed to mutate to different states. One way in - * which that can happen is that initplan sublinks would get replaced by - * differently-numbered Params when sublink folding is done. (The end - * result of such a situation would be some unreferenced initplans, which - * is annoying but not really a problem.) + * expressions have managed to mutate to different states. This will + * happen during final plan construction when there are nested PHVs, since + * the inner PHV will get replaced by a Param in some copies of the outer + * PHV. Another way in which it can happen is that initplan sublinks + * could get replaced by differently-numbered Params when sublink folding + * is done. (The end result of such a situation would be some + * unreferenced initplans, which is annoying but not really a problem.) On + * the same reasoning, there is no need to examine phrels. * * COMPARE_NODE_FIELD(phexpr); + * + * COMPARE_BITMAPSET_FIELD(phrels); */ - COMPARE_BITMAPSET_FIELD(phrels); COMPARE_SCALAR_FIELD(phid); COMPARE_SCALAR_FIELD(phlevelsup); @@ -794,8 +798,8 @@ _equalSpecialJoinInfo(const SpecialJoinInfo *a, const SpecialJoinInfo *b) static bool _equalLateralJoinInfo(const LateralJoinInfo *a, const LateralJoinInfo *b) { - COMPARE_SCALAR_FIELD(lateral_rhs); COMPARE_BITMAPSET_FIELD(lateral_lhs); + COMPARE_BITMAPSET_FIELD(lateral_rhs); return true; } @@ -817,8 +821,9 @@ static bool _equalPlaceHolderInfo(const PlaceHolderInfo *a, const PlaceHolderInfo *b) { COMPARE_SCALAR_FIELD(phid); - COMPARE_NODE_FIELD(ph_var); + COMPARE_NODE_FIELD(ph_var); /* should be redundant */ COMPARE_BITMAPSET_FIELD(ph_eval_at); + COMPARE_BITMAPSET_FIELD(ph_lateral); COMPARE_BITMAPSET_FIELD(ph_needed); COMPARE_SCALAR_FIELD(ph_width); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 67aeb7e65c3..f6e211429c3 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1752,6 +1752,7 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node) WRITE_INT_FIELD(max_attr); WRITE_NODE_FIELD(lateral_vars); WRITE_BITMAPSET_FIELD(lateral_relids); + WRITE_BITMAPSET_FIELD(lateral_referencers); WRITE_NODE_FIELD(indexlist); WRITE_UINT_FIELD(pages); WRITE_FLOAT_FIELD(tuples, "%.0f"); @@ -1909,8 +1910,8 @@ _outLateralJoinInfo(StringInfo str, const LateralJoinInfo *node) { WRITE_NODE_TYPE("LATERALJOININFO"); - WRITE_UINT_FIELD(lateral_rhs); WRITE_BITMAPSET_FIELD(lateral_lhs); + WRITE_BITMAPSET_FIELD(lateral_rhs); } static void @@ -1934,6 +1935,7 @@ _outPlaceHolderInfo(StringInfo str, const PlaceHolderInfo *node) WRITE_UINT_FIELD(phid); WRITE_NODE_FIELD(ph_var); WRITE_BITMAPSET_FIELD(ph_eval_at); + WRITE_BITMAPSET_FIELD(ph_lateral); WRITE_BITMAPSET_FIELD(ph_needed); WRITE_INT_FIELD(ph_width); } |