[PATCH] Add pg_get_database_ddl() function to reconstruct CREATE DATABASE statement
Hi Hackers,
I’m submitting a patch as part of the broader Retail DDL Functions project
described by Andrew Dunstan
/messages/by-id/945db7c5-be75-45bf-b55b-cb1e56f2e3e9@dunslane.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.
-----
Regards,
Akshay Joshi
EDB (EnterpriseDB)
Attachments:
0001-Add-pg_get_database_ddl-function-to-reconstruct-CREATE.patchapplication/octet-stream; name=0001-Add-pg_get_database_ddl-function-to-reconstruct-CREATE.patchDownload+430-1
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.
Regards,
Quan Zongliang
Show quoted text
-----
Regards,
Akshay Joshi
EDB (EnterpriseDB)
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));
Show quoted text
Regards,
Quan Zongliang-----
Regards,
Akshay Joshi
EDB (EnterpriseDB)
On Thu, Nov 13, 2025 at 9:47 AM Quan Zongliang <quanzongliang@yeah.net>
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.
pg_database_size() requires CONNECT or pg_read_all_stats privileges since
it accesses on-disk storage details of a database, which are treated as
sensitive information. In contrast, other system functions might not need
such privileges because they operate within the connected database or
reveal less sensitive data.
In my view, the pg_get_database_ddl() function *should not* require CONNECT
or pg_read_all_stats privileges for consistency and security.
Show quoted text
Regards,
Quan Zongliang-----
Regards,
Akshay Joshi
EDB (EnterpriseDB)
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.
Show quoted text
Regards,
Quan Zongliang-----
Regards,
Akshay Joshi
EDB (EnterpriseDB)
Attachments:
v2-0001-Add-pg_get_database_ddl-function-to-reconstruct-CREATE.patchapplication/octet-stream; name=v2-0001-Add-pg_get_database_ddl-function-to-reconstruct-CREATE.patchDownload+432-1
On 11/13/25 4:30 PM, Akshay Joshi wrote:
On Thu, Nov 13, 2025 at 9:47 AM Quan Zongliang <quanzongliang@yeah.net
<mailto:quanzongliang@yeah.net>> 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- <https://www.postgresql.org/message->
id/945db7c5-be75-45bf-b55b-cb1e56f2e3e9%40dunslane.net
<http://40dunslane.net> <https://
www.postgresql.org/message-id/945db7c5-be75-45bf-b55b- <http://
www.postgresql.org/message-id/945db7c5-be75-45bf-b55b->
cb1e56f2e3e9%40dunslane.net <http://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 = trueCONNECTION
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 formattedDDL
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_upgradetests
successfully, I had to add a helper function, ddl_filter (in
database.sql), which removes locale and collation-relatedinformation
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.
pg_database_size() requires CONNECT or pg_read_all_stats privileges
since it accesses on-disk storage details of a database, which are
treated as sensitive information. In contrast, other system functions
might not need such privileges because they operate within the connected
database or reveal less sensitive data.In my view, the pg_get_database_ddl() function *should not* require
CONNECT or pg_read_all_stats privileges for consistency and security.
Agree.
But what about the following scenario? If there is no permission to
access pg_database. Shouldn't the DDL be returned?
postgres=> SELECT * FROM pg_database;
ERROR: permission denied for table pg_database
postgres=> SELECT pg_get_database_ddl('testdb');
Show quoted text
Regards,
Quan Zongliang-----
Regards,
Akshay Joshi
EDB (EnterpriseDB)
On 11/13/25 8:28 PM, Álvaro Herrera wrote:
But what about the following scenario? If there is no permission to access
pg_database. Shouldn't the DDL be returned?postgres=> SELECT * FROM pg_database;
ERROR: permission denied for table pg_database
postgres=> SELECT pg_get_database_ddl('testdb');Hmm, what scenario is this? Did you purposefully REVOKE the SELECT
privileges from pg_database somehow?
Yes. I revoked the access permission using the REVOKE command.
The pg_get_xxx_ddl function is actually revealing system information to
the users. This is equivalent to accessing the corresponding system
table. So I think we should consider this issue.
The access permission to the system tables has been revoked. This user
is unable to directly view the contents of the system tables from the
client side via SQL. However, it is still possible to obtain the object
definitions (which was previously inaccessible) through pg_get_xxx_ddl.
This is more like a security flaw.
A more specific example. Originally, it was impossible to obtain the
definition of "testdb" by accessing pg_database:
postgres=> SELECT * FROM pg_database WHERE datname='testdb';
ERROR: permission denied for table pg_database
And after having this function. However, users can view these in another
format.
postgres=> SELECT pg_get_database_ddl('testdb');
------------- ...
CREATE DATABASE testdb WITH OWNER = quanzl ENCODING = "UTF8" ...
Perhaps it's just that I'm overthinking things. What do you think about it?
Import Notes
Reply to msg id not found: 202511131220.vxws67smjbvn@alvherre.pgsqlReference msg id not found: 202511131220.vxws67smjbvn@alvherre.pgsql | Resolved by subject fallback
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.
Hi Akshay,
I quick went through the patch, I do see some problems, but I need some time to wrap up, so I will do a deep review next week. In the meantime, I want to first ask that why there is no privilege check? I think that’s a serious issue.
On Nov 13, 2025, at 16:32, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
<v2-0001-Add-pg_get_database_ddl-function-to-reconstruct-CREATE.patch>
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On 2025-Nov-13, Quan Zongliang wrote:
A more specific example. Originally, it was impossible to obtain the
definition of "testdb" by accessing pg_database:postgres=> SELECT * FROM pg_database WHERE datname='testdb';
ERROR: permission denied for table pg_database
Hmm. So I was thinking that running things in this mode (where catalog
access is restricted) has never been supported. But you're right that
we would be opening a hole that we don't have today, because if the
admin closes down permissions on pg_database, then this new function
would be a way to obtain information that the user can't currently
obtain.
My further point was to be that you still need to obtain a list of
database names or OIDs in order to do anything of value. But it turns
out that this is extremely easy and quick to do, with something like
SELECT i, pg_describe_object('pg_database'::regclass, i, 0)
FROM generate_series(1, 1_000_000) i
WHERE pg_describe_object('pg_database'::regclass, i, 0) IS NOT NULL;
... and with this function, the user could again obtain everything about
the database even when they can't read the catalog directly.
Maybe checking privs for the database being dumped is enough protection
against this -- the equivalent of has_database_privilege( ..., 'CONNECT')
I suppose.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"¿Qué importan los años? Lo que realmente importa es comprobar que
a fin de cuentas la mejor edad de la vida es estar vivo" (Mafalda)
Import Notes
Reply to msg id not found: fb7deab9-e413-4e14-ad91-b9749d1648af@yeah.net | Resolved by subject fallback
On Fri, Nov 14, 2025 at 11:19 AM Japin Li <japinli@hotmail.com> 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 <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 = trueCONNECTION
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 formattedDDL
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-relatedinformation
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 CONNECTIONLIMIT
= -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.
Show quoted text
--
Best regards,
Japin Li
ChengDu WenWu Information Technology Co., LTD.
The v3 patch adds a check for the CONNECT privilege on the target database
for pg_get_database_ddl(). This aligns its security model with functions
like pg_database_size(). Note that revoking permissions on the *pg_database*
table alone is insufficient to restrict DDL access; users must manually
revoke permission on the pg_get_database_ddl() function itself if
restriction is desired.
Attached is the v3 patch ready for review.
On Fri, Nov 14, 2025 at 4:42 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
Show quoted text
On 2025-Nov-13, Quan Zongliang wrote:
A more specific example. Originally, it was impossible to obtain the
definition of "testdb" by accessing pg_database:postgres=> SELECT * FROM pg_database WHERE datname='testdb';
ERROR: permission denied for table pg_databaseHmm. So I was thinking that running things in this mode (where catalog
access is restricted) has never been supported. But you're right that
we would be opening a hole that we don't have today, because if the
admin closes down permissions on pg_database, then this new function
would be a way to obtain information that the user can't currently
obtain.My further point was to be that you still need to obtain a list of
database names or OIDs in order to do anything of value. But it turns
out that this is extremely easy and quick to do, with something likeSELECT i, pg_describe_object('pg_database'::regclass, i, 0)
FROM generate_series(1, 1_000_000) i
WHERE pg_describe_object('pg_database'::regclass, i, 0) IS NOT NULL;... and with this function, the user could again obtain everything about
the database even when they can't read the catalog directly.Maybe checking privs for the database being dumped is enough protection
against this -- the equivalent of has_database_privilege( ..., 'CONNECT')
I suppose.--
Álvaro Herrera PostgreSQL Developer —
https://www.EnterpriseDB.com/
"¿Qué importan los años? Lo que realmente importa es comprobar que
a fin de cuentas la mejor edad de la vida es estar vivo" (Mafalda)
Attachments:
v3-0001-Add-pg_get_database_ddl-function-to-reconstruct-C.patchapplication/octet-stream; name=v3-0001-Add-pg_get_database_ddl-function-to-reconstruct-C.patchDownload+438-1
Hi Akshay,
I just reviewed v3 and got some comments:
On Nov 17, 2025, at 22:34, Akshay Joshi <akshay.joshi@enterprisedb.com> 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.
4 - ruleutils.c
```
+ /*
+ * User must have connect privilege for target database.
+ */
+ aclresult = object_aclcheck(DatabaseRelationId, dbOid, GetUserId(),
+ ACL_CONNECT);
+ if (aclresult != ACLCHECK_OK)
+ {
+ aclcheck_error(aclresult, OBJECT_DATABASE,
+ get_database_name(dbOid));
+ }
```
I don’t think CONNECT privilege is good enough. By default, a new user gets CONNECT privilege via the PUBLIC role. I just did a quick test to confirm that.
```
# Create a new cluster
% initdb .
% pg_ctl -D . start
% createdb evantest
% createdb evan
# connect to the db
% psql -d evantest -U evan
psql (19devel)
Type "help" for help. # Got into the database successfully
# Without any privilege grant, the user can get ddl of the system database, which seems not good
evantest=> select pg_get_database_ddl('postgres', true);
pg_get_database_ddl
------------------------------------
CREATE DATABASE postgres +
WITH +
OWNER = chaol +
ENCODING = "UTF8" +
LC_COLLATE = "en_US.UTF-8"+
LC_CTYPE = "en_US.UTF-8" +
LOCALE_PROVIDER = 'libc' +
TABLESPACE = pg_default +
ALLOW_CONNECTIONS = true +
CONNECTION LIMIT = -1;
(1 row)
```
IMO, only super user and database owner should be to get ddl of the database.
5 - as you can see from the above test output, “+” in the end of every line is weird.
6 - “WITH” has the same indent as the parameters, which doesn’t good look. If we look at the doc https://www.postgresql.org/docs/18/sql-createdatabase.html, “WITH” takes the first level of indent, and parameters take the second level of indent.
7 - For those parameters that have default values should be omitted. For example:
```
evantest=> select pg_get_database_ddl('evantest', true);
pg_get_database_ddl
------------------------------------
CREATE DATABASE evantest +
WITH +
OWNER = chaol +
ENCODING = "UTF8" +
LC_COLLATE = "en_US.UTF-8"+
LC_CTYPE = "en_US.UTF-8" +
LOCALE_PROVIDER = 'libc' +
TABLESPACE = pg_default +
ALLOW_CONNECTIONS = true +
CONNECTION LIMIT = -1;
(1 row)
```
I created the database “evantest” without providing any parameter. I think at least OWNER, TABLESPACE and ALLOW_CNONNECTIONS should be omitted. For CONNECTION LIMIT, you already have a logic to omit it but the logic has some problem as comment 1.
8 - ruleutils.c
```
+/*
+ * get_formatted_string
+ *
+ * Return a formatted version of the string.
+ *
+ * prettyFlags - Based on prettyFlags the output includes tabs (\t) and
+ * newlines (\n).
+ * nTabChars - indent with specified number of tab characters.
+ * fmt - printf-style format string used by appendStringInfoVA.
+ */
+static void
+get_formatted_string(StringInfo buf, int prettyFlags, int nTabChars, const char *fmt,...)
```
I don’t feel good with this function, but not because of the implementation of the function.
I have reviewed a bunch of get_xxx_ddl patches submitted by different persons. All of them are under a big project, however, looks like to me that all authors work independently without properly coordinated. A function like this one should be common for all those patches. Maybe Alvaro can help here, pushing a common function that is used to format DDL and requesting all patches to use the function.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Hi Chao
Thanks for reviewing my patch.
On Tue, Nov 18, 2025 at 5:59 AM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Akshay,
I just reviewed v3 and got some comments:
On Nov 17, 2025, at 22:34, Akshay Joshi <akshay.joshi@enterprisedb.com>
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.
4 - ruleutils.c ``` + /* + * User must have connect privilege for target database. + */ + aclresult = object_aclcheck(DatabaseRelationId, dbOid, GetUserId(), + ACL_CONNECT); + if (aclresult != ACLCHECK_OK) + { + aclcheck_error(aclresult, OBJECT_DATABASE, + get_database_name(dbOid)); + } ```I don’t think CONNECT privilege is good enough. By default, a new user
gets CONNECT privilege via the PUBLIC role. I just did a quick test to
confirm that.```
# Create a new cluster
% initdb .
% pg_ctl -D . start
% createdb evantest
% createdb evan# connect to the db
% psql -d evantest -U evan
psql (19devel)
Type "help" for help. # Got into the database successfully# Without any privilege grant, the user can get ddl of the system
database, which seems not good
evantest=> select pg_get_database_ddl('postgres', true);
pg_get_database_ddl
------------------------------------
CREATE DATABASE postgres +
WITH +
OWNER = chaol +
ENCODING = "UTF8" +
LC_COLLATE = "en_US.UTF-8"+
LC_CTYPE = "en_US.UTF-8" +
LOCALE_PROVIDER = 'libc' +
TABLESPACE = pg_default +
ALLOW_CONNECTIONS = true +
CONNECTION LIMIT = -1;
(1 row)
```IMO, only super user and database owner should be to get ddl of the
database.
I wasn't entirely sure, but after reviewing the *pg_database_size*()
function, I've concluded that its usage extends beyond the Superuser and
Database Owner. Specifically, other roles can view the database size if
they have the *CONNECT* privilege or are Members of the *pg_read_all_stats*
role.
5 - as you can see from the above test output, “+” in the end of every
line is weird.
The plus sign (+) is merely an artifact of *psql's* output formatting when
a result cell contains a newline character (\n). It serves as a visual cue
to the user that the data continues on the next line. This is confirmed by
the absence of the + sign when viewing the same data in a different client,
such as *pgAdmin*." To suppress this visual cue in psql, you can use the
command: \pset format unaligned
6 - “WITH” has the same indent as the parameters, which doesn’t good look.
If we look at the doc
https://www.postgresql.org/docs/18/sql-createdatabase.html, “WITH” takes
the first level of indent, and parameters take the second level of indent.
Fixed in the v4 patch and followed the docs.
7 - For those parameters that have default values should be omitted. For
example:
```
evantest=> select pg_get_database_ddl('evantest', true);
pg_get_database_ddl
------------------------------------
CREATE DATABASE evantest +
WITH +
OWNER = chaol +
ENCODING = "UTF8" +
LC_COLLATE = "en_US.UTF-8"+
LC_CTYPE = "en_US.UTF-8" +
LOCALE_PROVIDER = 'libc' +
TABLESPACE = pg_default +
ALLOW_CONNECTIONS = true +
CONNECTION LIMIT = -1;
(1 row)
```I created the database “evantest” without providing any parameter. I think
at least OWNER, TABLESPACE and ALLOW_CNONNECTIONS should be omitted. For
CONNECTION LIMIT, you already have a logic to omit it but the logic has
some problem as comment 1.
IMHO, parameters with default values *should not* be omitted from the
output of the pg_get_xxx_ddl functions. The primary purpose of these
functions is to accurately reconstruct the DDL. Including all parameters
ensures clarity, as not everyone is familiar with the default value of
every single parameter.
8 - ruleutils.c ``` +/* + * get_formatted_string + * + * Return a formatted version of the string. + * + * prettyFlags - Based on prettyFlags the output includes tabs (\t) and + * newlines (\n). + * nTabChars - indent with specified number of tab characters. + * fmt - printf-style format string used by appendStringInfoVA. + */ +static void +get_formatted_string(StringInfo buf, int prettyFlags, int nTabChars, const char *fmt,...) ```I don’t feel good with this function, but not because of the
implementation of the function.I have reviewed a bunch of get_xxx_ddl patches submitted by different
persons. All of them are under a big project, however, looks like to me
that all authors work independently without properly coordinated. A
function like this one should be common for all those patches. Maybe Alvaro
can help here, pushing a common function that is used to format DDL and
requesting all patches to use the function.
Yes, all pg_get_xxx_ddl functions are part of a larger project, and
different authors have implemented output formatting in different ways;
some implementations may lack formatting altogether. Yes I agree one common
function should be developed and committed so that all other authors can
reuse it, ensuring consistency across the entire suite of DDL functions."
Show quoted text
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v4-0001-Add-pg_get_database_ddl-function-to-reconstruct-CREATE.patchapplication/octet-stream; name=v4-0001-Add-pg_get_database_ddl-function-to-reconstruct-CREATE.patchDownload+435-1
Hi Akshay,
Thanks for updating the patch.
On Tue, 18 Nov 2025 at 13:33, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Chao
Thanks for reviewing my patch.
On Tue, Nov 18, 2025 at 5:59 AM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Akshay,
I just reviewed v3 and got some comments:
On Nov 17, 2025, at 22:34, Akshay Joshi <akshay.joshi@enterprisedb.com> 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)
2.
We can restrict the scope of the dbTablespace variable as follows:
+ if (OidIsValid(dbForm->dattablespace))
+ {
+ char *dbTablespace = get_tablespace_name(dbForm->dattablespace);
+ if (dbTablespace)
+ get_formatted_string(&buf, prettyFlags, 2, "TABLESPACE = %s",
+ quote_identifier(dbTablespace));
+ }
7 - For those parameters that have default values should be omitted. For
example:
```
evantest=> select pg_get_database_ddl('evantest', true);
pg_get_database_ddl
------------------------------------
CREATE DATABASE evantest +
WITH +
OWNER = chaol +
ENCODING = "UTF8" +
LC_COLLATE = "en_US.UTF-8"+
LC_CTYPE = "en_US.UTF-8" +
LOCALE_PROVIDER = 'libc' +
TABLESPACE = pg_default +
ALLOW_CONNECTIONS = true +
CONNECTION LIMIT = -1;
(1 row)
```I created the database “evantest” without providing any parameter. I think
at least OWNER, TABLESPACE and ALLOW_CNONNECTIONS should be omitted. For
CONNECTION LIMIT, you already have a logic to omit it but the logic has
some problem as comment 1.IMHO, parameters with default values *should not* be omitted from the
output of the pg_get_xxx_ddl functions. The primary purpose of these
functions is to accurately reconstruct the DDL. Including all parameters
ensures clarity, as not everyone is familiar with the default value of
every single parameter.
+1
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
Hello,
One thing I realized a few days ago is that since commit bd09f024a1bb we
have type regdatabase, so instead of having two functions (one taking
name and one taking Oid), we should have just one, taking regdatabase,
just like the functions for producing DDL for other object types that
have corresponding reg* type.
Regards,
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On Wed, Nov 19, 2025 at 3:48 PM Japin Li <japinli@hotmail.com> wrote:
Hi Akshay,
Thanks for updating the patch.
On Tue, 18 Nov 2025 at 13:33, Akshay Joshi <akshay.joshi@enterprisedb.com>
wrote:Hi Chao
Thanks for reviewing my patch.
On Tue, Nov 18, 2025 at 5:59 AM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Akshay,
I just reviewed v3 and got some comments:
On Nov 17, 2025, at 22:34, Akshay Joshi <
akshay.joshi@enterprisedb.com> wrote:
All the review comments have been addressed in v3 patch.
1 - ruleutils.c ``` + if (dbForm->datconnlimit != 0) + get_formatted_string(&buf, prettyFlags, 1, "CONNECTIONLIMIT = %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 notexist", 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*,
and reconstructing the *OID* serves no practical purpose. If the majority
agrees, I can extend the DDL to include OID.
2.
We can restrict the scope of the dbTablespace variable as follows:+ if (OidIsValid(dbForm->dattablespace)) + { + char *dbTablespace = get_tablespace_name(dbForm->dattablespace); + if (dbTablespace) + get_formatted_string(&buf, prettyFlags, 2, "TABLESPACE = %s", + quote_identifier(dbTablespace)); + }
Will fix this in the next patch.
Show quoted text
7 - For those parameters that have default values should be omitted.
For
example:
```
evantest=> select pg_get_database_ddl('evantest', true);
pg_get_database_ddl
------------------------------------
CREATE DATABASE evantest +
WITH +
OWNER = chaol +
ENCODING = "UTF8" +
LC_COLLATE = "en_US.UTF-8"+
LC_CTYPE = "en_US.UTF-8" +
LOCALE_PROVIDER = 'libc' +
TABLESPACE = pg_default +
ALLOW_CONNECTIONS = true +
CONNECTION LIMIT = -1;
(1 row)
```I created the database “evantest” without providing any parameter. I
think
at least OWNER, TABLESPACE and ALLOW_CNONNECTIONS should be omitted.
For
CONNECTION LIMIT, you already have a logic to omit it but the logic has
some problem as comment 1.IMHO, parameters with default values *should not* be omitted from the
output of the pg_get_xxx_ddl functions. The primary purpose of these
functions is to accurately reconstruct the DDL. Including all parameters
ensures clarity, as not everyone is familiar with the default value of
every single parameter.+1
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
Thanks Álvaro
Will work on it and send the updated patch.
On Wed, Nov 19, 2025 at 4:17 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
Show quoted text
Hello,
One thing I realized a few days ago is that since commit bd09f024a1bb we
have type regdatabase, so instead of having two functions (one taking
name and one taking Oid), we should have just one, taking regdatabase,
just like the functions for producing DDL for other object types that
have corresponding reg* type.Regards,
--
Álvaro Herrera PostgreSQL Developer —
https://www.EnterpriseDB.com/
On Wed, 19 Nov 2025 at 16:36, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
On Wed, Nov 19, 2025 at 3:48 PM Japin Li <japinli@hotmail.com> wrote:
Hi Akshay,
Thanks for updating the patch.
On Tue, 18 Nov 2025 at 13:33, Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Chao
Thanks for reviewing my patch.
On Tue, Nov 18, 2025 at 5:59 AM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Akshay,
I just reviewed v3 and got some comments:
On Nov 17, 2025, at 22:34, Akshay Joshi <akshay.joshi@enterprisedb.com> 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.
Hi Álvaro,
On Wed, Nov 19, 2025 at 4:17 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
Hello,
One thing I realized a few days ago is that since commit bd09f024a1bb we
have type regdatabase, so instead of having two functions (one taking
name and one taking Oid), we should have just one, taking regdatabase,
just like the functions for producing DDL for other object types that
have corresponding reg* type.
Implemented in the suggested solution. Attached is the v5 patch for review.
Show quoted text
Regards,
--
Álvaro Herrera PostgreSQL Developer —
https://www.EnterpriseDB.com/