Re: pg_get_*_ddl() needs a redesign

Sorry for top-posting but I reckon I'm not replying to each statement
separately but as a whole.

Hello all,

As one of the authors of pg_get_role_ddl(), I've been reading this
thread *carefully*, and I took quite a long time to analise what and
how I'd address the concerns. I noticed that despite the fact that
I've put a lot of effort a) I may not be understanding all of the
objections correctly and b) before anyone (hopefully me) spends more
time on code for v20 or newer, I'd like to check my reading with you,
and ask whether a possible direction makes sense. Please correct me
wherever I've got it wrong.

sorry.... It's a *very* long email.

How I'm reading the problems
----------------------------

If I understand correctly, the concerns fall into three groups:

1. Plain defects (passwords, "PG_DEFAULT", locking, permissions). I
reckon Andrew and Euler's v6 patches [1] fix these, apart from two
loose ends: password defaults to true, so a default call still needs
pg_authid access, and permission errors name the target object rather
than the denied catalog. However, am I right in thinking these are not
the main concern? (please keep reading if possible)

2. Ordering. Each function returns one object's script, but some of
its statements depend on other objects: IN DATABASE settings need the
database, and GRANT ... GRANTED BY g needs g to hold ADMIN OPTION
first, which comes from g's output. I'm worried the cross-role case
can't be solved one role at a time, which I believe is why
dumpRoleMembership() makes repeated passes. Noah, is this the redesign
issue you meant?

3. Two independent implementations. pg_dumpall builds the same DDL on
the client side, and nothing would tell us when the two drift apart,
which I understood to be Robert's main concern.

I believe that's a fair summary, so what I hope we could achieve:

* Users can get the definition of an existing object from SQL, for the
'give me something like this object so I can tweak it' case that
Andrew and Marcos described.
* There is one implementation of 'catalog state -> SQL' per object
type, ideally used by pg_dump/pg_dumpall too, so it gets tested all
the time.
* The backend functions describe one object at a time and make no
promise about the order in which their output can be run. Working out
that order, across all objects, stays with pg_dump/pg_dumpall, which
already does it.

Does this match what others would want, or are some of these goals,
maybe not realistic?

A possible direction, for discussion
------------------------------------

a) Split pg_get_role_ddl() into smaller functions: one for the CREATE
ROLE, one for the role's settings, one for its memberships. None of
them tries to put statements in restore order; the caller decides when
to run each part.
(Meant to address Noah's ordering concern.)

b) pg_dumpall calls these functions instead of building the same SQL
itself, so there is only one copy of the code that turns catalog rows
into SQL. For servers too old to have the functions, pg_dumpall keeps
its current code, which no longer needs to change because those
servers won't change either. I reckon this is close to what Tom
sketched [2]. (Meant to address the 'two implementations' concern.)

c) Tests compare the functions' output with pg_dumpall's for the same
objects, as v6 begins to. They'd need to be broader than v6's,
perhaps driven by pg_dump's own tests, to catch a newly added option.
(Meant to address Robert's silent-drift and Andres's cross-check concerns.)

Questions where I'd need confirmation
-------------------------------------

1. Noah, Tom: if pg_dumpall actually used these functions, would that
address the 'one implementation' concern? If not, what would?

2. Would it be acceptable to keep the current client-side code for
older source servers, expecting to remove it eventually?

I'd rather not start on a patch until I know whether this direction is
reasonable. If it is, I'd be happy to start with roles, since
that's the part I know best.

Thank you for your patience, and for the reviews so far.

Regards,
Mario Gonzalez

[1] https://postgr.es/m/88e205f5-f561-4a00-b6e6-ab070ea6400a@dunslane.net
[2] https://postgr.es/m/956966.1789102554@sss.pgh.pa.us

On Fri, 11 Sept 2026 at 13:28, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Fri, Sep 11, 2026 at 11:58 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Maybe --quote-all-identifiers should be the default? I don't think
> > that we break old syntax "pretty regularly" in any other way.
> > But that's a side point.
>
> It's possible it's been less in recent years, but I feel like that's
> just the most common way, like you know 50% of the total breakage or
> something.
>
> > I'm not quite convinced. But if that's really the case, I'm voting
> > for "discard the feature". It's not useful enough to justify doubling
> > the maintenance load associated with DDL reconstruction.
>
> I think what really bothers me here is the chances of silent breakage,
> which appear to be high. If it were somehow the case that adding a new
> feature to some kind of DDL object, like a FLAVOR banana option to
> CREATE TABLE, were likely to produce test failures until the
> corresponding code was updated, I don't think that would be bad. It
> would just be one more small step that everyone extending a DDL
> command would have to do. But as it is, I don't think anything would
> alert you to the oversight. That's likely to lead to a lot of future
> headaches as we discover that things have been broken for years
> without anybody noticing. Or maybe in the LLM era we'll just get a
> flood of post-commit reports pointing out the problem. But that still
> leads to us playing catch-up, which is not a good situation.
>
> --
> Robert Haas

--
Mario Gonzalez