From 120df5be62b581308f22c2a1f60d51368978f1f3 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Fri, 30 Oct 2026 12:03:08 +0900 Subject: [PATCH v1 2/2] Fix deparse of parameter references in SQL-function bodies get_parameter() printed a reference to a parameter of an SQL-standard function body as funcname.argname when the query has relations, and as the bare argname otherwise. Neither form was always read back as the same parameter. The parser resolves funcname.argname as a column reference first, and sql_fn_post_column_ref never overrides that. So whenever a name that is visible as a relation qualifier equals the function name, the qualified name can be captured: by a column of that relation, including a system column or one added after the function was created, or by a function taking the whole row, as in argname(funcname.*). Besides the names of range table entries, a JOIN/USING alias is such a name. Print $N in that case, looking only at the names. Without relations, a bare parameter name used as an ORDER BY, DISTINCT ON or GROUP BY key is matched by findTargetlistEntrySQL92 against the output column names first, so it is captured whenever an output column has that name. Print $N there too. (On master a query with GROUP BY always has an RTE_GROUP entry, so only the back branches reach this for GROUP BY.) Unnamed parameters are stored as empty strings, not NULLs, and were printed as an empty identifier. Treat an empty name as no name. This changes the displayed body of ts_debug(text). The function in its FROM clause carries the default alias ts_debug, equal to the function name, so the parameter is now shown as $1 instead of ts_debug.document. That reference was not actually ambiguous, since a function's arguments cannot see the alias of the function's own range table entry, but the check does not try to distinguish that case. $1 is also what the function's source uses. Backpatch-through: 15 --- src/backend/utils/adt/ruleutils.c | 87 +++++++++++++++++-- .../regress/expected/create_function_sql.out | 55 +++++------- src/test/regress/expected/psql.out | 4 +- 3 files changed, 104 insertions(+), 42 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 7e78740e9d8..0cd9d834d99 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -123,6 +123,7 @@ typedef struct bool varprefix; /* true to print prefixes on Vars */ ParseExprKind special_exprkind; /* set only for exprkinds needing special * handling */ + bool varInOrderBy; /* deparsing simple Param in ORDER BY? */ Bitmapset *appendparents; /* if not null, map child Vars of these relids * back to the parent rel */ } deparse_context; @@ -438,6 +439,8 @@ static void resolve_special_varno(Node *node, deparse_context *context, rsv_callback callback, void *callback_arg); static Node *find_param_referent(Param *param, deparse_context *context, deparse_namespace **dpns_p, ListCell **ancestor_cell_p); +static bool funcname_is_visible_refname(List *namespaces, + const char *funcname); static void get_parameter(Param *param, deparse_context *context); static const char *get_simple_binary_op_name(OpExpr *expr); static bool isSimpleNode(Node *node, Node *parentNode, int prettyFlags); @@ -1072,6 +1075,7 @@ pg_get_triggerdef_worker(Oid trigid, bool pretty) context.wrapColumn = WRAP_COLUMN_DEFAULT; context.indentLevel = PRETTYINDENT_STD; context.special_exprkind = EXPR_KIND_NONE; + context.varInOrderBy = false; context.appendparents = NULL; get_rule_expr(qual, &context, false); @@ -3660,6 +3664,7 @@ deparse_expression_pretty(Node *expr, List *dpcontext, context.wrapColumn = WRAP_COLUMN_DEFAULT; context.indentLevel = startIndent; context.special_exprkind = EXPR_KIND_NONE; + context.varInOrderBy = false; context.appendparents = NULL; get_rule_expr(expr, &context, showimplicit); @@ -5316,6 +5321,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, context.wrapColumn = WRAP_COLUMN_DEFAULT; context.indentLevel = PRETTYINDENT_STD; context.special_exprkind = EXPR_KIND_NONE; + context.varInOrderBy = false; context.appendparents = NULL; set_deparse_for_query(&dpns, query, NIL); @@ -5485,6 +5491,7 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace, context.wrapColumn = wrapColumn; context.indentLevel = startIndent; context.special_exprkind = EXPR_KIND_NONE; + context.varInOrderBy = false; context.appendparents = NULL; set_deparse_for_query(&dpns, query, parentnamespace); @@ -6362,6 +6369,15 @@ get_rule_sortgroupclause(Index ref, List *tlist, bool force_colno, get_const_expr((Const *) expr, context, 1); else if (!expr || IsA(expr, Var)) get_rule_expr(expr, context, true); + else if (IsA(expr, Param) && ((Param *) expr)->paramkind == PARAM_EXTERN) + { + /* Tell get_parameter not to print a bare parameter name */ + bool save_varinorderby = context->varInOrderBy; + + context->varInOrderBy = true; + get_parameter((Param *) expr, context); + context->varInOrderBy = save_varinorderby; + } else { /* @@ -8227,6 +8243,42 @@ find_param_referent(Param *param, deparse_context *context, return NULL; } +/* + * Is a name that the parser could take as a relation qualifier equal to the + * name of the SQL function whose body is being deparsed? + * + * If so, "funcname.argname" might not be read back as the parameter: it can + * be captured by a column of that relation, including a system column or one + * added later, or by a function taking the relation's whole row + * ("argname(funcname.*)"). Look only at the names, not at the columns. + * Besides the RTE names, a JOIN/USING alias is visible as a qualifier. + */ +static bool +funcname_is_visible_refname(List *namespaces, const char *funcname) +{ + ListCell *lc1; + + foreach(lc1, namespaces) + { + deparse_namespace *depns = (deparse_namespace *) lfirst(lc1); + ListCell *lc; + int rtindex = 0; + + foreach(lc, depns->rtable_names) + { + const char *rname = lfirst(lc); + RangeTblEntry *rte = rt_fetch(++rtindex, depns->rtable); + + if (rname && strcmp(rname, funcname) == 0) + return true; + if (rte->rtekind == RTE_JOIN && rte->join_using_alias && + strcmp(rte->join_using_alias->aliasname, funcname) == 0) + return true; + } + } + return false; +} + /* * Display a Param appropriately. */ @@ -8298,16 +8350,19 @@ get_parameter(Param *param, deparse_context *context) { char *argname = dpns->argnames[param->paramid - 1]; - if (argname) + /* unnamed arguments are stored as empty strings, not NULLs */ + if (argname && argname[0] != '\0') { bool should_qualify = false; ListCell *lc; /* - * Qualify the parameter name if there are any other deparse - * namespaces with range tables. This avoids qualifying in - * trivial cases like "RETURN a + b", but makes it safe in all - * other cases. + * With no range table at any query level, the bare name + * cannot be taken as a column, so prefer it for readability + * in cases like "RETURN a + b". Otherwise qualify it with + * the function name, unless that name could itself be taken + * as a relation qualifier, in which case only $N is + * unambiguous. */ foreach(lc, context->namespaces) { @@ -8319,14 +8374,28 @@ get_parameter(Param *param, deparse_context *context) break; } } - if (should_qualify) + + if (!should_qualify) + { + /* + * A bare name used as a sort or group key would be taken + * by findTargetlistEntrySQL92 as an output-column name, + * so print $N there. + */ + if (!context->varInOrderBy) + { + appendStringInfoString(context->buf, quote_identifier(argname)); + return; + } + } + else if (!funcname_is_visible_refname(context->namespaces, + dpns->funcname)) { appendStringInfoString(context->buf, quote_identifier(dpns->funcname)); appendStringInfoChar(context->buf, '.'); + appendStringInfoString(context->buf, quote_identifier(argname)); + return; } - - appendStringInfoString(context->buf, quote_identifier(argname)); - return; } } } diff --git a/src/test/regress/expected/create_function_sql.out b/src/test/regress/expected/create_function_sql.out index 541d0ebd24c..73d6e497fd5 100644 --- a/src/test/regress/expected/create_function_sql.out +++ b/src/test/regress/expected/create_function_sql.out @@ -722,7 +722,7 @@ SELECT pg_get_functiondef('functest_unnamed'::regproc); RETURNS integer + LANGUAGE sql + BEGIN ATOMIC + - SELECT (a + ""); + + SELECT (a + $2); + END + (1 row) @@ -748,7 +748,7 @@ SELECT pg_get_functiondef('functest_cap'::regproc); RETURNS integer + LANGUAGE sql + BEGIN ATOMIC + - SELECT (functest_cap.a + functest_cap.x) + + SELECT ($1 + functest_cap.x) + FROM functest_cap; + END + @@ -759,7 +759,7 @@ SELECT pg_get_functiondef('functest_cap'::regproc) AS def \gset SELECT functest_cap(10); functest_cap -------------- - 105 + 15 (1 row) -- the same if a column is renamed to the name of a parameter that the body @@ -782,7 +782,7 @@ SELECT pg_get_functiondef('functest_ren'::regproc); RETURNS integer + LANGUAGE sql + BEGIN ATOMIC + - SELECT (functest_ren.a + functest_ren.a) + + SELECT ($1 + functest_ren.a) + FROM functest_ren; + END + @@ -793,7 +793,7 @@ SELECT pg_get_functiondef('functest_ren'::regproc) AS def \gset SELECT functest_ren(10); functest_ren -------------- - 10 + 15 (1 row) -- the same for any name the parser can take as a qualifier: a function @@ -815,7 +815,7 @@ SELECT pg_get_functiondef('functest_nn'::regproc); RETURNS integer + LANGUAGE sql + BEGIN ATOMIC + - SELECT (functest_nn.num_nulls + functest_nn.x) + + SELECT ($1 + functest_nn.x) + FROM functest_nn; + END + @@ -826,7 +826,7 @@ SELECT pg_get_functiondef('functest_nn'::regproc) AS def \gset SELECT functest_nn(7); functest_nn ------------- - 1 + 8 (1 row) -- a system column @@ -847,7 +847,7 @@ SELECT pg_get_functiondef('functest_sys'::regproc); RETURNS integer + LANGUAGE sql + BEGIN ATOMIC + - SELECT functest_sys.cmin + + SELECT $1 + FROM functest_sys; + END + @@ -855,9 +855,6 @@ SELECT pg_get_functiondef('functest_sys'::regproc); SELECT pg_get_functiondef('functest_sys'::regproc) AS def \gset :def ; -ERROR: return type mismatch in function declared to return integer -DETAIL: Actual return type is cid. -CONTEXT: SQL function "functest_sys" SELECT functest_sys(5); functest_sys -------------- @@ -885,7 +882,7 @@ SELECT pg_get_functiondef('functest_join'::regproc); RETURNS integer + LANGUAGE sql + BEGIN ATOMIC + - SELECT functest_join.a AS a + + SELECT $1 AS a + FROM (functest_j1 + CROSS JOIN functest_j2) functest_join; + END + @@ -897,7 +894,7 @@ SELECT pg_get_functiondef('functest_join'::regproc) AS def \gset SELECT functest_join(5); functest_join --------------- - 100 + 5 (1 row) -- a JOIN/USING alias @@ -920,7 +917,7 @@ SELECT pg_get_functiondef('functest_using'::regproc); RETURNS integer + LANGUAGE sql + BEGIN ATOMIC + - SELECT functest_using.a + + SELECT $1 + FROM (functest_u1 + JOIN functest_u2 USING (a) AS functest_using); + END + @@ -932,7 +929,7 @@ SELECT pg_get_functiondef('functest_using'::regproc) AS def \gset SELECT functest_using(5); functest_using ---------------- - 100 + 5 (1 row) -- without relations a parameter is printed by its bare name, but as a sort @@ -951,7 +948,7 @@ SELECT pg_get_functiondef('functest_sort'::regproc); CREATE OR REPLACE FUNCTION temp_func_test.functest_sort(a integer)+ RETURNS integer[] + LANGUAGE sql + - RETURN ARRAY(SELECT (4 - generate_series(1, 3)) AS a ORDER BY a) + + RETURN ARRAY(SELECT (4 - generate_series(1, 3)) AS a ORDER BY $1) + (1 row) @@ -960,7 +957,7 @@ SELECT pg_get_functiondef('functest_sort'::regproc) AS def \gset SELECT functest_sort(5); functest_sort --------------- - {1,2,3} + {3,2,1} (1 row) CREATE FUNCTION functest_distinct(a int) RETURNS int[] @@ -977,7 +974,7 @@ SELECT pg_get_functiondef('functest_distinct'::regproc); CREATE OR REPLACE FUNCTION temp_func_test.functest_distinct(a integer)+ RETURNS integer[] + LANGUAGE sql + - RETURN ARRAY(SELECT DISTINCT ON (a) generate_series(1, 3) AS a) + + RETURN ARRAY(SELECT DISTINCT ON ($1) generate_series(1, 3) AS a) + (1 row) @@ -986,7 +983,7 @@ SELECT pg_get_functiondef('functest_distinct'::regproc) AS def \gset SELECT functest_distinct(5); functest_distinct ------------------- - {1,2,3} + {1} (1 row) CREATE FUNCTION functest_group(a int) RETURNS int[] @@ -998,12 +995,12 @@ SELECT functest_group(5); (1 row) SELECT pg_get_functiondef('functest_group'::regproc); - pg_get_functiondef ------------------------------------------------------------------------- - CREATE OR REPLACE FUNCTION temp_func_test.functest_group(a integer) + - RETURNS integer[] + - LANGUAGE sql + - RETURN ARRAY(SELECT ((4 - generate_series(1, 3)) % 2) AS a GROUP BY a)+ + pg_get_functiondef +------------------------------------------------------------------------- + CREATE OR REPLACE FUNCTION temp_func_test.functest_group(a integer) + + RETURNS integer[] + + LANGUAGE sql + + RETURN ARRAY(SELECT ((4 - generate_series(1, 3)) % 2) AS a GROUP BY $1)+ (1 row) @@ -1012,7 +1009,7 @@ SELECT pg_get_functiondef('functest_group'::regproc) AS def \gset SELECT functest_group(5); functest_group ---------------- - {0,1} + {1,0,1} (1 row) -- the output column may have no ordering operator, so the body fails to @@ -1033,17 +1030,13 @@ SELECT pg_get_functiondef('functest_point'::regproc); LANGUAGE sql + BEGIN ATOMIC + SELECT point((1)::double precision, (2)::double precision) AS a + - ORDER BY a; + + ORDER BY $1; + END + (1 row) SELECT pg_get_functiondef('functest_point'::regproc) AS def \gset :def ; -ERROR: could not identify an ordering operator for type point -LINE 6: ORDER BY a; - ^ -HINT: Use an explicit ordering operator or modify the query. SELECT functest_point(5); functest_point ---------------- diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index cf3a7a0c29d..0473baceda6 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -5309,7 +5309,7 @@ BEGIN ATOMIC ts_debug.dictionaries, ts_debug.dictionary, ts_debug.lexemes - FROM ts_debug(get_current_ts_config(), ts_debug.document) ts_debug(alias, description, token, dictionaries, dictionary, lexemes); + FROM ts_debug(get_current_ts_config(), $1) ts_debug(alias, description, token, dictionaries, dictionary, lexemes); END \sf+ ts_debug(text) CREATE OR REPLACE FUNCTION pg_catalog.ts_debug(document text, OUT alias text, OUT description text, OUT token text, OUT dictionaries regdictionary[], OUT dictionary regdictionary, OUT lexemes text[]) @@ -5323,7 +5323,7 @@ END 5 ts_debug.dictionaries, 6 ts_debug.dictionary, 7 ts_debug.lexemes -8 FROM ts_debug(get_current_ts_config(), ts_debug.document) ts_debug(alias, description, token, dictionaries, dictionary, lexemes); +8 FROM ts_debug(get_current_ts_config(), $1) ts_debug(alias, description, token, dictionaries, dictionary, lexemes); 9 END -- AUTOCOMMIT CREATE TABLE ac_test (a int); -- 2.54.0 (Apple Git-157)