Re: [PATCH] Add pg_get_subscription_ddl() function

7

```
+       /* Append connection info to the CREATE SUBSCRIPTION statement */
+       appendStringInfo(&buf, "CONNECTION \'%s\'", conninfo);
```
A connection string contains a db access credential, and ROLE_PG_READ_ALL_DATA (not a high privilege) can view the DDL, is there a concern of leaking the secret? Should we redact the password in connection string?

> 1.  Question - is it deliberate to *always* return DLL with every

> possible option assigned, even if those are just the option default
> values?  e.g. For something like the CREATE PUBLICATION command the
> string returned could be only half the size if it accounts for
> default.
Yeah, I was asking myself the same.  I think we definitely want options
to be printed when there are GUCs that can affect the outcome (e.g.,
something that is considered default in this server but not on a
differently- configured one would give different results).  But for
those that are just hardcoded defaults, omitting them would make sense.

> 2.  I was also wondering if it was really necessary to have so many

> appendStringInfoString() calls to reconstruct the command.
There are a couple of these patches that have an auxiliary
pretty-printing helper function to add newline-tabs instead of
individual spaces.  I think that wouldn't work as nicely if you tried to
condense the printing in the way you suggest.  On the other hand, if you
have a long format string, it's harder to visually match each specifier
to its corresponding argument.  If this was performance-critical code I
would agree to use denser code and avoid function calls, but for this
usage I don't think we care much.