public inbox for [email protected]  
help / color / mirror / Atom feed
From: Haritabh Gupta <[email protected]>
To: [email protected]
Cc: Florin Irion <[email protected]>
Cc: Tim Waizenegger <[email protected]>
Subject: Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
Date: Tue, 27 Jan 2026 18:27:15 +0000
Message-ID: <176953843500.1247419.17388294117341687241.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <[email protected]>
References: <CAPgqM1V4LW2qiDLPsusb7s0kYbSDJjH5Tt+-ZzVmPU7xV0TJNQ@mail.gmail.com>
	<[email protected]>

Hello,
Please find below some comments (mostly minor ones):

1. We need to add the following comma in the docs change. so that it looks same as other functions:
diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml
index 25f87b78344..bc01c73f4ea 100644
--- a/doc/src/sgml/func/func-info.sgml
+++ b/doc/src/sgml/func/func-info.sgml
@@ -3861,7 +3861,7 @@ acl      | {postgres=arwdDxtm/postgres,foo=r/postgres}
          <primary>pg_get_domain_ddl</primary>
         </indexterm>
         <function>pg_get_domain_ddl</function> ( <parameter>domain</parameter> <type>regtype</type>
-         <optional> <parameter>pretty</parameter> <type>boolean</type> </optional>)
+         <optional>, <parameter>pretty</parameter> <type>boolean</type> </optional>)
         <returnvalue>text</returnvalue>
        </para>
        <para>


2. In the function signature there is `int prettyFlags` argument, while the doc suggests `pretty`:
+/*
+ * get_formatted_string
+ *
+ * Return a formatted version of the string.
+ *
+ * pretty - If pretty is true, the output includes tabs (\t) and newlines (\n).
+ * noOfTabChars - indent with specified no of tabs.
+ * fmt - printf-style format string used by appendStringInfoVA.
+ */
+static void
+get_formatted_string(StringInfo buf, int prettyFlags, int noOfTabChars, const char *fmt,...)


3. In a similar patch (https://www.postgresql.org/message-id/flat/CANxoLDdJsRJqnjMXV3yjsk07Z5iRWxG-c2hZJC7bAKqf8ZXj_A@mail....), author has defined a separate macro to make the usage of `GET_PRETTY_FLAGS` cleaner, We can use the same in function `pg_get_domain_ddl_ext`:
+#define GET_DDL_PRETTY_FLAGS(pretty) \
+    ((pretty) ? (PRETTYFLAG_PAREN | PRETTYFLAG_INDENT | PRETTYFLAG_SCHEMA) \
+     : 0)

+Datum
+pg_get_policy_ddl(PG_FUNCTION_ARGS)
+{
+    Oid            tableID = PG_GETARG_OID(0);
+    Name        policyName = PG_GETARG_NAME(1);
+    bool        pretty = PG_GETARG_BOOL(2);
+    int            prettyFlags;
+    char       *res;
+
+    prettyFlags = GET_DDL_PRETTY_FLAGS(pretty);

4. Usually the tests for the function to get the DDL definition of an object are present in the same testcase file where the `CREATE...` command exists, e.g. test for `pg_get_indexdef` exists in `create_index.sql` file. Similarly tests for `pg_get_functiondef` exists in `create_procedure.sql` file and so on. Currently in the patch, the tests for `pg_get_domain_ddl` are put in a new file `object_ddl.sql` but I guess it can be put in the existing file `domain.sql` because that is where the `CREATE DOMAIN...` tests reside.

view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] pg_get_domain_ddl: DDL reconstruction function for CREATE DOMAIN statement
  In-Reply-To: <176953843500.1247419.17388294117341687241.pgcf@coridan.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox