Re: [PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement - Mailing list pgsql-hackers

On Thu, Nov 13, 2025 at 02:02:30PM +0530, Akshay Joshi wrote:
> On Thu, Nov 13, 2025 at 10:18 AM Quan Zongliang <quanzongliang@yeah.net>
> wrote:
> 
> >
> >
> > On 11/13/25 12:17 PM, Quan Zongliang wrote:
> > >
> > >
> > > On 11/12/25 8:04 PM, Akshay Joshi wrote:
> > >> Hi Hackers,
> > >>
> > >> I’m submitting a patch as part of the broader Retail DDL Functions
> > >> project described by Andrew Dunstan https://www.postgresql.org/
> > >> message- id/945db7c5-be75-45bf-b55b-cb1e56f2e3e9%40dunslane.net
> > >> <https:// www.postgresql.org/message-id/945db7c5-be75-45bf-b55b-
> > >> cb1e56f2e3e9%40dunslane.net>
> > >>
> > >> This patch adds a new system function
> > >> pg_get_database_ddl(database_name/ database_oid, pretty), which
> > >> reconstructs the CREATE DATABASE statement for a given database name
> > >> or database oid. When the pretty flag is set to true, the function
> > >> returns a neatly formatted, multi-line DDL statement instead of a
> > >> single-line statement.
> > >>
> > >> *Usage examples:*
> > >>
> > >> 1) SELECT pg_get_database_ddl('test_get_database_ddl_builtin');  --
> > >> *non-pretty formatted DDL*
> > >> pg_get_database_ddl
> > >>
> >
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > >>    CREATE DATABASE test_get_database_ddl_builtin WITH OWNER =
> > >> regress_ddl_database ENCODING = "UTF8" LC_COLLATE = "C" LC_CTYPE = "C"
> > >> BUILTIN_LOCALE = "C.UTF-8" COLLATION_VERSION = "1" LOCALE_PROVIDER =
> > >> 'builtin' TABLESPACE = pg_default ALLOW_CONNECTIONS = true CONNECTION
> > >> LIMIT = -1;
> > >>
> > >>
> > >> 2) SELECT pg_get_database_ddl('test_get_database_ddl_builtin', true);
> > >> -- *pretty formatted DDL*
> > >>
> > >> CREATE DATABASE test_get_database_ddl_builtin
> > >>           WITH
> > >>           OWNER = regress_ddl_database
> > >>           ENCODING = "UTF8"
> > >>           LC_COLLATE = "C"
> > >>           LC_CTYPE = "C"
> > >>           BUILTIN_LOCALE = "C.UTF-8"
> > >>           COLLATION_VERSION = "1"
> > >>           LOCALE_PROVIDER = 'builtin'
> > >>           TABLESPACE = pg_default
> > >>           ALLOW_CONNECTIONS = true
> > >>           CONNECTION LIMIT = -1;
> > >>
> > >> 3) SELECT pg_get_database_ddl(16835);      -- *non-pretty formatted
> > >> DDL for OID*
> > >> 4) SELECT pg_get_database_ddl(16835, true);  -- *pretty formatted DDL
> > >> for OID*
> > >>
> > >> The patch includes documentation, in-code comments, and regression
> > >> tests, all of which pass successfully.
> > >> *
> > >> **Note:* To run the regression tests, particularly the pg_upgrade
> > >> tests successfully, I had to add a helper function, ddl_filter (in
> > >> database.sql), which removes locale and collation-related information
> > >> from the pg_get_database_ddl output.
> > >>
> > > I think we should check the connection permissions here. Otherwise:
> > >
> > > postgres=> SELECT pg_database_size('testdb');
> > > ERROR:  permission denied for database testdb
> > > postgres=> SELECT pg_get_database_ddl('testdb');
> > >
> > >                          pg_get_database_ddl
> > >
> >
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> > >   CREATE DATABASE testdb WITH OWNER = quanzl ENCODING = "UTF8"
> > > LC_COLLATE = "zh_CN.UTF-8" LC_CTYPE = "zh_CN.UTF-8" LOCALE_PROVIDER =
> > > 'libc' TABLESPACE = pg_default ALLOW_CONNECTIONS = true CONNECTION LIMIT
> > > = -1;
> > > (1 row)
> > >
> > > Users without connection permissions should not generate DDL.
> > >
> >
> > The "dbOwner" is defined as a null pointer.
> > char       *dbOwner = NULL;
> >
> > Later, there might be a risk of it not being assigned a value.
> >     if (OidIsValid(dbForm->datdba))
> >        dbOwner = GetUserNameFromId(dbForm->datdba, false);
> >
> > Although there is no problem in normal circumstances here. Many parts of
> > the existing code have not been checked either. Since this possibility
> > exists, it should be checked before using it. Just like the function
> > roles_is_member_of (acl.c).
> >
> > if (dbOwner)
> >    get_formatted_string(&buf, prettyFlags, 1, "OWNER = %s",
> >                          quote_identifier(dbOwner));
> >
> 
>  Fixed the given review comment. I've attached the v2 patch ready for
> review.
> 

Thanks for updating the patch, some comments on v2.

1.
Should we merge the functions pg_get_database_ddl(oid, [boolean]) and
pg_get_database_ddl(name, [boolean]) in documentation, following the
precedent set by pg_database_size in func-admin.sgml.

2.
+ * noOfTabChars - indent with specified no of tabs.

How about using 'indent with specified number of tab characters'?
And for variable noOfTabChars, I'd like use nTabs or nTabChars.

3.
+/*
+ * pg_get_database_ddl_oid
+ *
+ * Generate a CREATE DATABASE statement for the specified database oid.
+ *
+ * dbName - Name of the database for which to generate the DDL.
+ * pretty - If true, format the DDL with indentation and line breaks.
+ */

A copy-paste error resulted in an incorrect comments (dbName).

-- 
Best regards,
Japin Li
ChengDu WenWu Information Technology Co., LTD.