public inbox for [email protected]  

help / color / mirror / Atom feed

From: Euler Taveira <[email protected]>
To: Akshay Joshi <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: Chao Li <[email protected]>
Cc: japin <[email protected]>
Cc: Quan Zongliang <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement
Date: Thu, 11 Dec 2025 10:59:07 -0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <CANxoLDeEzSvC8hy7oF=qZZGVD---MDwxesdoQnAQMuJQGOSaJw@mail.gmail.com>
References: <CANxoLDdPqyTLyYhBnekFYkaSn_8DBdtf32cH=MLNJc=+BQP=MQ@mail.gmail.com>
	<[email protected]>
	<CANxoLDeEzSvC8hy7oF=qZZGVD---MDwxesdoQnAQMuJQGOSaJw@mail.gmail.com>

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/

view thread (38+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox