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

Jonathan S. Katz <jkatz@postgresql.org>

From: "Jonathan S. Katz" <jkatz@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: vignesh C <vignesh21@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-17T19:00:21Z
Lists: pgsql-hackers
On 2/17/23 1:18 PM, Tom Lane wrote:

> 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.

Ah, I see based on your example below. I did not alias the INSERT 
statement in the way (and I don't know how common of a pattern it is to 
o that).

> 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.

I think I agree. Most people should not be looking at the disambiguated 
statements unless they are troubleshooting an issue (such as $SUBJECT). 
The main goal is to disambiguate correctly.

> 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.

I tested this against HEAD (+v69 of the DDL replication patch). My cases 
are now all passing.

The code looks good to me -- I don't know if moving that logic is 
overkill, but it makes the solution relatively clean.

I didn't test in any back branches yet, but given this can generate an 
invalid function body, it does likely need to be backpatched.

Thanks,

Jonathan

Commits

  1. Print the correct aliases for DML target tables in ruleutils.