On Wed, 19 Nov 2025 at 16:36, Akshay Joshi <[email protected]> wrote: > On Wed, Nov 19, 2025 at 3:48 PM Japin Li <[email protected]> wrote: > > Hi Akshay, > > Thanks for updating the patch. > > On Tue, 18 Nov 2025 at 13:33, Akshay Joshi <[email protected]> > wrote: > > Hi Chao > > > > Thanks for reviewing my patch. > > > > On Tue, Nov 18, 2025 at 5:59 AM Chao Li <[email protected]> wrote: > > > > Hi Akshay, > > > > I just reviewed v3 and got some comments: > > > > > On Nov 17, 2025, at 22:34, Akshay Joshi <[email protected]> > wrote: > > > > > > All the review comments have been addressed in v3 patch. > > > > 1 - ruleutils.c > > ``` > > + if (dbForm->datconnlimit != 0) > > + get_formatted_string(&buf, prettyFlags, 1, "CONNECTION > LIMIT = %d", > > + > dbForm->datconnlimit); > > ``` > > > > I think this is wrong. Default value of CONNECTION_LIMIT is -1 rather > than 0. 0 means no connection is allowed, > users > > should intentionally set the value, thus 0 should be printed. > > > > 2 - ruleutils.c > > ``` > > + if (!attrIsNull) > > + get_formatted_string(&buf, prettyFlags, 1, "ICU_RULES = > %s", > > + > quote_identifier(TextDatumGetCString(dbValue))); > > ``` > > > > ICU_RULES should be omitted if provider is not icu. > > > > 3 - ruleutils.c > > ``` > > + if (!HeapTupleIsValid(tupleDatabase)) > > + ereport(ERROR, > > + errcode(ERRCODE_UNDEFINED_OBJECT), > > + errmsg("database with oid %d does not > exist", dbOid)); > > ``` > > > > I believe all existing code use %u to format oid. I ever raised the same > comment to the other get_xxx_ddl patch. > > > > Fixed all above in the attached v4 patch. > > 1. > Since the STRATEGY and OID parameters are not being reconstructed, should we > document this behavior? > > postgres=# CREATE DATABASE mydb WITH STRATEGY file_copy OID 19876 > IS_TEMPLATE true; > CREATE DATABASE > postgres=# SELECT pg_get_database_ddl('mydb', true); > pg_get_database_ddl > -------------------------------------------- > CREATE DATABASE mydb + > WITH OWNER = japin + > ENCODING = "UTF8" + > LC_COLLATE = "en_US.UTF-8"+ > LC_CTYPE = "en_US.UTF-8" + > COLLATION_VERSION = "2.39"+ > LOCALE_PROVIDER = 'libc' + > TABLESPACE = pg_default + > ALLOW_CONNECTIONS = true + > CONNECTION LIMIT = -1 + > IS_TEMPLATE = true; > (1 row) > > The FormData_pg_database structure does not expose the database STRATEGY,
Yes, it's not exposed. > and reconstructing the OID serves no practical > purpose. If the majority agrees, I can extend the DDL to include OID. I'm not insisting on reconstructing those parameters; I mean, we can provide a sentence to describe this behavior. -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd.
