Feedback-ID: i0c21471d:Fastmail
MIME-Version: 1.0
Date: Thu, 11 Dec 2025 10:59:07 -0300
From: "Euler Taveira" <euler@eulerto.com>
To: "Akshay Joshi" <akshay.joshi@enterprisedb.com>,
 =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
Cc: "Chao Li" <li.evan.chao@gmail.com>, japin <japinli@hotmail.com>,
 "Quan Zongliang" <quanzongliang@yeah.net>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Message-Id: <4e60bcae-8222-4e1f-8e5b-d73b59c93304@app.fastmail.com>
In-Reply-To: 
 <CANxoLDeEzSvC8hy7oF=qZZGVD---MDwxesdoQnAQMuJQGOSaJw@mail.gmail.com>
References: 
 <CANxoLDdPqyTLyYhBnekFYkaSn_8DBdtf32cH=MLNJc=+BQP=MQ@mail.gmail.com>
 <202511191045.tckydhpoxumx@alvherre.pgsql>
 <CANxoLDeEzSvC8hy7oF=qZZGVD---MDwxesdoQnAQMuJQGOSaJw@mail.gmail.com>
Subject: Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE
 DATABASE
 statement
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/4e60bcae-8222-4e1f-8e5b-d73b59c93304%40app.fastmail.com>
Precedence: bulk

On Thu, Nov 20, 2025, at 6:18 AM, Akshay Joshi wrote:
>
>  Implemented in the suggested solution. Attached is the v5 patch for review.
>

I reviewed your patch and have some suggestions for this patch.

* You shouldn't include the property if the value is default. A long command
  adds nothing. Clarity? Tell someone that needs to select, copy and paste a
  long statement. It is a good goal to provide a short command to reconstruct
  the object. If you don't know why it didn't include CONNECTION LIMIT, it is
  time to check the manual again.

$ psql -AtqX -c "SELECT pg_get_database_ddl('postgres')" -d postgres
CREATE DATABASE postgres WITH OWNER = euler ENCODING = "UTF8" LC_COLLATE = "pt_BR.UTF-8" LC_CTYPE = "pt_BR.UTF-8" COLLATION_VERSION = "2.36" LOCALE_PROVIDER = 'libc' TABLESPACE = pg_default ALLOW_CONNECTIONS = true CONNECTION LIMIT = -1;

* Use single quotes. The encoding, locale, lc_collate, lc_type,
  collation_version and some other properties should use single quotes. The
  locale_provider doesn't need a single quote because it is an enum. See how
  pg_dumpall constructs the command. Use simple_quote_literal.

$ pg_dumpall --binary-upgrade | grep 'p5'
-- Database "p5" dump
-- Name: p5; Type: DATABASE; Schema: -; Owner: euler
CREATE DATABASE p5 WITH TEMPLATE = template0 OID = 16392 STRATEGY = FILE_COPY ENCODING = 'UTF8' LOCALE_PROVIDER = libc LOCALE = 'en_US.UTF-8' COLLATION_VERSION = '2.36';
ALTER DATABASE p5 OWNER TO euler;
\connect p5

* OWNER. There is no guarantee that the owner exists in the cluster you will
  use this output. That's something that pg_dumpall treats separately (see
  above). Does it mean we should include the owner? No. We can make it an
  option.

* LOCALE. Why didn't you include it? I know there are some combinations that
  does not work together but this function can provide a minimal set of
  properties related to locale.

postgres=# CREATE DATABASE p6 LOCALE_PROVIDER builtin LOCALE 'C' TEMPLATE template0;
CREATE DATABASE

* STRATEGY. Although this is a runtime property, it should be an option.

* TEMPLATE. Ditto.

* options. Since I mentioned options for some properties (owner, strategy,
  template), these properties can be accommodated as a VARIADIC argument. The
  function signature can be something like

pg_get_database_ddl(oid, VARIADIC options text[])

I would include the pretty print into options too.

* Tabs. I don't think we use tabs to format output. Use spaces. A good practice
  is to use EXPLAIN style (2 spaces)and depending on the nesting, 4 spaces are
  fine too.

$ psql -AtqX -c "SELECT pg_get_database_ddl('postgres', true)" -d postgres
CREATE DATABASE postgres
	WITH OWNER = euler
		ENCODING = "UTF8"
		LC_COLLATE = "pt_BR.UTF-8"
		LC_CTYPE = "pt_BR.UTF-8"
		COLLATION_VERSION = "2.36"
		LOCALE_PROVIDER = 'libc'
		TABLESPACE = pg_default
		ALLOW_CONNECTIONS = true
		CONNECTION LIMIT = -1;

* permission. I don't think you need to check for permissions inside the
  function. I wouldn't want a different behavior than pg_dump(all). You can
  always adjust it in system_functions.sql.

* typo.

+--                                                                             
+-- Reconsturct DDL                                                             
+--

s/Reconsturct/Reconstruct/


-- 
Euler Taveira
EDB   https://www.enterprisedb.com/