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/[email protected] [2] https://postgr.es/m/[email protected] On Fri, 11 Sept 2026 at 13:28, Robert Haas <[email protected]> wrote: > > On Fri, Sep 11, 2026 at 11:58 AM Tom Lane <[email protected]> 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
