From 3753df8f8ae8e16bf81bc809514982b35a94f4a1 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 13 Jul 2020 20:38:21 -0400 Subject: [PATCH] Cope with lateral references in the quals of a subquery RTE. The qual pushdown logic assumed that all Vars in a restriction clause must be Vars referencing subquery outputs; but since we introduced LATERAL, it's possible for such a Var to be a lateral reference instead. This led to an assertion failure in debug builds. In a non-debug build, there might be no ill effects (if qual_is_pushdown_safe decided the qual was unsafe anyway), or we could get failures later due to construction of an invalid plan. I've not gone to much length to characterize the possible failures, but at least segfaults in the executor have been observed. Given that this has been busted since 9.3 and it took this long for anybody to notice, I judge that the case isn't worth going to great lengths to optimize. Hence, fix by just teaching qual_is_pushdown_safe that such quals are unsafe to push down, matching the previous behavior when it accidentally didn't fail. Per report from Tom Ellis. Back-patch to all supported branches. Discussion: https://postgr.es/m/20200713175124.GQ8220@cloudinit-builder --- src/backend/optimizer/path/allpaths.c | 20 ++++++++-- src/test/regress/expected/subselect.out | 53 +++++++++++++++++++++++++ src/test/regress/sql/subselect.sql | 26 ++++++++++++ 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index b601cc6a11b..0c6a348f0ea 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -3154,8 +3154,10 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual, Assert(!contain_window_function(qual)); /* - * Examine all Vars used in clause; since it's a restriction clause, all - * such Vars must refer to subselect output columns. + * Examine all Vars used in clause. Since it's a restriction clause, all + * such Vars must refer to subselect output columns ... unless this is + * part of a LATERAL subquery, in which case there could be lateral + * references. */ vars = pull_var_clause(qual, PVC_INCLUDE_PLACEHOLDERS); foreach(vl, vars) @@ -3175,7 +3177,19 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual, break; } - Assert(var->varno == rti); + /* + * Punt if we find any lateral references. It would be safe to push + * these down, but we'd have to convert them into outer references, + * which subquery_push_qual lacks the infrastructure to do. The case + * arises so seldom that it doesn't seem worth working hard on. + */ + if (var->varno != rti) + { + safe = false; + break; + } + + /* Subqueries have no system columns */ Assert(var->varattno >= 0); /* Check point 4 */ diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 181a792fcf1..48e1572afa4 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -1127,6 +1127,59 @@ from int4_tbl; (4,5,6.0) (5 rows) +-- +-- Check for sane handling of a lateral reference in a subquery's quals +-- (most of the complication here is to prevent the test case from being +-- flattened too much) +-- +explain (verbose, costs off) +select * from + int4_tbl i4, + lateral ( + select i4.f1 > 1 as b, 1 as id + from (select random() order by 1) as t1 + union all + select true as b, 2 as id + ) as t2 +where b and f1 >= 0; + QUERY PLAN +-------------------------------------------- + Nested Loop + Output: i4.f1, ((i4.f1 > 1)), (1) + -> Seq Scan on public.int4_tbl i4 + Output: i4.f1 + Filter: (i4.f1 >= 0) + -> Append + -> Subquery Scan on t1 + Output: (i4.f1 > 1), 1 + Filter: (i4.f1 > 1) + -> Sort + Output: (random()) + Sort Key: (random()) + -> Result + Output: random() + -> Result + Output: true, 2 +(16 rows) + +select * from + int4_tbl i4, + lateral ( + select i4.f1 > 1 as b, 1 as id + from (select random() order by 1) as t1 + union all + select true as b, 2 as id + ) as t2 +where b and f1 >= 0; + f1 | b | id +------------+---+---- + 0 | t | 2 + 123456 | t | 1 + 123456 | t | 2 + 2147483647 | t | 1 + 2147483647 | t | 2 +(5 rows) + -- -- Check that volatile quals aren't pushed down past a DISTINCT: -- nextval() should not be called more than the nominal number of times diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 527d512b690..9909ee28963 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -601,6 +601,32 @@ select (select q from ) q ) from int4_tbl; +-- +-- Check for sane handling of a lateral reference in a subquery's quals +-- (most of the complication here is to prevent the test case from being +-- flattened too much) +-- +explain (verbose, costs off) +select * from + int4_tbl i4, + lateral ( + select i4.f1 > 1 as b, 1 as id + from (select random() order by 1) as t1 + union all + select true as b, 2 as id + ) as t2 +where b and f1 >= 0; + +select * from + int4_tbl i4, + lateral ( + select i4.f1 > 1 as b, 1 as id + from (select random() order by 1) as t1 + union all + select true as b, 2 as id + ) as t2 +where b and f1 >= 0; + -- -- Check that volatile quals aren't pushed down past a DISTINCT: -- nextval() should not be called more than the nominal number of times