public inbox for [email protected]
help / color / mirror / Atom feedAdd pg_get_publication_ddl function
6+ messages / 5 participants
[nested] [flat]
* Add pg_get_publication_ddl function
@ 2026-01-11 11:20 Jonathan Gonzalez V. <[email protected]>
0 siblings, 3 replies; 6+ messages in thread
From: Jonathan Gonzalez V. @ 2026-01-11 11:20 UTC (permalink / raw)
To: [email protected]
Hi all!
Following the set of pg_get_{object}_ddl functions described by Andrew
Dustan[1] I'm attaching another patch that I created for the
PUBLICATION object, I had to left some stuff out and added others based
on other patches feedback for the same type of functions:
* Pretty print: There's a few patched that implemented the pretty print
using spaces or tabs, by default or not, etc. I just decided to keep
this out of this patch, the idea is to follow any decision that can
be made for all the patches at some point, and mostly probably have
a common set of functions to use in all these functions since there
will be a lot of them, and rewriting every time the same and
reviewing the same possible code may take some time and I think it's
better to have the base set of functions and later work on stuff like
pretty print.
* Testing: I've added a regress test with the name
`publiction_ddl.sql`, those, it's close to the publication object in
names on the list of files and not inside a big generic file with
many SQL for different objects. Those, we can keep a list of file in
a fixed section inside the file `parallel_schedule`.
The patch add the function, tests and documentation, but on the
documentation side I don't know if it should go anywhere else or if the
current location it's ok, I haven't found any decision on the that
specific subject.
Related to the required privileges, to CREATE or ALTER a PUBLICATION
you require a specific set of privileges, but there's no defined
privilege to "view" a publication, being really strict, this function
could leak information like columns and tables that exist inside the
database without even have permissions to see those tables, the same
will be for sequences in the future. This took me to the question, does
a kind of "view" privilege exists for these objects? Because you can
have access to the database or even some tables but not all of them at
the same time, and this could lead to a leak of data. I'm really new on
PostgreSQL development, but I couldn't find the "view" privilege that
will allow to read a PUBLICATION and check it, but in my opinion not
being allowed to create a publication doesn't mean that you cannot see
the content of it, am I missing a privilege here or if you can't create
you shouldn't be able to "view" a publication?
Related to the default values, there's a couple of things that I think
it should be there just because, either are always defined or to make
clear the value is there since it will affect the object:
* publish: this field contains a list of permissions [2] that are a
list of variables inside a struct with values always being set
* publish_generated_columns: the default is well defined and not adding
the default to the statement will make sense, but since the others
two will be there, for useful and educational purposes, this makes
sense to me.
* publish_via_partition_root: this boolean is always set, so adding it
make sense since it will make explicit to someone debugging a
publication how this will behave on a TRUNCATE situation.
Thank you in advance for the reviews and all the time on this!
[1]
https://www.postgresql.org/message-id/flat/945db7c5-be75-45bf-b55b-cb1e56f2e3e9%40dunslane.net
[2]
https://www.postgresql.org/docs/18/sql-createpublication.html#SQL-CREATEPUBLICATION-PARAMS-WITH
--
Jonathan Gonzalez V. <[email protected]>
EnterpriseDB
Attachments:
[text/x-patch] v1-0001-Introduce-a-new-function-pg_get_publication_ddl.patch (36.6K, 2-v1-0001-Introduce-a-new-function-pg_get_publication_ddl.patch)
download | inline diff:
From b7741d5f02cee5bef210f6975c850b70123f75a4 Mon Sep 17 00:00:00 2001
From: "Jonathan Gonzalez V." <[email protected]>
Date: Sun, 5 Oct 2025 18:11:56 +0200
Subject: [PATCH v1 1/1] Introduce a new function pg_get_publication_ddl() that
returns the CREATE ddl statement for a giving PUBLICATION.
The functions accepts a publication name or OID indistinctively,
thuse, provides an easy way to call the function as a user or from
inside the Postgres code or any SQL statement.
Comprhensive regression tests are included covering various
possible situations for a given publication.
Signed-off-by: Jonathan Gonzalez V. <[email protected]>
---
doc/src/sgml/func/func-info.sgml | 45 ++++
src/backend/utils/adt/ruleutils.c | 247 ++++++++++++++++++
src/include/catalog/pg_proc.dat | 8 +
src/test/regress/expected/publication_ddl.out | 211 +++++++++++++++
src/test/regress/parallel_schedule | 4 +
src/test/regress/sql/publication_ddl.sql | 121 +++++++++
6 files changed, 636 insertions(+)
create mode 100644 src/test/regress/expected/publication_ddl.out
create mode 100644 src/test/regress/sql/publication_ddl.sql
diff --git a/doc/src/sgml/func/func-info.sgml b/doc/src/sgml/func/func-info.sgml
index 175f18315cd..600534ff11f 100644
--- a/doc/src/sgml/func/func-info.sgml
+++ b/doc/src/sgml/func/func-info.sgml
@@ -3830,4 +3830,49 @@ acl | {postgres=arwdDxtm/postgres,foo=r/postgres}
</sect2>
+ <sect2 id="functions-get-object-ddl">
+ <title>Get Object DDL Functions</title>
+
+ <para>
+ The functions shown in <xref linkend="functions-get-object-ddl-table"/>
+ print the DDL statements for various database objects.
+ (This is a decompiled reconstruction, not the original text
+ of the command.)
+ </para>
+
+ <table id="functions-get-object-ddl-table">
+ <title>Get Object DDL Functions</title>
+ <tgroup cols="1">
+ <thead>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ Function
+ </para>
+ <para>
+ Description
+ </para></entry>
+ </row>
+ </thead>
+
+ <tbody>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_get_publication_ddl</primary>
+ </indexterm>
+ <function>pg_get_publication_ddl</function> ( <parameter>publication</parameter> <type>text</type> or <type>oid</type> )
+ <returnvalue>text</returnvalue>
+ </para>
+ <para>
+ Recreate the CREATE statement for a giving PUBLICATION.
+ The result is a complete <command>CREATE PUBLICATION</command> statement.
+ </para></entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ </sect2>
+
+
</sect1>
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 033b625f3fc..b4e06d3f883 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -30,10 +30,12 @@
#include "catalog/pg_constraint.h"
#include "catalog/pg_depend.h"
#include "catalog/pg_language.h"
+#include "catalog/pg_namespace_d.h"
#include "catalog/pg_opclass.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_partitioned_table.h"
#include "catalog/pg_proc.h"
+#include "catalog/pg_publication_rel.h"
#include "catalog/pg_statistic_ext.h"
#include "catalog/pg_trigger.h"
#include "catalog/pg_type.h"
@@ -546,6 +548,7 @@ static void get_json_table_nested_columns(TableFunc *tf, JsonTablePlan *plan,
deparse_context *context,
bool showimplicit,
bool needcomma);
+static Datum pg_do_publication_ddl(Publication *pub);
#define only_marker(rte) ((rte)->inh ? "" : "ONLY ")
@@ -13743,3 +13746,247 @@ get_range_partbound_string(List *bound_datums)
return buf.data;
}
+
+/*
+ * pg_do_publication_ddl
+ * Common code to get the DDL fro a PUBLICATION
+ * by pg_get_publication_ddl and pg_get_publication_ddl_name
+ */
+Datum
+pg_do_publication_ddl(Publication *pub)
+{
+ StringInfo buf;
+ List *publications;
+ List *pub_schemas;
+ bool first_perm;
+
+ if (pub == NULL)
+ return (Datum) NULL;
+
+ publications = GetPublicationRelations(pub->oid, PUBLICATION_PART_ALL);
+ pub_schemas = GetPublicationSchemas(pub->oid);
+
+ buf = makeStringInfo();
+
+ /*
+ * we add the publication name, this isn't covered by the schema, yet?
+ */
+ appendStringInfo(buf, "CREATE PUBLICATION %s", quote_identifier(pub->name));
+
+ /*
+ * having all tables or all sequence means there's not publications per
+ * table
+ */
+ if (pub->alltables || pub->allsequences)
+ {
+ appendStringInfo(buf, " FOR ");
+
+ if (pub->alltables)
+ appendStringInfo(buf, "ALL TABLES");
+
+ /* The sequence are published only on versions 19+ */
+ if (pub->allsequences)
+ appendStringInfo(buf,
+ "%sALL SEQUENCE",
+ pub->alltables ? ", " : " ");
+ }
+ else if (publications != NIL)
+ {
+
+ ListCell *pub_cell;
+ Relation rel;
+ char *schemaname = NULL;
+ char *tablename;
+ bool first = true;
+
+ appendStringInfo(buf, " FOR TABLE ");
+
+ /*
+ * publication an have relations of tables, in schema or current
+ * schema
+ *
+ */
+ foreach(pub_cell, publications)
+ {
+ HeapTuple pubtuple = NULL;
+ Datum columns;
+ Datum conditions;
+ bool cols_nulls,
+ condition_nulls;
+
+ /* Open the table relation for this publication */
+ rel = relation_open(pub_cell->oid_value, AccessShareLock);
+ tablename = RelationGetRelationName(rel);
+
+ if (RelationGetNamespace(rel) != PG_PUBLIC_NAMESPACE)
+ schemaname = get_namespace_name(RelationGetNamespace(rel));
+
+ appendStringInfo(buf, "%s%s",
+ first ? "" : ", ",
+ quote_qualified_identifier(schemaname, tablename));
+ first = false;
+
+ pubtuple = SearchSysCacheCopy2(PUBLICATIONRELMAP,
+ ObjectIdGetDatum(rel->rd_id),
+ ObjectIdGetDatum(pub->oid));
+ columns = SysCacheGetAttr(PUBLICATIONRELMAP, pubtuple,
+ Anum_pg_publication_rel_prattrs,
+ &cols_nulls);
+
+ conditions = SysCacheGetAttr(PUBLICATIONRELMAP, pubtuple,
+ Anum_pg_publication_rel_prqual,
+ &condition_nulls);
+
+ /* If no null, means we have a list of columns to publish */
+ if (!cols_nulls)
+ {
+ ArrayType *arr;
+ int ncolumns;
+ int16 *cols;
+ bool fcol = true;
+
+ arr = DatumGetArrayTypeP(columns);
+ ncolumns = ARR_DIMS(arr)[0];
+ cols = (int16 *) ARR_DATA_PTR(arr);
+
+ appendStringInfoChar(buf, '(');
+ for (int i = 0; i < ncolumns; i++)
+ {
+ appendStringInfo(buf, "%s%s",
+ fcol ? "" : ", ",
+ get_attname(rel->rd_id, cols[i], true));
+ fcol = false;
+ }
+ appendStringInfoChar(buf, ')');
+ }
+
+ /*
+ * If there's a condition goes after the columns but if there's no
+ * column we can have conditions too
+ */
+ if (!condition_nulls)
+ {
+ Node *node;
+ List *context;
+ char *str;
+
+ node = stringToNode(TextDatumGetCString(conditions));
+ context = deparse_context_for(
+ get_relation_name(rel->rd_id),
+ rel->rd_id);
+ str = deparse_expression_pretty(
+ node, context, false, false,
+ GET_PRETTY_FLAGS(false), 0);
+
+ appendStringInfo(buf, " WHERE %s ", str);
+ }
+
+ /* Close the relation */
+ relation_close(rel, AccessShareLock);
+ }
+ }
+
+ /* if we have schemas, for sure this will go right before the WITH */
+ if (pub_schemas != NIL)
+ {
+ ListCell *schema_cell;
+ bool first_schema = true;
+
+ /*
+ * schemas can be preceeded by a list of tables
+ */
+ appendStringInfo(buf, "%s TABLES IN SCHEMA",
+ publications == NIL ? " FOR" : ",");
+
+ /* schemas can be one or a list */
+ foreach(schema_cell, pub_schemas)
+ {
+ appendStringInfo(buf, "%s %s",
+ first_schema ? "" : ",",
+ quote_identifier(get_namespace_name(schema_cell->oid_value)));
+ first_schema = false;
+ }
+ }
+
+ /* always add the WITH options */
+ appendStringInfo(buf, " WITH (");
+
+ /* publish string */
+ appendStringInfo(buf, "publish='");
+
+ /*
+ * we need to know if we're the second permission added to prefix with a
+ * ", " string
+ */
+ first_perm = true;
+ if (pub->pubactions.pubinsert)
+ {
+ /*
+ * by precedence we know that the insert will always be first, no need
+ * to check previous values
+ */
+ appendStringInfo(buf, "insert");
+ first_perm = false;
+ }
+
+ if (pub->pubactions.pubupdate)
+ {
+ appendStringInfo(buf, "%supdate", first_perm ? "" : ", ");
+ first_perm = false;
+ }
+ if (pub->pubactions.pubdelete)
+ {
+ appendStringInfo(buf, "%sdelete", first_perm ? "" : ", ");
+ first_perm = false;
+ }
+
+ if (pub->pubactions.pubtruncate)
+ {
+ appendStringInfo(buf, "%struncate", first_perm ? "" : ", ");
+ }
+
+ appendStringInfo(buf, "', ");
+
+ /* publish_generated_columns string */
+ appendStringInfo(buf, "publish_generated_columns='%s', ",
+ pub->pubgencols_type == PUBLISH_GENCOLS_NONE ? "none" : "stored");
+
+ /* publish_via_partition_root value */
+ appendStringInfo(buf, "publish_via_partition_root='%s')",
+ pub->pubviaroot ? "true" : "false");
+
+ /* the statement should always finish with a ; */
+ appendStringInfoChar(buf, ';');
+
+ PG_RETURN_TEXT_P(string_to_text(buf->data));
+}
+
+/*
+ * pg_get_publication_ddl_name
+ * Get the CREATE statement for PUBLICATION by name
+ */
+Datum
+pg_get_publication_ddl_name(PG_FUNCTION_ARGS)
+{
+ char *pub_name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ Publication *pub;
+
+ pub = GetPublicationByName(pub_name, false);
+
+ return pg_do_publication_ddl(pub);
+}
+
+/*
+ * pg_get_publication_ddl
+ * Get the CREATE statement for PUBLICATION by OID
+ */
+Datum
+pg_get_publication_ddl(PG_FUNCTION_ARGS)
+{
+ Oid pub_oid = PG_GETARG_OID(0);
+ Publication *pub;
+
+ pub = GetPublication(pub_oid);
+
+ return pg_do_publication_ddl(pub);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 2ac69bf2df5..7bbb1d2751f 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12336,6 +12336,14 @@
proname => 'pg_relation_is_publishable', provolatile => 's',
prorettype => 'bool', proargtypes => 'regclass',
prosrc => 'pg_relation_is_publishable' },
+{ oid => '6122',
+ descr => 'returns the create statement for a publication',
+ proname => 'pg_get_publication_ddl', prorettype => 'text',
+ proargtypes => 'oid', prosrc => 'pg_get_publication_ddl' },
+{ oid => '6123',
+ descr => 'returns the create statement for a publication',
+ proname => 'pg_get_publication_ddl', prorettype => 'text',
+ proargtypes => 'text', prosrc => 'pg_get_publication_ddl_name' },
# rls
{ oid => '3298',
diff --git a/src/test/regress/expected/publication_ddl.out b/src/test/regress/expected/publication_ddl.out
new file mode 100644
index 00000000000..9d162c07d53
--- /dev/null
+++ b/src/test/regress/expected/publication_ddl.out
@@ -0,0 +1,211 @@
+--
+-- Test for DDL statement from:
+-- - pg_get_publication_ddl
+--
+-- supress warning that depends on wal_level
+SET client_min_messages = 'ERROR';
+-- empty publication is possible and allowed
+CREATE PUBLICATION testpub_ddl_1;
+SELECT pg_get_publication_ddl('testpub_ddl_1');
+ pg_get_publication_ddl
+-----------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_1 WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_1'));
+ pg_get_publication_ddl
+-----------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_1 WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create base table to test basic table publication
+CREATE TABLE testpub_ddl_tbl1 (foo int, bar int);
+CREATE TABLE testpub_ddl_tbl2 (foo int, bar int);
+CREATE TABLE testpub_ddl_tbl3 (foo int, bar int, beque int, baz int);
+CREATE TABLE testpub_ddl_tbl4 (foo int, bar int, beque bool);
+CREATE PUBLICATION testpub_ddl_2 FOR TABLE testpub_ddl_tbl1, testpub_ddl_tbl2, testpub_ddl_tbl3 WITH (publish='delete', publish_generated_columns='stored', publish_via_partition_root='true');
+SELECT pg_get_publication_ddl('testpub_ddl_2');
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_2 FOR TABLE testpub_ddl_tbl1, testpub_ddl_tbl2, testpub_ddl_tbl3 WITH (publish='delete', publish_generated_columns='stored', publish_via_partition_root='true');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_2'));
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_2 FOR TABLE testpub_ddl_tbl1, testpub_ddl_tbl2, testpub_ddl_tbl3 WITH (publish='delete', publish_generated_columns='stored', publish_via_partition_root='true');
+(1 row)
+
+ALTER PUBLICATION testpub_ddl_2 SET (publish = 'delete, update');
+SELECT pg_get_publication_ddl('testpub_ddl_2');
+ pg_get_publication_ddl
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_2 FOR TABLE testpub_ddl_tbl1, testpub_ddl_tbl2, testpub_ddl_tbl3 WITH (publish='update, delete', publish_generated_columns='stored', publish_via_partition_root='true');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_2'));
+ pg_get_publication_ddl
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_2 FOR TABLE testpub_ddl_tbl1, testpub_ddl_tbl2, testpub_ddl_tbl3 WITH (publish='update, delete', publish_generated_columns='stored', publish_via_partition_root='true');
+(1 row)
+
+-- create publication for one table with one column and a condition
+CREATE PUBLICATION testpub_ddl_3 FOR TABLE ONLY testpub_ddl_tbl1 (bar) WHERE (bar = 1);
+SELECT pg_get_publication_ddl('testpub_ddl_3');
+ pg_get_publication_ddl
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_3 FOR TABLE testpub_ddl_tbl1(bar) WHERE (bar = 1) WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_3'));
+ pg_get_publication_ddl
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_3 FOR TABLE testpub_ddl_tbl1(bar) WHERE (bar = 1) WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create publication for one table with two columns and a condition
+CREATE PUBLICATION testpub_ddl_4 FOR TABLE ONLY testpub_ddl_tbl3 (bar,baz) WHERE (bar = baz);
+SELECT pg_get_publication_ddl('testpub_ddl_4');
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_4 FOR TABLE testpub_ddl_tbl3(bar, baz) WHERE (bar = baz) WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_4'));
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_4 FOR TABLE testpub_ddl_tbl3(bar, baz) WHERE (bar = baz) WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create publication for one table with two columns and a condition with an expresion
+CREATE PUBLICATION testpub_ddl_5 FOR TABLE ONLY testpub_ddl_tbl4 (bar,beque) WHERE (beque IS TRUE);
+SELECT pg_get_publication_ddl('testpub_ddl_5');
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_5 FOR TABLE testpub_ddl_tbl4(bar, beque) WHERE (beque IS TRUE) WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_5'));
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_5 FOR TABLE testpub_ddl_tbl4(bar, beque) WHERE (beque IS TRUE) WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create publication for all tables
+CREATE PUBLICATION testpub_ddl_6 FOR ALL TABLES;
+SELECT pg_get_publication_ddl('testpub_ddl_6');
+ pg_get_publication_ddl
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_6 FOR ALL TABLES WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_6'));
+ pg_get_publication_ddl
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_6 FOR ALL TABLES WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create publation for all tables and all sequences
+CREATE PUBLICATION testpub_ddl_7 FOR ALL TABLES, ALL SEQUENCES;
+SELECT pg_get_publication_ddl('testpub_ddl_7');
+ pg_get_publication_ddl
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_7 FOR ALL TABLES, ALL SEQUENCE WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_7'));
+ pg_get_publication_ddl
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_7 FOR ALL TABLES, ALL SEQUENCE WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create schema for schema publication
+CREATE SCHEMA pub_schema_test_ddl;
+CREATE TABLE pub_schema_test_ddl.schema_tbl1 (foo int, bar int);
+CREATE TABLE pub_schema_test_ddl.schema_tbl2 (foo int, bar int);
+CREATE TABLE pub_schema_test_ddl.schema_tbl3 (foo int, bar int, baz int);
+-- create a publication for a list of tables
+CREATE PUBLICATION testpub_ddl_8 FOR TABLES IN SCHEMA pub_schema_test_ddl, TABLE pub_schema_test_ddl.schema_tbl1, pub_schema_test_ddl.schema_tbl2;
+SELECT pg_get_publication_ddl('testpub_ddl_8');
+ pg_get_publication_ddl
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_8 FOR TABLE pub_schema_test_ddl.schema_tbl1, pub_schema_test_ddl.schema_tbl2, TABLES IN SCHEMA pub_schema_test_ddl WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_8'));
+ pg_get_publication_ddl
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_8 FOR TABLE pub_schema_test_ddl.schema_tbl1, pub_schema_test_ddl.schema_tbl2, TABLES IN SCHEMA pub_schema_test_ddl WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create publication in schema only for table
+CREATE PUBLICATION testpub_ddl_9 FOR TABLES IN SCHEMA pub_schema_test_ddl, TABLE pub_schema_test_ddl.schema_tbl1;
+SELECT pg_get_publication_ddl('testpub_ddl_9');
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_9 FOR TABLE pub_schema_test_ddl.schema_tbl1, TABLES IN SCHEMA pub_schema_test_ddl WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_9'));
+ pg_get_publication_ddl
+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_9 FOR TABLE pub_schema_test_ddl.schema_tbl1, TABLES IN SCHEMA pub_schema_test_ddl WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- create publication for all tables in schema
+CREATE PUBLICATION testpub_ddl_10 FOR TABLES IN SCHEMA pub_schema_test_ddl;
+SELECT pg_get_publication_ddl('testpub_ddl_10');
+ pg_get_publication_ddl
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_10 FOR TABLES IN SCHEMA pub_schema_test_ddl WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_10'));
+ pg_get_publication_ddl
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_10 FOR TABLES IN SCHEMA pub_schema_test_ddl WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- a new schema for multiple schemas
+CREATE SCHEMA pub_schema_test_ddl_2;
+CREATE TABLE pub_schema_test_ddl_2.schema_tbl1 (foo int, bar int);
+-- create a publication for a list of schemas
+CREATE PUBLICATION testpub_ddl_11 FOR TABLES IN SCHEMA pub_schema_test_ddl, pub_schema_test_ddl_2;
+SELECT pg_get_publication_ddl('testpub_ddl_11');
+ pg_get_publication_ddl
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_11 FOR TABLES IN SCHEMA pub_schema_test_ddl, pub_schema_test_ddl_2 WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_11'));
+ pg_get_publication_ddl
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ CREATE PUBLICATION testpub_ddl_11 FOR TABLES IN SCHEMA pub_schema_test_ddl, pub_schema_test_ddl_2 WITH (publish='insert, update, delete, truncate', publish_generated_columns='none', publish_via_partition_root='false');
+(1 row)
+
+-- cleanup publications
+DROP PUBLICATION testpub_ddl_1;
+DROP PUBLICATION testpub_ddl_2;
+DROP PUBLICATION testpub_ddl_3;
+DROP PUBLICATION testpub_ddl_4;
+DROP PUBLICATION testpub_ddl_5;
+DROP PUBLICATION testpub_ddl_6;
+DROP PUBLICATION testpub_ddl_7;
+DROP PUBLICATION testpub_ddl_8;
+DROP PUBLICATION testpub_ddl_9;
+DROP PUBLICATION testpub_ddl_10;
+DROP PUBLICATION testpub_ddl_11;
+-- cleanup tables
+DROP TABLE testpub_ddl_tbl1;
+DROP TABLE testpub_ddl_tbl2;
+DROP TABLE testpub_ddl_tbl3;
+DROP TABLE testpub_ddl_tbl4;
+-- cleanup tables in schemas
+DROP TABLE pub_schema_test_ddl.schema_tbl1;
+DROP TABLE pub_schema_test_ddl.schema_tbl2;
+DROP TABLE pub_schema_test_ddl.schema_tbl3;
+DROP TABLE pub_schema_test_ddl_2.schema_tbl1;
+-- cleanup schemas
+DROP SCHEMA pub_schema_test_ddl;
+DROP SCHEMA pub_schema_test_ddl_2;
+RESET client_min_messages;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 021d57f66bb..ce5351759b4 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -137,6 +137,10 @@ test: event_trigger_login
# this test also uses event triggers, so likewise run it by itself
test: fast_default
+# run the retail DDL tests at the end make sense to not interrupt with other
+# tests in case the object names are the same to other objects previously used.
+test: publication_ddl
+
# run tablespace test at the end because it drops the tablespace created during
# setup that other tests may use.
test: tablespace
diff --git a/src/test/regress/sql/publication_ddl.sql b/src/test/regress/sql/publication_ddl.sql
new file mode 100644
index 00000000000..e93bcf70692
--- /dev/null
+++ b/src/test/regress/sql/publication_ddl.sql
@@ -0,0 +1,121 @@
+--
+-- Test for DDL statement from:
+-- - pg_get_publication_ddl
+--
+
+-- supress warning that depends on wal_level
+SET client_min_messages = 'ERROR';
+-- empty publication is possible and allowed
+CREATE PUBLICATION testpub_ddl_1;
+
+SELECT pg_get_publication_ddl('testpub_ddl_1');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_1'));
+
+-- create base table to test basic table publication
+CREATE TABLE testpub_ddl_tbl1 (foo int, bar int);
+CREATE TABLE testpub_ddl_tbl2 (foo int, bar int);
+CREATE TABLE testpub_ddl_tbl3 (foo int, bar int, beque int, baz int);
+CREATE TABLE testpub_ddl_tbl4 (foo int, bar int, beque bool);
+
+CREATE PUBLICATION testpub_ddl_2 FOR TABLE testpub_ddl_tbl1, testpub_ddl_tbl2, testpub_ddl_tbl3 WITH (publish='delete', publish_generated_columns='stored', publish_via_partition_root='true');
+
+SELECT pg_get_publication_ddl('testpub_ddl_2');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_2'));
+
+ALTER PUBLICATION testpub_ddl_2 SET (publish = 'delete, update');
+
+SELECT pg_get_publication_ddl('testpub_ddl_2');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_2'));
+
+-- create publication for one table with one column and a condition
+
+CREATE PUBLICATION testpub_ddl_3 FOR TABLE ONLY testpub_ddl_tbl1 (bar) WHERE (bar = 1);
+
+SELECT pg_get_publication_ddl('testpub_ddl_3');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_3'));
+
+-- create publication for one table with two columns and a condition
+
+CREATE PUBLICATION testpub_ddl_4 FOR TABLE ONLY testpub_ddl_tbl3 (bar,baz) WHERE (bar = baz);
+
+SELECT pg_get_publication_ddl('testpub_ddl_4');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_4'));
+
+-- create publication for one table with two columns and a condition with an expresion
+
+CREATE PUBLICATION testpub_ddl_5 FOR TABLE ONLY testpub_ddl_tbl4 (bar,beque) WHERE (beque IS TRUE);
+
+SELECT pg_get_publication_ddl('testpub_ddl_5');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_5'));
+
+-- create publication for all tables
+CREATE PUBLICATION testpub_ddl_6 FOR ALL TABLES;
+
+SELECT pg_get_publication_ddl('testpub_ddl_6');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_6'));
+
+-- create publation for all tables and all sequences
+CREATE PUBLICATION testpub_ddl_7 FOR ALL TABLES, ALL SEQUENCES;
+
+SELECT pg_get_publication_ddl('testpub_ddl_7');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_7'));
+
+-- create schema for schema publication
+CREATE SCHEMA pub_schema_test_ddl;
+CREATE TABLE pub_schema_test_ddl.schema_tbl1 (foo int, bar int);
+CREATE TABLE pub_schema_test_ddl.schema_tbl2 (foo int, bar int);
+CREATE TABLE pub_schema_test_ddl.schema_tbl3 (foo int, bar int, baz int);
+
+-- create a publication for a list of tables
+CREATE PUBLICATION testpub_ddl_8 FOR TABLES IN SCHEMA pub_schema_test_ddl, TABLE pub_schema_test_ddl.schema_tbl1, pub_schema_test_ddl.schema_tbl2;
+SELECT pg_get_publication_ddl('testpub_ddl_8');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_8'));
+
+-- create publication in schema only for table
+CREATE PUBLICATION testpub_ddl_9 FOR TABLES IN SCHEMA pub_schema_test_ddl, TABLE pub_schema_test_ddl.schema_tbl1;
+SELECT pg_get_publication_ddl('testpub_ddl_9');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_9'));
+
+-- create publication for all tables in schema
+CREATE PUBLICATION testpub_ddl_10 FOR TABLES IN SCHEMA pub_schema_test_ddl;
+SELECT pg_get_publication_ddl('testpub_ddl_10');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_10'));
+
+-- a new schema for multiple schemas
+CREATE SCHEMA pub_schema_test_ddl_2;
+CREATE TABLE pub_schema_test_ddl_2.schema_tbl1 (foo int, bar int);
+
+-- create a publication for a list of schemas
+CREATE PUBLICATION testpub_ddl_11 FOR TABLES IN SCHEMA pub_schema_test_ddl, pub_schema_test_ddl_2;
+SELECT pg_get_publication_ddl('testpub_ddl_11');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE pubname='testpub_ddl_11'));
+
+-- cleanup publications
+DROP PUBLICATION testpub_ddl_1;
+DROP PUBLICATION testpub_ddl_2;
+DROP PUBLICATION testpub_ddl_3;
+DROP PUBLICATION testpub_ddl_4;
+DROP PUBLICATION testpub_ddl_5;
+DROP PUBLICATION testpub_ddl_6;
+DROP PUBLICATION testpub_ddl_7;
+DROP PUBLICATION testpub_ddl_8;
+DROP PUBLICATION testpub_ddl_9;
+DROP PUBLICATION testpub_ddl_10;
+DROP PUBLICATION testpub_ddl_11;
+
+-- cleanup tables
+DROP TABLE testpub_ddl_tbl1;
+DROP TABLE testpub_ddl_tbl2;
+DROP TABLE testpub_ddl_tbl3;
+DROP TABLE testpub_ddl_tbl4;
+
+-- cleanup tables in schemas
+DROP TABLE pub_schema_test_ddl.schema_tbl1;
+DROP TABLE pub_schema_test_ddl.schema_tbl2;
+DROP TABLE pub_schema_test_ddl.schema_tbl3;
+DROP TABLE pub_schema_test_ddl_2.schema_tbl1;
+
+-- cleanup schemas
+DROP SCHEMA pub_schema_test_ddl;
+DROP SCHEMA pub_schema_test_ddl_2;
+RESET client_min_messages;
--
2.51.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re:Add pg_get_publication_ddl function
@ 2026-01-11 12:18 =?gb18030?B?emVuZ21hbg==?= <[email protected]>
parent: Jonathan Gonzalez V. <[email protected]>
2 siblings, 0 replies; 6+ messages in thread
From: =?gb18030?B?emVuZ21hbg==?= @ 2026-01-11 12:18 UTC (permalink / raw)
To: =?gb18030?B?Sm9uYXRoYW4gR29uemFsZXogVi4=?= <[email protected]>; =?gb18030?B?cGdzcWwtaGFja2Vycw==?= <[email protected]>
Hi,
I haven't tested your patch yet, but I noticed a few points that may need adjustment:
Based on your code logic, the pub parameter passed here can never be NULL — if the corresponding PUBLICATION does not exist, an error should have already been thrown earlier in the code flow.
Therefore, the following code block can be removed, and the usage of return (Datum) NULL; in this block is also incorrect:
```
if (pub == NULL)
return (Datum) NULL;
```
pubtuple is not being freed — please use heap_freetuple to release it.
The format string "%sALL SEQUENCE" should be corrected to "%sALL SEQUENCES".
--
Regards,
Man Zeng
www.openhalo.org
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Add pg_get_publication_ddl function
@ 2026-02-27 07:53 Hüseyin Demir <[email protected]>
parent: Jonathan Gonzalez V. <[email protected]>
2 siblings, 0 replies; 6+ messages in thread
From: Hüseyin Demir @ 2026-02-27 07:53 UTC (permalink / raw)
To: [email protected]; +Cc: Jonathan Gonzalez V. <[email protected]>
Hi,
Thanks for the patch. I reviewed v1 and found a few issues that need
to be addressed before commit.
1. In pg_do_publication_ddl(), schemaname is initialized to NULL before
the loop but never reset per iteration. When the first table is in a
non-default schema and the second is in public, the public table
incorrectly inherits the previous schema prefix:
```
CREATE SCHEMA s1;
CREATE TABLE s1.t1 (id int);
CREATE TABLE public.t2 (id int);
CREATE PUBLICATION p FOR TABLE s1.t1, public.t2;
SELECT pg_get_publication_ddl('p');
-- Got: ... FOR TABLE s1.t1, s1.t2 ← WRONG
-- Want: ... FOR TABLE s1.t1, t2
```
2. FOR TABLE ONLY is silently dropped, changing semantics for publications
on tables with inheritance or partitioning:
```
CREATE PUBLICATION p FOR TABLE ONLY t (col) WHERE (col > 0);
SELECT pg_get_publication_ddl('p');
-- Got: FOR TABLE t(col) WHERE (col > 0) WITH ...
-- Want: FOR TABLE ONLY t(col) WHERE (col > 0) WITH ...
```
As already mentioned there are also typos in the code. Otherwise, the idea and way lgtm.
Regards.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Add pg_get_publication_ddl function
@ 2026-05-20 09:39 Peter Smith <[email protected]>
parent: Jonathan Gonzalez V. <[email protected]>
2 siblings, 2 replies; 6+ messages in thread
From: Peter Smith @ 2026-05-20 09:39 UTC (permalink / raw)
To: Jonathan Gonzalez V. <[email protected]>; +Cc: [email protected]
Hi.
Some review comments for the v2-0001 patch.
======
doc/src/sgml/func/func-info.sgml
(9.28.13. Get Object DDL Functions)
1.
+ <para role="func_signature">
+ <function>pg_get_publication_ddl</function>
+ ( <parameter>publication</parameter> <type>text</type>
+ <optional>, <literal>VARIADIC</literal> <parameter>options</parameter>
+ <type>text</type> </optional> )
+ <returnvalue>setof text</returnvalue>
+ </para>
I think "pubname" might be a more meaningful name for the first parameter.
~~~
2.
+ <para>
+ Reconstructs the <command>CREATE PUBLICATION</command> statement for
+ the specified publication (by OID or name), followed by an
+ <command>ALTER PUBLICATION ... OWNER TO</command> statement (the
+ <command>CREATE PUBLICATION</command> grammar has no
+ <literal>OWNER</literal> clause). Each statement is returned as a
+ separate row. An error is raised if no publication with the supplied
+ OID or name exists. When the publication was created with
+ <literal>FOR ALL TABLES, ALL SEQUENCES</literal>, the emitted
+ statement always lists <literal>ALL TABLES</literal> before
+ <literal>ALL SEQUENCES</literal> regardless of the original order.
+ The following options are supported:
+ <literal>pretty</literal> (boolean) for formatted output and
+ <literal>owner</literal> (boolean) to include
+ <literal>OWNER</literal>.
+ </para></entry>
2a.
That "CREATE PUBLICATION" should <link> back to the CREATE PUBLICATION
docs page.
~
2b.
It is overkill to mention about the potential reordering of ALL TABLES
and ALL SEQUENCES.
Apart from being unnecessary, there are many other things can also be
rearranged which are not mentioned:
- TABLES and ALL TABLES IN SCHEMA clauses might be different order
than specified
- The publication parameters might be in a different order than specified
- The values of 'publish' parameter might be different order than specified
- etc.
~~~
GENERAL
3.
It would be better if the the rows of "Table 9.96" were in alphabetical order.
======
src/backend/utils/adt/ddlutils.c
pg_get_publication_ddl_internal:
4.
+ if (pub->allsequences)
+ appendStringInfo(buf,
+ "%sALL SEQUENCES",
+ pub->alltables ? ", " : "");
Maybe better to avoid tricky format strings.
SUGGESTION
if (pub->allsequences)
{
if (pub->alltables)
appendStringInfo(buf, ", ");
appendStringInfo("ALL SEQUENCES");
}
~~~
5.
+ if (pub_incl_relids != NIL)
+ {
+ ListCell *pub_cell;
+ char *schemaname = NULL;
+ char *tablename;
+
+ append_ddl_option(buf, pretty, 4, "FOR TABLE ");
+
+ /*
+ * Publication can have table relations
+ */
+ foreach(pub_cell, pub_incl_relids)
Maybe that comment belongs earlier (above the if).
~~~
6.
+ appendStringInfo(buf, "%s%s",
+ foreach_current_index(pub_cell) > 0 ? ", " : "",
+ quote_qualified_identifier(schemaname, tablename));
Another place where avoiding a tricky format string may be tidier.
SUGGESTION
if (foreach_current_index(pub_cell) > 0)
appendStringInfo(buf, ", ");
appendStringInfo(buf, "%s", quote_qualified_identifier(schemaname, tablename));
~~~
7.
+ pubtuple = SearchSysCache2(PUBLICATIONRELMAP, ObjectIdGetDatum(relid),
+ ObjectIdGetDatum(pub->oid));
+
+ if (!HeapTupleIsValid(pubtuple))
+ elog(ERROR,
+ "cache lookup failed for publication relation %u in publication %u",
+ relid, pub->oid);
7a.
Maybe blank line here is not wanted.
~
7b.
Don't need to say "publication" 2x.
/publication relation/relation/
~~~
8.
+ /* If non-null, we have a list of columns to publish */
+ if (!cols_nulls)
SUGGESTION FOR THE COMMENT
Does this table specify a column-list?
~~~
9.
+ appendStringInfo(buf, "%s%s",
+ bms_member_index(attmap, attnum) ? ", " : "",
+ quote_identifier(get_attname(relid, attnum, true)));
Another place where avoiding a tricky format string may be tidier.
(change similar to previous review comments)
~~~
10.
+ /*
+ * If there is a condition it goes after the columns. We can have
+ * conditions without columns as well.
+ */
+ if (!condition_nulls)
10a.
The earlier assignment to 'conditions' should be moved to be directly
above here.
~
10b.
BTW, it is called a "row filter" so maybe it is better to refer to
that in the comments/vars instead of the generic sounding "condition".
~~~
11.
+ /* If we have schemas, they will go right before the WITH */
The kind of comments that just say "this-goes-after-that" or
"this-goes-after-that" are not very useful, because it is obvious from
the code logic that some appendStringInfo comes before or after
another one.
~~~
12.
+ /*
+ * Schemas can be preceded by a list of tables. When they are, the
+ * "TABLES IN SCHEMA" stays inline as a continuation of the existing
+ * FOR clause; otherwise it starts the FOR clause on its own line in
+ * pretty mode.
+ */
IMO it would be better for the FOR TABLE IN SCHEMA to come *before*
the specific tables in FOR TABLE.
e.g. For the case when there are specified tables "absorbed" into the
same named schemas I think it is more natural to see the schemas
first.
CREATE PUBLICATION mypub FOR TABLES IN SCHEMA s, TABLE s.t1;
~~~
13.
+ appendStringInfo(buf, "%s %s",
+ foreach_current_index(schema_cell) > 0 ? "," : "",
+ quote_identifier(nspname));
Another place where avoiding a tricky format string may be tidier.
(change similar to previous review comments)
~~~
14.
+ if (pub_excl_relids != NIL)
+ {
+ ListCell *excl_cell;
+ char *schemaname = NULL;
+
+ appendStringInfoString(buf, " EXCEPT (TABLE ");
The EXCEPT clause is currently permitted only with FOR ALL TABLES, so
it would be better moving this to earlier in this function where
pub->alltables was handled.
~~~
15.
+ appendStringInfo(buf, "%s%s",
+ foreach_current_index(excl_cell) > 0 ? ", " : "",
+ quote_qualified_identifier(schemaname, NameStr(reltup->relname)));
Another place where avoiding a tricky format string may be tidier.
(change similar to previous review comments)
~~~
16.
+ /*
+ * We need to know if we're the second permission added to prefix with a
+ * ", " string
+ */
+ if (pub->pubactions.pubinsert)
+ {
+ /*
+ * By precedence we know that the insert will always be first, no need
+ * to check previous values
+ */
+ appendStringInfoString(buf, "insert");
Both these comments are doing little more than just saying the same as
the code. IMO they are not needed.
~~~
17.
+ if (pub->pubactions.pubinsert)
+ {
+ /*
+ * By precedence we know that the insert will always be first, no need
+ * to check previous values
+ */
+ appendStringInfoString(buf, "insert");
+ first_perm = false;
+ }
+
+ if (pub->pubactions.pubupdate)
+ {
+ appendStringInfo(buf, "%supdate", first_perm ? "" : ", ");
+ first_perm = false;
+ }
+ if (pub->pubactions.pubdelete)
+ {
+ appendStringInfo(buf, "%sdelete", first_perm ? "" : ", ");
+ first_perm = false;
+ }
+
+ if (pub->pubactions.pubtruncate)
+ {
+ appendStringInfo(buf, "%struncate", first_perm ? "" : ", ");
+ }
+
17a.
There are some random blank lines that seem unnecessary.
~
17b.
IMO it is tidier to simply append the string you want, instead of
using a trick format string.
SUGGESTION (compare with patch)
if (pub->pubactions.pubinsert)
{
appendStringInfoString(buf, "insert");
first_perm = false;
}
if (pub->pubactions.pubupdate)
{
appendStringInfo(buf, first_perm ? "update" : ",update");
first_perm = false;
}
if (pub->pubactions.pubdelete)
{
appendStringInfo(buf, first_perm ? "delete" : ",delete");
first_perm = false;
}
if (pub->pubactions.pubtruncate)
{
appendStringInfo(buf, first_perm ? "truncate" : ",truncate");
}
======
src/test/regress/expected/publication_ddl.out
18.
+ CREATE PUBLICATION testpub_ddl_1
+
+ WITH (publish='insert, update, delete, truncate',
publish_generated_columns='none', publish_via_partition_root='false');
~
This "pretty" output appears to have "+" garbage in it. What's that
about -- it looks like some sort of line continuation character? Can
it be removed?
~~~
19.
The "pretty" output might be improved if each of the publication
options was on a new line.
~~~
20.
The generated boolean values (e.g. 'true'/'false') do not need to be quoted.
======
src/test/regress/sql/publication_ddl.sql
(Here are lots of test review comments; the first group are are
general so might apply to multiple test cases).
21.
I think you can create all the necessary schema and tables together
up-front instead of scatering them through the file.
~~~
22.
Make use of the proper publication teminology like "Column Lists" and
"Row Filters" instead of vague
"columns" and "conditions".
~~~
23.
Here is an idea:
Instead of having dozens of test publications, just have 1 test
publication, which you CREATE/DROP for each test case.
Then, since there is a fixed name publication (e.g. "mypub") for
everything, you can make a subroutine to encapsulate the common code:
+SELECT pg_get_publication_ddl('mypub');
+SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE
pubname='mypub'));
+SELECT pg_get_publication_ddl('mypub', 'pretty', 'true');
It means your test .sql file can become much shorter/simpler I think.
~~~
24.
There is duplication of some tests:
e.g.
+-- columns in publication must be quoted
and
+-- identifiers that require quoting: publication, schema, table and column
~~~
25.
It is not needed to quote the booleans 'true'/'false' for the options.
//////
26.
+-- create base table to test basic table publication
What does "basic table publication" mean? I expect it means different
things to different people. Better to be explicit about what this is
really testing.
~~~
27.
+-- create publication for one table with two columns and a condition
with an expression
What does "with an expression" mean? All Row-Filters are expressions
aren't they?
~~~
28.
+-- create a publication for a list of tables
Not really describing what this test is doing, which is mixing FOR
TABLE and FOR TABLES IN SCHEMA.
~~~
29.
+CREATE PUBLICATION testpub_ddl_part3 FOR TABLE testpub_ddl_part WITH
(publish_via_partition_root='true');
I guess it make no difference since these are only DDL syntax tests,
but it didn't really make much sense to set
publish_via_partition_root=true when testpub_ddl_part is the ROOT
anyway.
~~~
30.
+-- create publication for all tables except two tables
Actually this is also combining with an ALL SEQUENCES test.
~~~
31.
+-- cleanup publications
+DROP PUBLICATION testpub_ddl_1;
+DROP PUBLICATION testpub_ddl_2;
+DROP PUBLICATION testpub_ddl_3;
+DROP PUBLICATION testpub_ddl_4;
+DROP PUBLICATION testpub_ddl_5;
+DROP PUBLICATION testpub_ddl_6;
+DROP PUBLICATION testpub_ddl_7;
+DROP PUBLICATION testpub_ddl_8;
+DROP PUBLICATION testpub_ddl_9;
+DROP PUBLICATION testpub_ddl_10;
+DROP PUBLICATION testpub_ddl_schema_1;
+DROP PUBLICATION testpub_ddl_schema_2;
+DROP PUBLICATION testpub_ddl_schema_3;
+DROP PUBLICATION testpub_ddl_schema_4;
+DROP PUBLICATION testpub_ddl_schema_5;
+DROP PUBLICATION testpub_ddl_part1;
+DROP PUBLICATION testpub_ddl_part2;
+DROP PUBLICATION testpub_ddl_part3;
+DROP PUBLICATION testpub_ddl_part4;
+DROP PUBLICATION "Quoted Pub";
+DROP PUBLICATION testpub_ddl_except1;
+DROP PUBLICATION testpub_ddl_except2;
As per earlier review comment IMO it would make the test code simpler
to have just 1 publication that you CREATE/DROP om the fly.
~~~
32.
+-- cleanup tables in schemas
Not sure why this is done separately. Probably easier just to drop the
schemas with CASCADE so their tables will be auto-deleted.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Add pg_get_publication_ddl function
@ 2026-05-20 10:00 Japin Li <[email protected]>
parent: Peter Smith <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Japin Li @ 2026-05-20 10:00 UTC (permalink / raw)
To: Peter Smith <[email protected]>; +Cc: Jonathan Gonzalez V. <[email protected]>; [email protected]
On Wed, 20 May 2026 at 19:39, Peter Smith <[email protected]> wrote:
> Hi.
>
> Some review comments for the v2-0001 patch.
>
> ======
> doc/src/sgml/func/func-info.sgml
>
> (9.28.13. Get Object DDL Functions)
>
> 1.
> + <para role="func_signature">
> + <function>pg_get_publication_ddl</function>
> + ( <parameter>publication</parameter> <type>text</type>
> + <optional>, <literal>VARIADIC</literal> <parameter>options</parameter>
> + <type>text</type> </optional> )
> + <returnvalue>setof text</returnvalue>
> + </para>
>
> I think "pubname" might be a more meaningful name for the first parameter.
>
> ~~~
>
> 2.
> + <para>
> + Reconstructs the <command>CREATE PUBLICATION</command> statement for
> + the specified publication (by OID or name), followed by an
> + <command>ALTER PUBLICATION ... OWNER TO</command> statement (the
> + <command>CREATE PUBLICATION</command> grammar has no
> + <literal>OWNER</literal> clause). Each statement is returned as a
> + separate row. An error is raised if no publication with the supplied
> + OID or name exists. When the publication was created with
> + <literal>FOR ALL TABLES, ALL SEQUENCES</literal>, the emitted
> + statement always lists <literal>ALL TABLES</literal> before
> + <literal>ALL SEQUENCES</literal> regardless of the original order.
> + The following options are supported:
> + <literal>pretty</literal> (boolean) for formatted output and
> + <literal>owner</literal> (boolean) to include
> + <literal>OWNER</literal>.
> + </para></entry>
>
> 2a.
> That "CREATE PUBLICATION" should <link> back to the CREATE PUBLICATION
> docs page.
>
> ~
>
> 2b.
> It is overkill to mention about the potential reordering of ALL TABLES
> and ALL SEQUENCES.
>
> Apart from being unnecessary, there are many other things can also be
> rearranged which are not mentioned:
> - TABLES and ALL TABLES IN SCHEMA clauses might be different order
> than specified
> - The publication parameters might be in a different order than specified
> - The values of 'publish' parameter might be different order than specified
> - etc.
>
> ~~~
>
> GENERAL
>
> 3.
> It would be better if the the rows of "Table 9.96" were in alphabetical order.
>
> ======
> src/backend/utils/adt/ddlutils.c
>
> pg_get_publication_ddl_internal:
>
> 4.
> + if (pub->allsequences)
> + appendStringInfo(buf,
> + "%sALL SEQUENCES",
> + pub->alltables ? ", " : "");
>
> Maybe better to avoid tricky format strings.
>
> SUGGESTION
> if (pub->allsequences)
> {
> if (pub->alltables)
> appendStringInfo(buf, ", ");
>
> appendStringInfo("ALL SEQUENCES");
> }
>
> ~~~
>
> 5.
> + if (pub_incl_relids != NIL)
> + {
> + ListCell *pub_cell;
> + char *schemaname = NULL;
> + char *tablename;
> +
> + append_ddl_option(buf, pretty, 4, "FOR TABLE ");
> +
> + /*
> + * Publication can have table relations
> + */
> + foreach(pub_cell, pub_incl_relids)
>
> Maybe that comment belongs earlier (above the if).
>
> ~~~
>
> 6.
> + appendStringInfo(buf, "%s%s",
> + foreach_current_index(pub_cell) > 0 ? ", " : "",
> + quote_qualified_identifier(schemaname, tablename));
>
> Another place where avoiding a tricky format string may be tidier.
>
> SUGGESTION
> if (foreach_current_index(pub_cell) > 0)
> appendStringInfo(buf, ", ");
>
> appendStringInfo(buf, "%s", quote_qualified_identifier(schemaname, tablename));
>
> ~~~
>
> 7.
> + pubtuple = SearchSysCache2(PUBLICATIONRELMAP, ObjectIdGetDatum(relid),
> + ObjectIdGetDatum(pub->oid));
> +
> + if (!HeapTupleIsValid(pubtuple))
> + elog(ERROR,
> + "cache lookup failed for publication relation %u in publication %u",
> + relid, pub->oid);
>
> 7a.
> Maybe blank line here is not wanted.
>
> ~
>
> 7b.
> Don't need to say "publication" 2x.
>
> /publication relation/relation/
>
> ~~~
>
> 8.
> + /* If non-null, we have a list of columns to publish */
> + if (!cols_nulls)
>
> SUGGESTION FOR THE COMMENT
> Does this table specify a column-list?
>
> ~~~
>
> 9.
> + appendStringInfo(buf, "%s%s",
> + bms_member_index(attmap, attnum) ? ", " : "",
> + quote_identifier(get_attname(relid, attnum, true)));
>
> Another place where avoiding a tricky format string may be tidier.
>
> (change similar to previous review comments)
>
> ~~~
>
> 10.
> + /*
> + * If there is a condition it goes after the columns. We can have
> + * conditions without columns as well.
> + */
> + if (!condition_nulls)
>
> 10a.
> The earlier assignment to 'conditions' should be moved to be directly
> above here.
>
> ~
>
> 10b.
> BTW, it is called a "row filter" so maybe it is better to refer to
> that in the comments/vars instead of the generic sounding "condition".
>
> ~~~
>
> 11.
> + /* If we have schemas, they will go right before the WITH */
>
> The kind of comments that just say "this-goes-after-that" or
> "this-goes-after-that" are not very useful, because it is obvious from
> the code logic that some appendStringInfo comes before or after
> another one.
>
> ~~~
>
> 12.
> + /*
> + * Schemas can be preceded by a list of tables. When they are, the
> + * "TABLES IN SCHEMA" stays inline as a continuation of the existing
> + * FOR clause; otherwise it starts the FOR clause on its own line in
> + * pretty mode.
> + */
>
> IMO it would be better for the FOR TABLE IN SCHEMA to come *before*
> the specific tables in FOR TABLE.
>
> e.g. For the case when there are specified tables "absorbed" into the
> same named schemas I think it is more natural to see the schemas
> first.
> CREATE PUBLICATION mypub FOR TABLES IN SCHEMA s, TABLE s.t1;
>
> ~~~
>
> 13.
> + appendStringInfo(buf, "%s %s",
> + foreach_current_index(schema_cell) > 0 ? "," : "",
> + quote_identifier(nspname));
>
> Another place where avoiding a tricky format string may be tidier.
>
> (change similar to previous review comments)
>
> ~~~
>
> 14.
> + if (pub_excl_relids != NIL)
> + {
> + ListCell *excl_cell;
> + char *schemaname = NULL;
> +
> + appendStringInfoString(buf, " EXCEPT (TABLE ");
>
> The EXCEPT clause is currently permitted only with FOR ALL TABLES, so
> it would be better moving this to earlier in this function where
> pub->alltables was handled.
>
> ~~~
>
> 15.
> + appendStringInfo(buf, "%s%s",
> + foreach_current_index(excl_cell) > 0 ? ", " : "",
> + quote_qualified_identifier(schemaname, NameStr(reltup->relname)));
>
> Another place where avoiding a tricky format string may be tidier.
>
> (change similar to previous review comments)
>
> ~~~
>
> 16.
> + /*
> + * We need to know if we're the second permission added to prefix with a
> + * ", " string
> + */
> + if (pub->pubactions.pubinsert)
> + {
> + /*
> + * By precedence we know that the insert will always be first, no need
> + * to check previous values
> + */
> + appendStringInfoString(buf, "insert");
>
> Both these comments are doing little more than just saying the same as
> the code. IMO they are not needed.
>
> ~~~
>
> 17.
> + if (pub->pubactions.pubinsert)
> + {
> + /*
> + * By precedence we know that the insert will always be first, no need
> + * to check previous values
> + */
> + appendStringInfoString(buf, "insert");
> + first_perm = false;
> + }
> +
> + if (pub->pubactions.pubupdate)
> + {
> + appendStringInfo(buf, "%supdate", first_perm ? "" : ", ");
> + first_perm = false;
> + }
> + if (pub->pubactions.pubdelete)
> + {
> + appendStringInfo(buf, "%sdelete", first_perm ? "" : ", ");
> + first_perm = false;
> + }
> +
> + if (pub->pubactions.pubtruncate)
> + {
> + appendStringInfo(buf, "%struncate", first_perm ? "" : ", ");
> + }
> +
>
> 17a.
> There are some random blank lines that seem unnecessary.
>
> ~
>
> 17b.
> IMO it is tidier to simply append the string you want, instead of
> using a trick format string.
>
> SUGGESTION (compare with patch)
>
> if (pub->pubactions.pubinsert)
> {
> appendStringInfoString(buf, "insert");
> first_perm = false;
> }
> if (pub->pubactions.pubupdate)
> {
> appendStringInfo(buf, first_perm ? "update" : ",update");
> first_perm = false;
> }
> if (pub->pubactions.pubdelete)
> {
> appendStringInfo(buf, first_perm ? "delete" : ",delete");
> first_perm = false;
> }
> if (pub->pubactions.pubtruncate)
> {
> appendStringInfo(buf, first_perm ? "truncate" : ",truncate");
> }
>
> ======
> src/test/regress/expected/publication_ddl.out
>
> 18.
> + CREATE PUBLICATION testpub_ddl_1
> +
> + WITH (publish='insert, update, delete, truncate',
> publish_generated_columns='none', publish_via_partition_root='false');
>
> ~
>
> This "pretty" output appears to have "+" garbage in it. What's that
> about -- it looks like some sort of line continuation character? Can
> it be removed?
>
> ~~~
>
> 19.
> The "pretty" output might be improved if each of the publication
> options was on a new line.
>
> ~~~
>
> 20.
> The generated boolean values (e.g. 'true'/'false') do not need to be quoted.
>
> ======
> src/test/regress/sql/publication_ddl.sql
>
> (Here are lots of test review comments; the first group are are
> general so might apply to multiple test cases).
>
> 21.
> I think you can create all the necessary schema and tables together
> up-front instead of scatering them through the file.
>
> ~~~
>
> 22.
> Make use of the proper publication teminology like "Column Lists" and
> "Row Filters" instead of vague
> "columns" and "conditions".
>
> ~~~
>
> 23.
> Here is an idea:
>
> Instead of having dozens of test publications, just have 1 test
> publication, which you CREATE/DROP for each test case.
>
> Then, since there is a fixed name publication (e.g. "mypub") for
> everything, you can make a subroutine to encapsulate the common code:
>
> +SELECT pg_get_publication_ddl('mypub');
> +SELECT pg_get_publication_ddl((SELECT oid FROM pg_publication WHERE
> pubname='mypub'));
> +SELECT pg_get_publication_ddl('mypub', 'pretty', 'true');
>
> It means your test .sql file can become much shorter/simpler I think.
>
> ~~~
>
> 24.
> There is duplication of some tests:
>
> e.g.
> +-- columns in publication must be quoted
> and
> +-- identifiers that require quoting: publication, schema, table and column
>
> ~~~
>
> 25.
> It is not needed to quote the booleans 'true'/'false' for the options.
>
> //////
>
> 26.
> +-- create base table to test basic table publication
>
> What does "basic table publication" mean? I expect it means different
> things to different people. Better to be explicit about what this is
> really testing.
>
> ~~~
>
> 27.
> +-- create publication for one table with two columns and a condition
> with an expression
>
> What does "with an expression" mean? All Row-Filters are expressions
> aren't they?
>
> ~~~
>
> 28.
> +-- create a publication for a list of tables
>
> Not really describing what this test is doing, which is mixing FOR
> TABLE and FOR TABLES IN SCHEMA.
>
> ~~~
>
> 29.
> +CREATE PUBLICATION testpub_ddl_part3 FOR TABLE testpub_ddl_part WITH
> (publish_via_partition_root='true');
>
> I guess it make no difference since these are only DDL syntax tests,
> but it didn't really make much sense to set
> publish_via_partition_root=true when testpub_ddl_part is the ROOT
> anyway.
>
> ~~~
>
> 30.
> +-- create publication for all tables except two tables
>
> Actually this is also combining with an ALL SEQUENCES test.
>
> ~~~
>
> 31.
> +-- cleanup publications
> +DROP PUBLICATION testpub_ddl_1;
> +DROP PUBLICATION testpub_ddl_2;
> +DROP PUBLICATION testpub_ddl_3;
> +DROP PUBLICATION testpub_ddl_4;
> +DROP PUBLICATION testpub_ddl_5;
> +DROP PUBLICATION testpub_ddl_6;
> +DROP PUBLICATION testpub_ddl_7;
> +DROP PUBLICATION testpub_ddl_8;
> +DROP PUBLICATION testpub_ddl_9;
> +DROP PUBLICATION testpub_ddl_10;
> +DROP PUBLICATION testpub_ddl_schema_1;
> +DROP PUBLICATION testpub_ddl_schema_2;
> +DROP PUBLICATION testpub_ddl_schema_3;
> +DROP PUBLICATION testpub_ddl_schema_4;
> +DROP PUBLICATION testpub_ddl_schema_5;
> +DROP PUBLICATION testpub_ddl_part1;
> +DROP PUBLICATION testpub_ddl_part2;
> +DROP PUBLICATION testpub_ddl_part3;
> +DROP PUBLICATION testpub_ddl_part4;
> +DROP PUBLICATION "Quoted Pub";
> +DROP PUBLICATION testpub_ddl_except1;
> +DROP PUBLICATION testpub_ddl_except2;
>
> As per earlier review comment IMO it would make the test code simpler
> to have just 1 publication that you CREATE/DROP om the fly.
>
> ~~~
>
> 32.
> +-- cleanup tables in schemas
>
> Not sure why this is done separately. Probably easier just to drop the
> schemas with CASCADE so their tables will be auto-deleted.
>
+ buf = makeStringInfo();
I have one more comment: where possible, we should use stack-allocated
StringInfoData objects, as was done in commits a63bbc811d4 and 6d0eba66275.
> ======
> Kind Regards,
> Peter Smith.
> Fujitsu Australia
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Add pg_get_publication_ddl function
@ 2026-05-20 22:27 Peter Smith <[email protected]>
parent: Peter Smith <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Peter Smith @ 2026-05-20 22:27 UTC (permalink / raw)
To: Jonathan Gonzalez V. <[email protected]>; +Cc: [email protected]
On Wed, May 20, 2026 at 7:39 PM Peter Smith <[email protected]> wrote:
...
>
> 18.
> + CREATE PUBLICATION testpub_ddl_1
> +
> + WITH (publish='insert, update, delete, truncate',
> publish_generated_columns='none', publish_via_partition_root='false');
>
> ~
>
> This "pretty" output appears to have "+" garbage in it. What's that
> about -- it looks like some sort of line continuation character? Can
> it be removed?
>
Later, I learned that this is a character added by psql to indicate
rows that span multiple lines. So, please ignore this comment.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-05-20 22:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-01-11 11:20 Add pg_get_publication_ddl function Jonathan Gonzalez V. <[email protected]>
2026-01-11 12:18 ` =?gb18030?B?emVuZ21hbg==?= <[email protected]>
2026-02-27 07:53 ` Hüseyin Demir <[email protected]>
2026-05-20 09:39 ` Peter Smith <[email protected]>
2026-05-20 10:00 ` Japin Li <[email protected]>
2026-05-20 22:27 ` Peter Smith <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox