| From: | "Euler Taveira" <euler(at)eulerto(dot)com> |
|---|---|
| To: | "Akshay Joshi" <akshay(dot)joshi(at)enterprisedb(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | "Chao Li" <li(dot)evan(dot)chao(at)gmail(dot)com>, japin <japinli(at)hotmail(dot)com>, "Quan Zongliang" <quanzongliang(at)yeah(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement |
| Date: | 2025-12-11 13:59:07 |
| Message-ID: | 4e60bcae-8222-4e1f-8e5b-d73b59c93304@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dilip Kumar | 2025-12-11 14:19:29 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Bertrand Drouvot | 2025-12-11 13:27:56 | Fix and improve allocation formulas |