| From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, Euler Taveira <euler(at)eulerto(dot)com> |
| Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Japin Li <japinli(at)hotmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pg_get__*_ddl consolidation |
| Date: | 2026-03-31 19:42:54 |
| Message-ID: | 555cdee4-c024-4872-9d96-82ef4216239c@dunslane.net |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-03-24 Tu 5:56 PM, Zsolt Parragi wrote:
> v2 is definitely better, I can confirm the improvements.
>
>> including all suggested changes in this thread
> But I want to point out that the permission check question, and the
> role membership question is still open.
>
> For the membership question
>
>> I'm not opposed
>> to your idea but IMO it should be an option.
> I'm also fine with leaving it out, but then it should be a mentioned limitation.
>
> For v2:
>
> Shouldn't the tablespace function also support an owner option
> similarly to the database function?
>
> A pfree(nulls) and pfree(spcname) seem to be missing, all other
> variables are freed properly.
>
> +
> + /*
> + * Variables that are marked GUC_LIST_QUOTE were already fully
> + * quoted before they were put into the setconfig array. Break
> + * the list value apart and then quote the elements as string
> + * literals.
> + */
> + if (GetConfigOptionFlags(s, true) & GUC_LIST_QUOTE)
> + {
>
> This part seems to be duplicated between the two functions, maybe
> could be a helper?
>
>
> +ALTER ROLE regress_role_ddl_test4 SET search_path TO 'myschema, public';
>
> Maybe it would be useful to also test the "SET search_path TO
> myschema, public" variant, without quotes?
>
>
> I also want to go back to the datestyle question one more time:
>
>> The non-fixed DateStyle is by design. It mimics pg_dumpall.
> Isn't pg_dump and pg_dumpall inconsistent in this? pg_dump sets it to
> ISO, pg_dumpall uses DateStyle. So I'm not that sure about the
> "pg_dumpall works this way" argument because pg_dump works
> differently. Maybe either of those tools should also be fixed?
>
> The pg_dump behavior is actually a bugfix in cf4cee1b, which was never
> applied to pg_dumpall, so it seems like an oversight to me?
>
>> At present, dates are put into a dump in the format specified by the
>> default datestyle. This is not portable between installations.
>>
>> This patch sets DATESTYLE to ISO at the start of a pg_dump, so that the
>> dates written into the dump will be restorable onto any database,
>> regardless of how its default datestyle is set.
OK, I hope the attached set addresses all the outstanding issues. We're
using ISO dates, and there are appropriate permissions checks. There is
an option to dump role memberships.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Add-infrastructure-for-pg_get_-_ddl-functions.patch | text/x-patch | 9.9 KB |
| v3-0002-Add-pg_get_role_ddl-function.patch | text/x-patch | 27.2 KB |
| v3-0003-Add-pg_get_tablespace_ddl-function.patch | text/x-patch | 20.4 KB |
| v3-0004-Add-pg_get_database_ddl-function.patch | text/x-patch | 21.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David G. Johnston | 2026-03-31 19:48:31 | Re: docs: warn about post-data-only schema dumps with parallel restore. |
| Previous Message | Alvaro Herrera | 2026-03-31 19:41:53 | Re: Adding REPACK [concurrently] |