Track nesting depth correctly when drilling down into RECORD Vars.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 15 Sep 2023 21:01:26 +0000 (17:01 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 15 Sep 2023 21:01:52 +0000 (17:01 -0400)
expandRecordVariable() failed to adjust the parse nesting structure
correctly when recursing to inspect an outer-level Var.  This could
result in assertion failures or core dumps in corner cases.

Likewise, get_name_for_var_field() failed to adjust the deparse
namespace stack correctly when recursing to inspect an outer-level
Var.  In this case the likely result was a "bogus varno" error
while deparsing a view.

Per bug #18077 from Jingzhou Fu.  Back-patch to all supported
branches.

Richard Guo, with some adjustments by me

Discussion: https://postgr.es/m/18077-b9db97c6e0ab45d8@postgresql.org

src/backend/parser/parse_target.c
src/backend/utils/adt/ruleutils.c
src/test/regress/expected/rowtypes.out
src/test/regress/sql/rowtypes.sql

index 57247de363baf2d4c3a3b8122b401aa46329843d..3bc62ac3ba5f23580dd110fad20552fa56bd0462 100644 (file)
@@ -1499,7 +1499,8 @@ ExpandRowReference(ParseState *pstate, Node *expr,
  * drill down to find the ultimate defining expression and attempt to infer
  * the tupdesc from it.  We ereport if we can't determine the tupdesc.
  *
- * levelsup is an extra offset to interpret the Var's varlevelsup correctly.
+ * levelsup is an extra offset to interpret the Var's varlevelsup correctly
+ * when recursing.  Outside callers should pass zero.
  */
 TupleDesc
 expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
@@ -1587,10 +1588,17 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
                    /*
                     * Recurse into the sub-select to see what its Var refers
                     * to.  We have to build an additional level of ParseState
-                    * to keep in step with varlevelsup in the subselect.
+                    * to keep in step with varlevelsup in the subselect;
+                    * furthermore, the subquery RTE might be from an outer
+                    * query level, in which case the ParseState for the
+                    * subselect must have that outer level as parent.
                     */
                    ParseState  mypstate = {0};
+                   Index       levelsup;
 
+                   /* this loop must work, since GetRTEByRangeTablePosn did */
+                   for (levelsup = 0; levelsup < netlevelsup; levelsup++)
+                       pstate = pstate->parentParseState;
                    mypstate.parentParseState = pstate;
                    mypstate.p_rtable = rte->subquery->rtable;
                    /* don't bother filling the rest of the fake pstate */
@@ -1641,12 +1649,11 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
                     * Recurse into the CTE to see what its Var refers to. We
                     * have to build an additional level of ParseState to keep
                     * in step with varlevelsup in the CTE; furthermore it
-                    * could be an outer CTE.
+                    * could be an outer CTE (compare SUBQUERY case above).
                     */
-                   ParseState  mypstate;
+                   ParseState  mypstate = {0};
                    Index       levelsup;
 
-                   MemSet(&mypstate, 0, sizeof(mypstate));
                    /* this loop must work, since GetCTEForRTE did */
                    for (levelsup = 0;
                         levelsup < rte->ctelevelsup + netlevelsup;
index 97b0ef22ac5cc60406c65dde7a0b746c72b8c094..68f301484e38a720504ee003f58e7b3b58bcb8cc 100644 (file)
@@ -7820,22 +7820,28 @@ get_name_for_var_field(Var *var, int fieldno,
                         * Recurse into the sub-select to see what its Var
                         * refers to. We have to build an additional level of
                         * namespace to keep in step with varlevelsup in the
-                        * subselect.
+                        * subselect; furthermore, the subquery RTE might be
+                        * from an outer query level, in which case the
+                        * namespace for the subselect must have that outer
+                        * level as parent namespace.
                         */
+                       List       *save_nslist = context->namespaces;
+                       List       *parent_namespaces;
                        deparse_namespace mydpns;
                        const char *result;
 
+                       parent_namespaces = list_copy_tail(context->namespaces,
+                                                          netlevelsup);
+
                        set_deparse_for_query(&mydpns, rte->subquery,
-                                             context->namespaces);
+                                             parent_namespaces);
 
-                       context->namespaces = lcons(&mydpns,
-                                                   context->namespaces);
+                       context->namespaces = lcons(&mydpns, parent_namespaces);
 
                        result = get_name_for_var_field((Var *) expr, fieldno,
                                                        0, context);
 
-                       context->namespaces =
-                           list_delete_first(context->namespaces);
+                       context->namespaces = save_nslist;
 
                        return result;
                    }
@@ -7927,7 +7933,7 @@ get_name_for_var_field(Var *var, int fieldno,
                                                        attnum);
 
                    if (ste == NULL || ste->resjunk)
-                       elog(ERROR, "subquery %s does not have attribute %d",
+                       elog(ERROR, "CTE %s does not have attribute %d",
                             rte->eref->aliasname, attnum);
                    expr = (Node *) ste->expr;
                    if (IsA(expr, Var))
@@ -7935,21 +7941,22 @@ get_name_for_var_field(Var *var, int fieldno,
                        /*
                         * Recurse into the CTE to see what its Var refers to.
                         * We have to build an additional level of namespace
-                        * to keep in step with varlevelsup in the CTE.
-                        * Furthermore it could be an outer CTE, so we may
-                        * have to delete some levels of namespace.
+                        * to keep in step with varlevelsup in the CTE;
+                        * furthermore it could be an outer CTE (compare
+                        * SUBQUERY case above).
                         */
                        List       *save_nslist = context->namespaces;
-                       List       *new_nslist;
+                       List       *parent_namespaces;
                        deparse_namespace mydpns;
                        const char *result;
 
+                       parent_namespaces = list_copy_tail(context->namespaces,
+                                                          ctelevelsup);
+
                        set_deparse_for_query(&mydpns, ctequery,
-                                             context->namespaces);
+                                             parent_namespaces);
 
-                       new_nslist = list_copy_tail(context->namespaces,
-                                                   ctelevelsup);
-                       context->namespaces = lcons(&mydpns, new_nslist);
+                       context->namespaces = lcons(&mydpns, parent_namespaces);
 
                        result = get_name_for_var_field((Var *) expr, fieldno,
                                                        0, context);
index 981ee0811a7a0e88e779b8bc6a98b2b01eb42b1e..8f3c153bac24533dec0381a23df33ec1e868bb62 100644 (file)
@@ -1240,6 +1240,66 @@ select r, r is null as isnull, r is not null as isnotnull from r;
  (,)         | t      | f
 (6 rows)
 
+--
+-- Check parsing of indirect references to composite values (bug #18077)
+--
+explain (verbose, costs off)
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select * from cte2 as t
+where (select * from (select c as c1) s
+       where (select (c1).f1 > 0)) is not null;
+                 QUERY PLAN                 
+--------------------------------------------
+ CTE Scan on cte
+   Output: cte.c
+   Filter: ((SubPlan 3) IS NOT NULL)
+   CTE cte
+     ->  Result
+           Output: '(1,2)'::record
+   SubPlan 3
+     ->  Result
+           Output: cte.c
+           One-Time Filter: $2
+           InitPlan 2 (returns $2)
+             ->  Result
+                   Output: ((cte.c).f1 > 0)
+(13 rows)
+
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select * from cte2 as t
+where (select * from (select c as c1) s
+       where (select (c1).f1 > 0)) is not null;
+   c   
+-------
+ (1,2)
+(1 row)
+
+-- Also check deparsing of such cases
+create view composite_v as
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select 1 as one from cte2 as t
+where (select * from (select c as c1) s
+       where (select (c1).f1 > 0)) is not null;
+select pg_get_viewdef('composite_v', true);
+                     pg_get_viewdef                     
+--------------------------------------------------------
+  WITH cte(c) AS MATERIALIZED (                        +
+          SELECT ROW(1, 2) AS "row"                    +
+         ), cte2(c) AS (                               +
+          SELECT cte.c                                 +
+            FROM cte                                   +
+         )                                             +
+  SELECT 1 AS one                                      +
+    FROM cte2 t                                        +
+   WHERE (( SELECT s.c1                                +
+            FROM ( SELECT t.c AS c1) s                 +
+           WHERE ( SELECT (s.c1).f1 > 0))) IS NOT NULL;
+(1 row)
+
+drop view composite_v;
 --
 -- Tests for component access / FieldSelect
 --
index 565e6249d501530e82d1345e895a98d07524941e..fd47dc9e0f674ec5d6c0dbf5352df73f01cf7ba1 100644 (file)
@@ -494,6 +494,31 @@ with r(a,b) as materialized
           (null,row(1,2)), (null,row(null,null)), (null,null) )
 select r, r is null as isnull, r is not null as isnotnull from r;
 
+--
+-- Check parsing of indirect references to composite values (bug #18077)
+--
+explain (verbose, costs off)
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select * from cte2 as t
+where (select * from (select c as c1) s
+       where (select (c1).f1 > 0)) is not null;
+
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select * from cte2 as t
+where (select * from (select c as c1) s
+       where (select (c1).f1 > 0)) is not null;
+
+-- Also check deparsing of such cases
+create view composite_v as
+with cte(c) as materialized (select row(1, 2)),
+     cte2(c) as (select * from cte)
+select 1 as one from cte2 as t
+where (select * from (select c as c1) s
+       where (select (c1).f1 > 0)) is not null;
+select pg_get_viewdef('composite_v', true);
+drop view composite_v;
 
 --
 -- Tests for component access / FieldSelect