MIME-Version: 1.0
References: <202603201311.yhtqmvektawm@alvherre.pgsql>
 <bf49721a-a56f-4bdc-ac48-5d4dc1d01805@dunslane.net>
 <8ec9b67d-939e-4b22-8d56-a5129f92d32d@app.fastmail.com>
In-Reply-To: <8ec9b67d-939e-4b22-8d56-a5129f92d32d@app.fastmail.com>
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Tue, 24 Mar 2026 21:56:56 +0000
Message-ID: 
 <CAN4CZFNYM5jNA_gLu9miAXg_7c8Z2zf-ePc+0rzidAV3CBs=cw@mail.gmail.com>
Subject: Re: pg_get__*_ddl consolidation
To: Euler Taveira <euler@eulerto.com>
Cc: Andrew Dunstan <andrew@dunslane.net>,
 =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>,
	Japin Li <japinli@hotmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAN4CZFNYM5jNA_gLu9miAXg_7c8Z2zf-ePc%2B0rzidAV3CBs%3Dcw%40mail.gmail.com>
Precedence: bulk

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.