On Fri, Nov 14, 2025 at 11:19 AM Japin Li <[email protected]> wrote:
> On Thu, Nov 13, 2025 at 02:02:30PM +0530, Akshay Joshi wrote: > > On Thu, Nov 13, 2025 at 10:18 AM Quan Zongliang <[email protected]> > > 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). > > All the review comments have been addressed in v3 patch. > -- > Best regards, > Japin Li > ChengDu WenWu Information Technology Co., LTD. >
