This is an automated email from the ASF dual-hosted git repository. tuhaihe pushed a commit to branch REL_2_STABLE in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit b7bb2d493fe5a61af6113dac16e11c43a15493ab Author: Tom Lane <[email protected]> AuthorDate: Fri Sep 15 17:01:26 2023 -0400 Track nesting depth correctly when drilling down into RECORD Vars. 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/[email protected] --- src/backend/parser/parse_target.c | 20 ++++++++---- src/backend/utils/adt/ruleutils.c | 37 ++++++++++++--------- src/test/regress/expected/rowtypes.out | 60 ++++++++++++++++++++++++++++++++++ src/test/regress/sql/rowtypes.sql | 25 ++++++++++++++ 4 files changed, 120 insertions(+), 22 deletions(-) diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 039952aa8ae..274b38d2643 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1507,7 +1507,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) @@ -1595,11 +1596,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; + ParseState mypstate = {0}; + Index levelsup; - MemSet(&mypstate, 0, sizeof(mypstate)); + /* 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 */ @@ -1651,12 +1658,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; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 186cb8fed16..89d4d91c6c1 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -7727,22 +7727,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; } @@ -7835,7 +7841,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)) @@ -7843,21 +7849,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); diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index 03dc2ab3a79..2c85583cce0 100644 --- a/src/test/regress/expected/rowtypes.out +++ b/src/test/regress/expected/rowtypes.out @@ -1234,6 +1234,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 -- diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql index 54f338af1f8..50e0ca6cd6b 100644 --- a/src/test/regress/sql/rowtypes.sql +++ b/src/test/regress/sql/rowtypes.sql @@ -501,6 +501,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 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
