Re: The output sql generated by pg_dump for a create function refers to a modified table name

"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> I spoke too soon -- I was looking at the wrong logs. I did reproduce it 
> with UPDATE, but not INSERT.

It can be reproduced with INSERT too, on the same principle as the others:
put the DML command inside a WITH, and give it an alias conflicting with
the outer query.

Being a lazy sort, I tried to collapse all three cases into a single
test case, and observed something I hadn't thought of: we disambiguate
aliases in a WITH query with respect to the outer query, but not with
respect to other WITH queries.  This makes the example (see attached)
a bit more confusing than I would have hoped.  However, the same sort
of thing happens within other kinds of nested subqueries, so I think
it's probably all right as-is.  In any case, changing this aspect
would require a significantly bigger patch with more risk of unwanted
side-effects.

To fix it, I pulled out the print-an-alias logic within
get_from_clause_item and called that new function for
INSERT/UPDATE/DELETE.  This is a bit of overkill perhaps, because
only the RTE_RELATION case can be needed by these other callers, but
it seemed like a sane refactorization.

I've not tested, but I imagine this will need patched all the way back.
The rule case should be reachable in all supported versions.

            regards, tom lane

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 9ac42efdbc..6dc117dea8 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -475,6 +475,8 @@ static void get_from_clause(Query *query, const char *prefix,
                             deparse_context *context);
 static void get_from_clause_item(Node *jtnode, Query *query,
                                  deparse_context *context);
+static void get_rte_alias(RangeTblEntry *rte, int varno, bool use_as,
+                          deparse_context *context);
 static void get_column_alias_list(deparse_columns *colinfo,
                                   deparse_context *context);
 static void get_from_clause_coldeflist(RangeTblFunction *rtfunc,
@@ -6648,12 +6650,14 @@ get_insert_query_def(Query *query, deparse_context *context,
         context->indentLevel += PRETTYINDENT_STD;
         appendStringInfoChar(buf, ' ');
     }
-    appendStringInfo(buf, "INSERT INTO %s ",
+    appendStringInfo(buf, "INSERT INTO %s",
                      generate_relation_name(rte->relid, NIL));
-    /* INSERT requires AS keyword for target alias */
-    if (rte->alias != NULL)
-        appendStringInfo(buf, "AS %s ",
-                         quote_identifier(rte->alias->aliasname));
+
+    /* Print the relation alias, if needed; INSERT requires explicit AS */
+    get_rte_alias(rte, query->resultRelation, true, context);
+
+    /* always want a space here */
+    appendStringInfoChar(buf, ' ');

     /*
      * Add the insert-column-names list.  Any indirection decoration needed on
@@ -6835,9 +6839,10 @@ get_update_query_def(Query *query, deparse_context *context,
     appendStringInfo(buf, "UPDATE %s%s",
                      only_marker(rte),
                      generate_relation_name(rte->relid, NIL));
-    if (rte->alias != NULL)
-        appendStringInfo(buf, " %s",
-                         quote_identifier(rte->alias->aliasname));
+
+    /* Print the relation alias, if needed */
+    get_rte_alias(rte, query->resultRelation, false, context);
+
     appendStringInfoString(buf, " SET ");

     /* Deparse targetlist */
@@ -7043,9 +7048,9 @@ get_delete_query_def(Query *query, deparse_context *context,
     appendStringInfo(buf, "DELETE FROM %s%s",
                      only_marker(rte),
                      generate_relation_name(rte->relid, NIL));
-    if (rte->alias != NULL)
-        appendStringInfo(buf, " %s",
-                         quote_identifier(rte->alias->aliasname));
+
+    /* Print the relation alias, if needed */
+    get_rte_alias(rte, query->resultRelation, false, context);

     /* Add the USING clause if given */
     get_from_clause(query, " USING ", context);
@@ -10887,10 +10892,8 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
     {
         int            varno = ((RangeTblRef *) jtnode)->rtindex;
         RangeTblEntry *rte = rt_fetch(varno, query->rtable);
-        char       *refname = get_rtable_name(varno, context);
         deparse_columns *colinfo = deparse_columns_fetch(varno, dpns);
         RangeTblFunction *rtfunc1 = NULL;
-        bool        printalias;

         if (rte->lateral)
             appendStringInfoString(buf, "LATERAL ");
@@ -11027,59 +11030,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
         }

         /* Print the relation alias, if needed */
-        printalias = false;
-        if (rte->alias != NULL)
-        {
-            /* Always print alias if user provided one */
-            printalias = true;
-        }
-        else if (colinfo->printaliases)
-        {
-            /* Always print alias if we need to print column aliases */
-            printalias = true;
-        }
-        else if (rte->rtekind == RTE_RELATION)
-        {
-            /*
-             * No need to print alias if it's same as relation name (this
-             * would normally be the case, but not if set_rtable_names had to
-             * resolve a conflict).
-             */
-            if (strcmp(refname, get_relation_name(rte->relid)) != 0)
-                printalias = true;
-        }
-        else if (rte->rtekind == RTE_FUNCTION)
-        {
-            /*
-             * For a function RTE, always print alias.  This covers possible
-             * renaming of the function and/or instability of the
-             * FigureColname rules for things that aren't simple functions.
-             * Note we'd need to force it anyway for the columndef list case.
-             */
-            printalias = true;
-        }
-        else if (rte->rtekind == RTE_SUBQUERY ||
-                 rte->rtekind == RTE_VALUES)
-        {
-            /*
-             * For a subquery, always print alias.  This makes the output SQL
-             * spec-compliant, even though we allow the alias to be omitted on
-             * input.
-             */
-            printalias = true;
-        }
-        else if (rte->rtekind == RTE_CTE)
-        {
-            /*
-             * No need to print alias if it's same as CTE name (this would
-             * normally be the case, but not if set_rtable_names had to
-             * resolve a conflict).
-             */
-            if (strcmp(refname, rte->ctename) != 0)
-                printalias = true;
-        }
-        if (printalias)
-            appendStringInfo(buf, " %s", quote_identifier(refname));
+        get_rte_alias(rte, varno, false, context);

         /* Print the column definitions or aliases, if needed */
         if (rtfunc1 && rtfunc1->funccolnames != NIL)
@@ -11217,6 +11168,77 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
              (int) nodeTag(jtnode));
 }

+/*
+ * get_rte_alias - print the relation's alias, if needed
+ *
+ * If printed, the alias is preceded by a space, or by " AS " if use_as is true.
+ */
+static void
+get_rte_alias(RangeTblEntry *rte, int varno, bool use_as,
+              deparse_context *context)
+{
+    deparse_namespace *dpns = (deparse_namespace *) linitial(context->namespaces);
+    char       *refname = get_rtable_name(varno, context);
+    deparse_columns *colinfo = deparse_columns_fetch(varno, dpns);
+    bool        printalias = false;
+
+    if (rte->alias != NULL)
+    {
+        /* Always print alias if user provided one */
+        printalias = true;
+    }
+    else if (colinfo->printaliases)
+    {
+        /* Always print alias if we need to print column aliases */
+        printalias = true;
+    }
+    else if (rte->rtekind == RTE_RELATION)
+    {
+        /*
+         * No need to print alias if it's same as relation name (this would
+         * normally be the case, but not if set_rtable_names had to resolve a
+         * conflict).
+         */
+        if (strcmp(refname, get_relation_name(rte->relid)) != 0)
+            printalias = true;
+    }
+    else if (rte->rtekind == RTE_FUNCTION)
+    {
+        /*
+         * For a function RTE, always print alias.  This covers possible
+         * renaming of the function and/or instability of the FigureColname
+         * rules for things that aren't simple functions.  Note we'd need to
+         * force it anyway for the columndef list case.
+         */
+        printalias = true;
+    }
+    else if (rte->rtekind == RTE_SUBQUERY ||
+             rte->rtekind == RTE_VALUES)
+    {
+        /*
+         * For a subquery, always print alias.  This makes the output
+         * SQL-spec-compliant, even though we allow such aliases to be omitted
+         * on input.
+         */
+        printalias = true;
+    }
+    else if (rte->rtekind == RTE_CTE)
+    {
+        /*
+         * No need to print alias if it's same as CTE name (this would
+         * normally be the case, but not if set_rtable_names had to resolve a
+         * conflict).
+         */
+        if (strcmp(refname, rte->ctename) != 0)
+            printalias = true;
+    }
+
+    if (printalias)
+        appendStringInfo(context->buf, "%s%s",
+                         use_as ? " AS " : " ",
+                         quote_identifier(refname));
+}
+
 /*
  * get_column_alias_list - print column alias list for an RTE
  *
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 174b725fff..a3a5a62329 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2998,28 +2998,21 @@ select * from rules_log;
 (16 rows)

 create rule r4 as on delete to rules_src do notify rules_src_deletion;
-\d+ rules_src
-                                 Table "public.rules_src"
- Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description
---------+---------+-----------+----------+---------+---------+--------------+-------------
- f1     | integer |           |          |         | plain   |              |
- f2     | integer |           |          | 0       | plain   |              |
-Rules:
-    r1 AS
-    ON UPDATE TO rules_src DO  INSERT INTO rules_log (f1, f2, tag, id) VALUES (old.f1,old.f2,'old'::text,DEFAULT),
(new.f1,new.f2,'new'::text,DEFAULT)
-    r2 AS
-    ON UPDATE TO rules_src DO  VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
-    r3 AS
-    ON INSERT TO rules_src DO  INSERT INTO rules_log (f1, f2, tag, id) VALUES
(NULL::integer,NULL::integer,'-'::text,DEFAULT),(new.f1,new.f2,'new'::text,DEFAULT) 
-    r4 AS
-    ON DELETE TO rules_src DO
- NOTIFY rules_src_deletion
-
 --
 -- Ensure an aliased target relation for insert is correctly deparsed.
 --
 create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1,
trgt.f2;
 create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 =
new.f1;
+--
+-- Check deparse disambiguation of INSERT/UPDATE/DELETE targets.
+--
+create rule r7 as on delete to rules_src do instead
+  with wins as (insert into int4_tbl as trgt values (0) returning *),
+       wupd as (update int4_tbl trgt set f1 = f1+1 returning *),
+       wdel as (delete from int4_tbl trgt where f1 = 0 returning *)
+  insert into rules_log AS trgt select old.* from wins, wupd, wdel
+  returning trgt.f1, trgt.f2;
+-- check display of all rules added above
 \d+ rules_src
                                  Table "public.rules_src"
  Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description
@@ -3044,6 +3037,26 @@ Rules:
     r6 AS
     ON UPDATE TO rules_src DO INSTEAD  UPDATE rules_log trgt SET tag = 'updated'::text
   WHERE trgt.f1 = new.f1
+    r7 AS
+    ON DELETE TO rules_src DO INSTEAD  WITH wins AS (
+         INSERT INTO int4_tbl AS trgt_1 (f1)
+          VALUES (0)
+          RETURNING trgt_1.f1
+        ), wupd AS (
+         UPDATE int4_tbl trgt_1 SET f1 = trgt_1.f1 + 1
+          RETURNING trgt_1.f1
+        ), wdel AS (
+         DELETE FROM int4_tbl trgt_1
+          WHERE trgt_1.f1 = 0
+          RETURNING trgt_1.f1
+        )
+ INSERT INTO rules_log AS trgt (f1, f2)  SELECT old.f1,
+            old.f2
+           FROM wins,
+            wupd,
+            wdel
+  RETURNING trgt.f1,
+    trgt.f2

 --
 -- Also check multiassignment deparsing.
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 1f858129b8..ac1a4ce554 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1015,13 +1015,24 @@ insert into rules_src values(22,23), (33,default);
 select * from rules_src;
 select * from rules_log;
 create rule r4 as on delete to rules_src do notify rules_src_deletion;
-\d+ rules_src

 --
 -- Ensure an aliased target relation for insert is correctly deparsed.
 --
 create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1,
trgt.f2;
 create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 =
new.f1;
+
+--
+-- Check deparse disambiguation of INSERT/UPDATE/DELETE targets.
+--
+create rule r7 as on delete to rules_src do instead
+  with wins as (insert into int4_tbl as trgt values (0) returning *),
+       wupd as (update int4_tbl trgt set f1 = f1+1 returning *),
+       wdel as (delete from int4_tbl trgt where f1 = 0 returning *)
+  insert into rules_log AS trgt select old.* from wins, wupd, wdel
+  returning trgt.f1, trgt.f2;
+
+-- check display of all rules added above
 \d+ rules_src

 --