From a4cefcfe13b63ff7ba4529875d525f31dfe61699 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Thu, 22 Oct 2026 14:17:53 +0900 Subject: [PATCH 3/6] Remove the fix for columns of relations not printed in FROM This series carried its own fix for the deparse of the columns of a relation that is not printed in the FROM clause, such as NEW and OLD of a rule, which is not specific to row pattern recognition. It goes upstream as a patch of its own. Remove the fix here, the tests for it in rules, and its mention in README.rpr. mark_define_column() keeps the assertion that colname_is_fixed() made. That patch also changes the comment in colname_is_unique() on the names checked against dpns->using_names, which this series had reworded to mention the names DEFINE clauses read. Put the upstream wording back, and say that in an XXX note above it instead, so that the patch applies without conflict. The note carries the comment to replace the upstream one with once the patch is in. --- src/backend/executor/README.rpr | 3 +- src/backend/utils/adt/ruleutils.c | 44 ++++++++--------------- src/test/regress/expected/rules.out | 54 ----------------------------- src/test/regress/sql/rules.sql | 36 ------------------- 4 files changed, 16 insertions(+), 121 deletions(-) diff --git a/src/backend/executor/README.rpr b/src/backend/executor/README.rpr index afe366fa40d..a3695cf2091 100644 --- a/src/backend/executor/README.rpr +++ b/src/backend/executor/README.rpr @@ -2553,8 +2553,7 @@ every query, with a DEFINE clause or not. For a function RTE it appends the columns the composite result type has gained since parse time (function_rte_late_colnames(); only for a single function without WITH ORDINALITY or a column definition list), so a reserved name is kept off -them and the positional column alias list counts them. The columns -colname_is_fixed() reports are never renamed there. +them and the positional column alias list counts them. XIII-6. Navigation Names Shadow User Functions diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index d411d79c6ae..0cb462a585e 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -396,7 +396,6 @@ static void set_simple_column_names(deparse_namespace *dpns); static bool has_dangerous_join_using(deparse_namespace *dpns, Node *jtnode); static void set_using_names(deparse_namespace *dpns, Node *jtnode, List *parentUsing); -static bool colname_is_fixed(RangeTblEntry *rte); static void mark_define_columns(deparse_namespace *dpns, Query *query); static void mark_define_column(deparse_namespace *dpns, Var *var); static char *preset_input_colname(deparse_namespace *dpns, int varno, @@ -4555,21 +4554,6 @@ set_using_names(deparse_namespace *dpns, Node *jtnode, List *parentUsing) (int) nodeTag(jtnode)); } -/* - * colname_is_fixed: is this a column that no rename can reach? - * - * A relation RTE outside the FROM clause -- a rule's NEW or OLD, or the - * target of an UPDATE or DELETE -- has nowhere to print a column alias list, - * so a renamed column of one would reach the output only where it is - * referenced, naming a column that does not exist. - * set_relation_column_names() leaves such columns alone. - */ -static bool -colname_is_fixed(RangeTblEntry *rte) -{ - return rte->rtekind == RTE_RELATION && !rte->inFromCl; -} - /* * mark_define_columns: settle the names of the columns DEFINE clauses read * @@ -4680,7 +4664,7 @@ mark_define_column(deparse_namespace *dpns, Var *var) * cannot name a column of a relation outside the FROM clause: the * qualifier slot is taken by the pattern variables. */ - Assert(!colname_is_fixed(rte)); + Assert(rte->rtekind != RTE_RELATION || rte->inFromCl); /* * Choose the name unique within the RTE and against what is reserved, @@ -4972,17 +4956,8 @@ set_relation_column_names(deparse_namespace *dpns, RangeTblEntry *rte, else colname = real_colname; - /* - * Unique-ify and insert into colinfo, unless this is a column no - * rename can reach: a relation RTE outside the FROM clause has - * nowhere to carry a column alias list, so a renamed column would - * reach the output only where it is referenced, naming a column - * that does not exist. Other kinds reach here with inFromCl - * clear and still get printed, the subquery an INSERT ... SELECT - * reads from among them, so those are renamed as before. - */ - if (!colname_is_fixed(rte)) - colname = make_colname_unique(colname, dpns, colinfo); + /* Unique-ify and insert into colinfo */ + colname = make_colname_unique(colname, dpns, colinfo); colinfo->colnames[i] = colname; add_to_names_hash(colinfo, colname); @@ -5367,9 +5342,20 @@ colname_is_unique(const char *colname, deparse_namespace *dpns, } /* + * XXX dpns->using_names also holds the names DEFINE clauses read (see + * reserve_colname()). Once the fix that keeps the real column names of + * an RTE not printed in FROM is in, replace the comment below with: + * * Also check against the names reserved across the query level: USING * column names that must be globally unique, and the names DEFINE clauses - * read. These are not hashed, as they are expected to be few. + * read. These are not hashed, as they are expected to be few. Skip this + * check for an RTE that is not printed in FROM: it cannot have a column + * alias list, so its column names must stay the real ones. + */ + + /* + * Also check against USING-column names that must be globally unique. + * These are not hashed, but there should be few of them. */ foreach(lc, dpns->using_names) { diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index ba9139cce4f..acbe4667d89 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -3783,60 +3783,6 @@ INSERT INTO hats VALUES ('h7', 'black') RETURNING *; DROP RULE hat_confsel ON hats; drop table hats; drop table hat_data; --- An unnamed FULL JOIN USING makes USING names unique query-wide, but an RTE --- outside the FROM clause has no alias list to carry a renamed column. -create table rule_uniq1 (x int, y int); -create table rule_uniq2 (x int, z int); -create table rule_uniq3 (x int, w int); -create table rule_uniq_log (x int); -create rule rule_uniq_r as on update to rule_uniq1 do also - insert into rule_uniq_log - select g1.y from rule_uniq1 g1, rule_uniq2 full join rule_uniq3 using (x) - where g1.x <> new.x; -create rule rule_uniq_u as on update to rule_uniq1 do also - update rule_uniq_log set x = 1 - from rule_uniq2 full join rule_uniq3 using (x) - where rule_uniq_log.x = 0; --- g1 is in the FROM clause and is re-aliased to x_1; new and the update --- target are not, so they keep x -select rulename, definition from pg_rules where tablename = 'rule_uniq1' - order by rulename; - rulename | definition --------------+----------------------------------------------------------------------------------- - rule_uniq_r | CREATE RULE rule_uniq_r AS + - | ON UPDATE TO public.rule_uniq1 DO INSERT INTO rule_uniq_log (x) SELECT g1.y+ - | FROM rule_uniq1 g1(x_1, y), + - | (rule_uniq2 + - | FULL JOIN rule_uniq3 USING (x)) + - | WHERE (g1.x_1 <> new.x); - rule_uniq_u | CREATE RULE rule_uniq_u AS + - | ON UPDATE TO public.rule_uniq1 DO UPDATE rule_uniq_log SET x = 1 + - | FROM (rule_uniq2 + - | FULL JOIN rule_uniq3 USING (x)) + - | WHERE (rule_uniq_log.x = 0); -(2 rows) - -drop table rule_uniq1, rule_uniq2, rule_uniq3, rule_uniq_log; --- The subquery an INSERT ... SELECT reads from is outside the FROM clause --- too, but it is printed, so its unnamed columns still need unique names. --- LIMIT keeps the subquery from being pulled up, and the unfilled column --- makes the plan project through a subquery scan. -create table rule_uniq_ins (a int, b int, c text, d int); -explain (verbose, costs off) - insert into rule_uniq_ins (a, b, c) - select a + 1, b + 1, c || c from rule_uniq_ins limit 2; - QUERY PLAN --------------------------------------------------------------------------------------------------------------------------- - Insert on public.rule_uniq_ins - -> Subquery Scan on unnamed_subquery - Output: unnamed_subquery."?column?", unnamed_subquery."?column?_1", unnamed_subquery."?column?_2", NULL::integer - -> Limit - Output: ((rule_uniq_ins_1.a + 1)), ((rule_uniq_ins_1.b + 1)), ((rule_uniq_ins_1.c || rule_uniq_ins_1.c)) - -> Seq Scan on public.rule_uniq_ins rule_uniq_ins_1 - Output: (rule_uniq_ins_1.a + 1), (rule_uniq_ins_1.b + 1), (rule_uniq_ins_1.c || rule_uniq_ins_1.c) -(7 rows) - -drop table rule_uniq_ins; -- test for pg_get_functiondef properly regurgitating SET parameters -- Note that the function is kept around to stress pg_dump. CREATE FUNCTION func_with_set_params() RETURNS integer diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index 71b7b001e9c..abe89d097c9 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -1235,42 +1235,6 @@ DROP RULE hat_confsel ON hats; drop table hats; drop table hat_data; --- An unnamed FULL JOIN USING makes USING names unique query-wide, but an RTE --- outside the FROM clause has no alias list to carry a renamed column. -create table rule_uniq1 (x int, y int); -create table rule_uniq2 (x int, z int); -create table rule_uniq3 (x int, w int); -create table rule_uniq_log (x int); - -create rule rule_uniq_r as on update to rule_uniq1 do also - insert into rule_uniq_log - select g1.y from rule_uniq1 g1, rule_uniq2 full join rule_uniq3 using (x) - where g1.x <> new.x; - -create rule rule_uniq_u as on update to rule_uniq1 do also - update rule_uniq_log set x = 1 - from rule_uniq2 full join rule_uniq3 using (x) - where rule_uniq_log.x = 0; - --- g1 is in the FROM clause and is re-aliased to x_1; new and the update --- target are not, so they keep x -select rulename, definition from pg_rules where tablename = 'rule_uniq1' - order by rulename; - -drop table rule_uniq1, rule_uniq2, rule_uniq3, rule_uniq_log; - --- The subquery an INSERT ... SELECT reads from is outside the FROM clause --- too, but it is printed, so its unnamed columns still need unique names. --- LIMIT keeps the subquery from being pulled up, and the unfilled column --- makes the plan project through a subquery scan. -create table rule_uniq_ins (a int, b int, c text, d int); - -explain (verbose, costs off) - insert into rule_uniq_ins (a, b, c) - select a + 1, b + 1, c || c from rule_uniq_ins limit 2; - -drop table rule_uniq_ins; - -- test for pg_get_functiondef properly regurgitating SET parameters -- Note that the function is kept around to stress pg_dump. CREATE FUNCTION func_with_set_params() RETURNS integer -- 2.54.0 (Apple Git-157)