| From: | Japin Li <japinli(at)hotmail(dot)com> |
|---|---|
| To: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
| Cc: | 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-11-14 05:49:07 |
| Message-ID: | SY0P300MB153948953051E054E8F69487B6CAA@SY0P300MB1539.AUSP300.PROD.OUTLOOK.COM |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | 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(at)yeah(dot)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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2025-11-14 05:49:23 | Re: Extended Statistics set/restore/clear functions. |
| Previous Message | Chao Li | 2025-11-14 05:17:47 | Re: Confused static assertion implementation |