I wrote:
> What's apparently happening is that we pull up the sub-select,
> and eval_const_expressions flattens the CASE to constant-NULL,
> so we don't see the reference to sample_1.a and conclude that
> sample_1 can be dropped from the query. But when we do that,
> we try to mutate the original copy of the sub-select's tlist
> which still contains sample_1.a, and so we hit the Assert
> saying we should no longer see any such Vars.
More specifically, the raw CASE expression gets pushed down into
the lateral subquery, wrapped in a PlaceHolderVar. When we
pull it back up in extract_lateral_references(), we apply
expression preprocessing which flattens the CASE to constant-NULL.
So the lateral reference expression seen by the upper query has
no reference to sample_1, allowing join removal to remove sample_1.
But the PHV inside the subquery still has a reference, which makes
ChangeVarNodes fail, and even if we prevented that it'd still be
trouble when we repeat extract_lateral_references() after restarting.
So after contemplating my navel for awhile, the least grotty
solution I can see is to update the subquery's copy of the PHV
with the preprocessed expression, as attached. I don't really
love doing that, but the alternatives I can think of are worse.
Anyone have another idea?
One interesting point about this query is that if you remove
the topmost right join ("RIGHT JOIN public.ruletest_tbl2 AS ref_3
ON NULL" in the original example, "right join int4_tbl i42 on false"
in my proposed regression test), there's no failure. That's not
so surprising, because without that join we don't have to wrap the
CASE in a PlaceHolderVar, and then there's no discrepancy between what
the subquery contains and what the outer level thinks it contains.
But what is surprising is that we then fail to make the join removal,
which seems like a missed optimization. I didn't poke into that, but
I think it'd be worth figuring out why the removal doesn't happen.
regards, tom lane
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index a2fb0b55a79..1c60b7b5bcf 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -1118,9 +1118,9 @@ extract_lateral_references(PlannerInfo *root, RelOptInfo *brel, Index rtindex)
newvars = NIL;
foreach(lc, vars)
{
- Node *node = (Node *) lfirst(lc);
+ Node *orig_node = (Node *) lfirst(lc);
+ Node *node = copyObject(orig_node);
- node = copyObject(node);
if (IsA(node, Var))
{
Var *var = (Var *) node;
@@ -1141,9 +1141,27 @@ extract_lateral_references(PlannerInfo *root, RelOptInfo *brel, Index rtindex)
* If we pulled the PHV out of a subquery RTE, its expression
* needs to be preprocessed. subquery_planner() already did this
* for level-zero PHVs in function and values RTEs, though.
+ *
+ * Furthermore, we modify the subquery by putting the preprocessed
+ * expression back into the subquery's PHV, after reversing the
+ * levelsup adjustment again. This is rather grotty: it'd be
+ * better if this processing didn't modify the subquery. However,
+ * it's essential for some optimization scenarios. For example,
+ * if the PHV contains a Var of our level that is deleted during
+ * preprocessing (say, by simplifying a CASE with constant test
+ * expression), join simplification may decide that it can remove
+ * the rel that is the source of the Var. If the subquery still
+ * contains a reference to that Var, trouble will ensue.
*/
if (levelsup > 0)
+ {
+ Expr *repl_expr;
+
phv->phexpr = preprocess_phv_expression(root, phv->phexpr);
+ repl_expr = copyObject(phv->phexpr);
+ IncrementVarSublevelsUp((Node *) repl_expr, levelsup, 0);
+ ((PlaceHolderVar *) orig_node)->phexpr = repl_expr;
+ }
}
else
Assert(false);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index db4fcc5a5a0..bebad6e4406 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -7043,6 +7043,35 @@ on lhs.id = rhs.id;
-> Result
(5 rows)
+-- check handling of a removed Var that's pushed down into a subquery
+-- (fallout from the fix for bug #19560)
+explain (verbose, costs off)
+select c1, c2
+from (select case when false then remov.id end as c1
+ from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
+ right join int4_tbl i42 on false,
+ lateral (select ss1.c1 as c2 from int4_tbl i43 offset 0) ss2;
+ QUERY PLAN
+----------------------------------------------
+ Nested Loop
+ Output: (NULL::integer), ((NULL::integer))
+ -> Nested Loop Left Join
+ Output: (NULL::integer)
+ Join Filter: false
+ -> Seq Scan on public.int4_tbl i42
+ Output: i42.f1
+ -> Result
+ Output: NULL::integer
+ Replaces: Scan on i41
+ One-Time Filter: false
+ -> Memoize
+ Output: ((NULL::integer))
+ Cache Key: (NULL::integer)
+ Cache Mode: binary
+ -> Seq Scan on public.int4_tbl i43
+ Output: (NULL::integer)
+(17 rows)
+
-- More tests of correct placement of pseudoconstant quals
-- simple constant-false condition
explain (costs off)
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 9533af8656e..088aa26c69a 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2581,6 +2581,15 @@ full join
) as rhs
on lhs.id = rhs.id;
+-- check handling of a removed Var that's pushed down into a subquery
+-- (fallout from the fix for bug #19560)
+explain (verbose, costs off)
+select c1, c2
+from (select case when false then remov.id end as c1
+ from int4_tbl i41 left join a remov on i41.f1 = remov.id) ss1
+ right join int4_tbl i42 on false,
+ lateral (select ss1.c1 as c2 from int4_tbl i43 offset 0) ss2;
+
-- More tests of correct placement of pseudoconstant quals
-- simple constant-false condition
