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/
