From: Tom Lane Date: Tue, 10 Dec 2013 21:10:36 +0000 (-0500) Subject: Fix possible crash with nested SubLinks. X-Git-Tag: REL8_4_20~43 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=884c6384a2db34f6a65573e6bfd4b71dfba0de90;p=postgresql Fix possible crash with nested SubLinks. An expression such as WHERE (... x IN (SELECT ...) ...) IN (SELECT ...) could produce an invalid plan that results in a crash at execution time, if the planner attempts to flatten the outer IN into a semi-join. This happens because convert_testexpr() was not expecting any nested SubLinks and would wrongly replace any PARAM_SUBLINK Params belonging to the inner SubLink. (I think the comment denying that this case could happen was wrong when written; it's certainly been wrong for quite a long time, since very early versions of the semijoin flattening logic.) Per report from Teodor Sigaev. Back-patch to all supported branches. --- diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 896d3865ab..bf0f25d781 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -772,11 +772,6 @@ generate_subquery_vars(PlannerInfo *root, List *tlist, Index varno) * with Params or Vars representing the results of the sub-select. The * nodes to be substituted are passed in as the List result from * generate_subquery_params or generate_subquery_vars. - * - * The given testexpr has already been recursively processed by - * process_sublinks_mutator. Hence it can no longer contain any - * PARAM_SUBLINK Params for lower SubLink nodes; we can safely assume that - * any we find are for our own level of SubLink. */ static Node * convert_testexpr(PlannerInfo *root, @@ -815,6 +810,28 @@ convert_testexpr_mutator(Node *node, param->paramid - 1)); } } + if (IsA(node, SubLink)) + { + /* + * If we come across a nested SubLink, it is neither necessary nor + * correct to recurse into it: any PARAM_SUBLINKs we might find inside + * belong to the inner SubLink not the outer. So just return it as-is. + * + * This reasoning depends on the assumption that nothing will pull + * subexpressions into or out of the testexpr field of a SubLink, at + * least not without replacing PARAM_SUBLINKs first. If we did want + * to do that we'd need to rethink the parser-output representation + * altogether, since currently PARAM_SUBLINKs are only unique per + * SubLink not globally across the query. The whole point of + * replacing them with Vars or PARAM_EXEC nodes is to make them + * globally unique before they escape from the SubLink's testexpr. + * + * Note: this can't happen when called during SS_process_sublinks, + * because that recursively processes inner SubLinks first. It can + * happen when called from convert_ANY_sublink_to_join, though. + */ + return node; + } return expression_tree_mutator(node, convert_testexpr_mutator, (void *) context); diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index 850777acd5..6194d259a1 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -639,3 +639,14 @@ where a.thousand = b.thousand ---------- (0 rows) +-- +-- Check sane behavior with nested IN SubLinks +-- +select * from int4_tbl where + (case when f1 in (select unique1 from tenk1 a) then f1 else null end) in + (select ten from tenk1 b); + f1 +---- + 0 +(1 row) + diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 8ca7a3bd2f..33b894c2b5 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -389,3 +389,10 @@ where a.thousand = b.thousand and exists ( select 1 from tenk1 c where b.hundred = c.hundred and not exists ( select 1 from tenk1 d where a.thousand = d.thousand ) ); + +-- +-- Check sane behavior with nested IN SubLinks +-- +select * from int4_tbl where + (case when f1 in (select unique1 from tenk1 a) then f1 else null end) in + (select ten from tenk1 b);