From 2a15ed35db5f6e3e844d6f62e8a0b5dbb58ee6d2 Mon Sep 17 00:00:00 2001 From: Andrey Rachitskiy Date: Thu, 27 Aug 2026 19:47:46 +0500 Subject: [PATCH] Fix field expansion of anonymous RECORD via scalar SubLink Drill-down for RECORD field expansion already followed Var relays through subquery and CTE RTEs, but when the defining expression was a scalar SubLink the code fell through to get_expr_result_tupdesc(), which cannot resolve anonymous RECORD. That made (c).f1 / (c).* / f1(c) fail with "record type has not been registered" after a (SELECT ...) relay of ROW(), while the same value still worked with row_to_json() and casts. Generalize that path as expandRecordExpr(Node *) so it also peels EXPR_SUBLINK (with a nested ParseState), and peel the same SubLinks inside get_name_for_var_field() for deparsing. Bug: #19498 Author: Andrey Rachitskiy Reported-by: muyehu Discussion: https://www.postgresql.org/message-id/19498-e49069f1ed6487cd%40postgresql.org --- src/backend/parser/parse_func.c | 8 +- src/backend/parser/parse_target.c | 154 ++++++++++++++----------- src/backend/utils/adt/ruleutils.c | 138 ++++++++++++---------- src/include/parser/parse_target.h | 4 +- src/test/regress/expected/rowtypes.out | 66 +++++++++++ src/test/regress/sql/rowtypes.sql | 40 +++++++ 6 files changed, 279 insertions(+), 131 deletions(-) diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index c87804f5d41..6d2378cea28 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -2109,8 +2109,8 @@ ParseComplexProjection(ParseState *pstate, const char *funcname, Node *first_arg * result can omit the whole-row Var and just be a Var for the selected * field. * - * This case could be handled by expandRecordVariable, but it's more - * efficient to do it this way when possible. + * This case could be handled by expandRecordExpr, but it's more efficient + * to do it this way when possible. */ if (IsA(first_arg, Var) && ((Var *) first_arg)->varattno == InvalidAttrNumber) @@ -2129,11 +2129,11 @@ ParseComplexProjection(ParseState *pstate, const char *funcname, Node *first_arg * * If it's a Var of type RECORD, we have to work even harder: we have to * find what the Var refers to, and pass that to get_expr_result_tupdesc. - * That task is handled by expandRecordVariable(). + * That task is handled by expandRecordExpr(). */ if (IsA(first_arg, Var) && ((Var *) first_arg)->vartype == RECORDOID) - tupdesc = expandRecordVariable(pstate, (Var *) first_arg, 0); + tupdesc = expandRecordExpr(pstate, first_arg, 0); else tupdesc = get_expr_result_tupdesc(first_arg, true); if (!tupdesc) diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 0ea10f8e882..68acb660791 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -1461,18 +1461,15 @@ ExpandRowReference(ParseState *pstate, Node *expr, * (This can be pretty inefficient if the expression involves nontrivial * computation :-(.) * - * Verify it's a composite type, and get the tupdesc. - * get_expr_result_tupdesc() handles this conveniently. + * Verify it's a composite type, and get the tupdesc. That task is handled + * by expandRecordExpr(). * * If it's a Var of type RECORD, we have to work even harder: we have to - * find what the Var refers to, and pass that to get_expr_result_tupdesc. - * That task is handled by expandRecordVariable(). + * find what the Var refers to. We also look through scalar SubLinks that + * relay anonymous RECORD. Other cases still go through + * get_expr_result_tupdesc() underneath. */ - if (IsA(expr, Var) && - ((Var *) expr)->vartype == RECORDOID) - tupleDesc = expandRecordVariable(pstate, (Var *) expr, 0); - else - tupleDesc = get_expr_result_tupdesc(expr, false); + tupleDesc = expandRecordExpr(pstate, expr, 0); Assert(tupleDesc); /* Generate a list of references to the individual fields */ @@ -1512,29 +1509,61 @@ ExpandRowReference(ParseState *pstate, Node *expr, } /* - * expandRecordVariable - * Get the tuple descriptor for a Var of type RECORD, if possible. + * expandRecordExpr + * Get the tuple descriptor for a composite-valued expression. * - * Since no actual table or view column is allowed to have type RECORD, such - * a Var must refer to a JOIN or FUNCTION RTE or to a subquery output. We - * 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. + * Looks through scalar SubLinks (EXPR_SUBLINK) so that an anonymous RECORD + * value relayed as (SELECT ...) can be resolved from the subquery's output + * expression. For a Var of type RECORD, drills down through JOIN / subquery + * / CTE RTEs to the defining expression (no table or view column may have + * type RECORD). Other composites fall through to get_expr_result_tupdesc(). + * We ereport if we can't determine the tupdesc. * * 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) +expandRecordExpr(ParseState *pstate, Node *expr, int levelsup) { TupleDesc tupleDesc; int netlevelsup; RangeTblEntry *rte; AttrNumber attnum; - Node *expr; + Var *var; + + /* Look through scalar SubLinks. */ + if (expr && IsA(expr, SubLink) && + ((SubLink *) expr)->subLinkType == EXPR_SUBLINK) + { + SubLink *sublink = (SubLink *) expr; + Query *subselect = castNode(Query, sublink->subselect); + TargetEntry *ste; + ParseState subpstate = {0}; + + /* EXPR_SUBLINK has a single non-junk output column. */ + ste = get_tle_by_resno(subselect->targetList, 1); + if (ste == NULL || ste->resjunk) + elog(ERROR, "EXPR_SUBLINK subquery has no non-junk output column"); - /* Check my caller didn't mess up */ - Assert(IsA(var, Var)); - Assert(var->vartype == RECORDOID); + /* + * Recurse with a ParseState for the SubLink subquery, so that Vars + * (and nested SubLinks) inside it are interpreted correctly. + */ + subpstate.parentParseState = pstate; + subpstate.p_rtable = subselect->rtable; + + return expandRecordExpr(&subpstate, (Node *) ste->expr, 0); + } + + /* + * If it's not a Var of type RECORD, get_expr_result_tupdesc can handle + * named composites, RowExpr, etc. + */ + if (!expr || !IsA(expr, Var) || + ((Var *) expr)->vartype != RECORDOID) + return get_expr_result_tupdesc(expr, false); + + var = (Var *) expr; /* * Note: it's tempting to use GetNSItemByRangeTablePosn here so that we @@ -1607,28 +1636,25 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup) elog(ERROR, "subquery %s does not have attribute %d", rte->eref->aliasname, attnum); expr = (Node *) ste->expr; - if (IsA(expr, Var)) - { - /* - * 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; - * 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}; - /* this loop must work, since GetRTEByRangeTablePosn did */ - for (int level = 0; level < netlevelsup; level++) - pstate = pstate->parentParseState; - mypstate.parentParseState = pstate; - mypstate.p_rtable = rte->subquery->rtable; - /* don't bother filling the rest of the fake pstate */ + /* + * Recurse into the sub-select on its output expression. + * We have to build an additional level of ParseState 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}; - return expandRecordVariable(&mypstate, (Var *) expr, 0); - } - /* else fall through to inspect the expression */ + /* this loop must work, since GetRTEByRangeTablePosn did */ + for (int level = 0; level < netlevelsup; level++) + pstate = pstate->parentParseState; + mypstate.parentParseState = pstate; + mypstate.p_rtable = rte->subquery->rtable; + /* don't bother filling the rest of the fake pstate */ + + return expandRecordExpr(&mypstate, expr, 0); } break; case RTE_JOIN: @@ -1636,11 +1662,8 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup) Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars)); expr = (Node *) list_nth(rte->joinaliasvars, attnum - 1); Assert(expr != NULL); - /* We intentionally don't strip implicit coercions here */ - if (IsA(expr, Var)) - return expandRecordVariable(pstate, (Var *) expr, netlevelsup); - /* else fall through to inspect the expression */ - break; + /* may be a coerced Var or COALESCE, leave it wrapped */ + return expandRecordExpr(pstate, expr, netlevelsup); case RTE_FUNCTION: /* @@ -1666,28 +1689,25 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup) elog(ERROR, "CTE %s does not have attribute %d", rte->eref->aliasname, attnum); expr = (Node *) ste->expr; - if (IsA(expr, Var)) - { - /* - * 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 (compare SUBQUERY case above). - */ - ParseState mypstate = {0}; - - /* this loop must work, since GetCTEForRTE did */ - for (Index level = 0; - level < rte->ctelevelsup + netlevelsup; - level++) - pstate = pstate->parentParseState; - mypstate.parentParseState = pstate; - mypstate.p_rtable = ((Query *) cte->ctequery)->rtable; - /* don't bother filling the rest of the fake pstate */ - - return expandRecordVariable(&mypstate, (Var *) expr, 0); - } - /* else fall through to inspect the expression */ + + /* + * Recurse into the CTE on its output expression. 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 (compare SUBQUERY case above). + */ + ParseState mypstate = {0}; + + /* this loop must work, since GetCTEForRTE did */ + for (Index level = 0; + level < rte->ctelevelsup + netlevelsup; + level++) + pstate = pstate->parentParseState; + mypstate.parentParseState = pstate; + mypstate.p_rtable = ((Query *) cte->ctequery)->rtable; + /* don't bother filling the rest of the fake pstate */ + + return expandRecordExpr(&mypstate, expr, 0); } break; case RTE_GROUP: diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 6506bf12e3c..9b6f431a491 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -8568,6 +8568,9 @@ resolve_special_varno(Node *node, deparse_context *context, * * Similarly, a PARAM of type RECORD has to refer to some expression of * a determinable composite type. + * + * We also look through scalar SubLinks (EXPR_SUBLINK) that relay anonymous + * RECORD, paralleling expandRecordExpr() in the parser. */ static const char * get_name_for_var_field(Var *var, int fieldno, @@ -8618,6 +8621,35 @@ get_name_for_var_field(Var *var, int fieldno, } } + /* Look through scalar SubLinks (see expandRecordExpr). */ + if (IsA(var, SubLink) && + ((SubLink *) var)->subLinkType == EXPR_SUBLINK) + { + SubLink *sublink = (SubLink *) var; + Query *subselect = castNode(Query, sublink->subselect); + TargetEntry *ste; + List *save_nslist = context->namespaces; + deparse_namespace mydpns; + const char *result; + + /* EXPR_SUBLINK has a single non-junk output column. */ + ste = get_tle_by_resno(subselect->targetList, 1); + if (ste == NULL || ste->resjunk) + elog(ERROR, "EXPR_SUBLINK subquery has no non-junk output column"); + + /* + * Recurse with a deparse namespace for the SubLink subquery, so that + * Vars (and nested SubLinks) inside it are interpreted correctly. + */ + set_deparse_for_query(&mydpns, subselect, context->namespaces); + context->namespaces = lcons(&mydpns, context->namespaces); + + result = get_name_for_var_field((Var *) ste->expr, fieldno, 0, context); + + context->namespaces = save_nslist; + return result; + } + /* * If it's a Var of type RECORD, we have to find what the Var refers to; * if not, we can use get_expr_result_tupdesc(). @@ -8737,7 +8769,7 @@ get_name_for_var_field(Var *var, int fieldno, /* * This part has essentially the same logic as the parser's - * expandRecordVariable() function, but we are dealing with a different + * expandRecordExpr() function, but we are dealing with a different * representation of the input context, and we only need one field name * not a TupleDesc. Also, we need special cases for finding subquery and * CTE subplans when deparsing Plan trees. @@ -8770,38 +8802,35 @@ get_name_for_var_field(Var *var, int fieldno, elog(ERROR, "subquery %s does not have attribute %d", rte->eref->aliasname, attnum); expr = (Node *) ste->expr; - if (IsA(expr, Var)) - { - /* - * 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; 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); + /* + * Recurse into the sub-select on its output expression. + * We have to build an additional level of namespace to + * keep in step with varlevelsup in the 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, - parent_namespaces); + set_deparse_for_query(&mydpns, rte->subquery, + parent_namespaces); - context->namespaces = lcons(&mydpns, parent_namespaces); + context->namespaces = lcons(&mydpns, parent_namespaces); - result = get_name_for_var_field((Var *) expr, fieldno, - 0, context); + result = get_name_for_var_field((Var *) expr, fieldno, + 0, context); - context->namespaces = save_nslist; + context->namespaces = save_nslist; - return result; - } - /* else fall through to inspect the expression */ + return result; } else { @@ -8855,13 +8884,10 @@ get_name_for_var_field(Var *var, int fieldno, Assert(attnum > 0 && attnum <= list_length(rte->joinaliasvars)); expr = (Node *) list_nth(rte->joinaliasvars, attnum - 1); Assert(expr != NULL); - /* we intentionally don't strip implicit coercions here */ - if (IsA(expr, Var)) - return get_name_for_var_field((Var *) expr, fieldno, - var->varlevelsup + levelsup, - context); - /* else fall through to inspect the expression */ - break; + /* may be a coerced Var or COALESCE, leave it wrapped */ + return get_name_for_var_field((Var *) expr, fieldno, + var->varlevelsup + levelsup, + context); case RTE_FUNCTION: case RTE_TABLEFUNC: @@ -8906,36 +8932,32 @@ get_name_for_var_field(Var *var, int fieldno, elog(ERROR, "CTE %s does not have attribute %d", rte->eref->aliasname, attnum); expr = (Node *) ste->expr; - if (IsA(expr, Var)) - { - /* - * 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 (compare - * SUBQUERY case above). - */ - List *save_nslist = context->namespaces; - List *parent_namespaces; - deparse_namespace mydpns; - const char *result; - parent_namespaces = list_copy_tail(context->namespaces, - ctelevelsup); + /* + * Recurse into the CTE on its output expression. 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 (compare SUBQUERY case above). + */ + List *save_nslist = context->namespaces; + List *parent_namespaces; + deparse_namespace mydpns; + const char *result; + + parent_namespaces = list_copy_tail(context->namespaces, + ctelevelsup); - set_deparse_for_query(&mydpns, ctequery, - parent_namespaces); + set_deparse_for_query(&mydpns, ctequery, + parent_namespaces); - context->namespaces = lcons(&mydpns, parent_namespaces); + context->namespaces = lcons(&mydpns, parent_namespaces); - result = get_name_for_var_field((Var *) expr, fieldno, - 0, context); + result = get_name_for_var_field((Var *) expr, fieldno, + 0, context); - context->namespaces = save_nslist; + context->namespaces = save_nslist; - return result; - } - /* else fall through to inspect the expression */ + return result; } else { diff --git a/src/include/parser/parse_target.h b/src/include/parser/parse_target.h index 3af662f84b4..78304329d53 100644 --- a/src/include/parser/parse_target.h +++ b/src/include/parser/parse_target.h @@ -50,8 +50,8 @@ extern Node *transformAssignmentIndirection(ParseState *pstate, int location); extern List *checkInsertTargets(ParseState *pstate, List *cols, List **attrnos); -extern TupleDesc expandRecordVariable(ParseState *pstate, Var *var, - int levelsup); +extern TupleDesc expandRecordExpr(ParseState *pstate, Node *expr, + int levelsup); extern char *FigureColname(Node *node); #endif /* PARSE_TARGET_H */ diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index 956bc2d02fc..2a1a7928cfa 100644 --- a/src/test/regress/expected/rowtypes.out +++ b/src/test/regress/expected/rowtypes.out @@ -1313,6 +1313,72 @@ select pg_get_viewdef('composite_v', true); drop view composite_v; -- +-- Check field expansion after scalar-subquery relay of anonymous RECORD +-- (bug #19498) +-- +select (c).f1 +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; + f1 +---- + 1 +(1 row) + +select (c).* +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; + f1 | f2 +----+---- + 1 | 2 +(1 row) + +select f1(c) +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; + f1 +---- + 1 +(1 row) + +-- Same via CTE, but re-exported through a scalar subquery. +with cte(c) as materialized (select row(1, 2)), + cte2(c) as (select (select c from cte) as c) +select (c).f1 +from cte2; + f1 +---- + 1 +(1 row) + +-- Nested scalar subquery relay. +select (c).f1 +from ( + select (select (select z.c from (select row(1, 2) as c) z)) as c +) s; + f1 +---- + 1 +(1 row) + +-- Deparse of field expansion over a SubLink relay. +create view composite_sublink_v as +select (c).f1 as f1 +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; +select pg_get_viewdef('composite_sublink_v', true); + pg_get_viewdef +-------------------------------------------------------------- + SELECT (c).f1 AS f1 + + FROM ( SELECT ( SELECT z.c + + FROM ( SELECT ROW(1, 2) AS c) z) AS c) s; +(1 row) + +drop view composite_sublink_v; +-- -- Check cases where the composite comes from a proven-dummy rel (bug #18576) -- explain (verbose, costs off) diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql index 174b062144a..a45e9937ebf 100644 --- a/src/test/regress/sql/rowtypes.sql +++ b/src/test/regress/sql/rowtypes.sql @@ -520,6 +520,46 @@ where (select * from (select c as c1) s select pg_get_viewdef('composite_v', true); drop view composite_v; +-- +-- Check field expansion after scalar-subquery relay of anonymous RECORD +-- (bug #19498) +-- +select (c).f1 +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; + +select (c).* +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; + +select f1(c) +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; + +-- Same via CTE, but re-exported through a scalar subquery. +with cte(c) as materialized (select row(1, 2)), + cte2(c) as (select (select c from cte) as c) +select (c).f1 +from cte2; + +-- Nested scalar subquery relay. +select (c).f1 +from ( + select (select (select z.c from (select row(1, 2) as c) z)) as c +) s; + +-- Deparse of field expansion over a SubLink relay. +create view composite_sublink_v as +select (c).f1 as f1 +from ( + select (select z.c from (select row(1, 2) as c) z) as c +) s; +select pg_get_viewdef('composite_sublink_v', true); +drop view composite_sublink_v; + -- -- Check cases where the composite comes from a proven-dummy rel (bug #18576) -- -- 2.53.0