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-c2hzjc7bakqf8zx...@mail.gmail.com),
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.
