public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Smith <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Logical Replication - revisit `is_table_publication` function implementation
Date: Tue, 7 Apr 2026 17:02:23 +1000
Message-ID: <CAHut+Pti83yGaV5-DZU=AvJHxFDuoKW8_pjSedRham8SgZxLYA@mail.gmail.com> (raw)
Hi, after confirming my understanding of pg_publication_rel [1], I
revisited some logical replication internal functions.
Specifically.
* The `is_table_publication` function is for checking if the
publication has a clause like "FOR TABLE t1".
* The `is_schema_publication` function is for checking if the
publication has a clause like "FOR TABLES IN SCHEMA s1".
Notice that neither of these ("FOR TABLE", "FOR TABLES IN SCHEMA")
clauses are possible simultaneously with "FOR ALL TABLES".
And we can readily discover if "FOR ALL TABLES" (aka `puballtables`)
is present from the pubform.
We can use this to optimise and simplify the implementations of the
`is_schema_publication` and `is_table_publication` functions.
PSA patch v1.
AFAICT, the result is:
- less code + simpler logic. e.g. is_table_publication does not check
'prexcept' anymore
- more efficient. e.g. skips unnecessary scanning when puballtables is true.
- more consistent. e.g., both functions are now almost identical.
Thoughts?
======
[1] https://www.postgresql.org/message-id/flat/CAHut%2BPv1UKR_bxmN7wcCCpQveHoYprvH-hbdFq8gsaH1Ye7B_w%40m...
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
[application/octet-stream] v1-0001-rewrite-is_table_publication.patch (5.1K, 2-v1-0001-rewrite-is_table_publication.patch)
download | inline diff:
From 4fb7f2180f5674e527635118a967b0e2fd228e5b Mon Sep 17 00:00:00 2001
From: Peter Smith <[email protected]>
Date: Tue, 7 Apr 2026 16:31:21 +1000
Subject: [PATCH v1] rewrite is_table_publication
---
src/backend/catalog/pg_publication.c | 38 ++++++++++++--------------
src/backend/commands/publicationcmds.c | 7 ++---
src/include/catalog/pg_publication.h | 4 +--
3 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index a43d385c605..c60885db10a 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -275,19 +275,23 @@ filter_partitions(List *table_infos)
* schema is associated with the publication.
*/
bool
-is_schema_publication(Oid pubid)
+is_schema_publication(Form_pg_publication pubform)
{
Relation pubschsrel;
ScanKeyData scankey;
SysScanDesc scan;
HeapTuple tup;
- bool result = false;
+ bool result;
+
+ /* FOR TABLES IN SCHEMA cannot coexist with FOR ALL TABLES. */
+ if (pubform->puballtables)
+ return false;
pubschsrel = table_open(PublicationNamespaceRelationId, AccessShareLock);
ScanKeyInit(&scankey,
Anum_pg_publication_namespace_pnpubid,
BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(pubid));
+ ObjectIdGetDatum(pubform->oid));
scan = systable_beginscan(pubschsrel,
PublicationNamespacePnnspidPnpubidIndexId,
@@ -302,41 +306,33 @@ is_schema_publication(Oid pubid)
}
/*
- * Returns true if the publication has explicitly included relation (i.e.,
- * not marked as EXCEPT).
+ * Returns true if the publication has explicitly included relations (e.g.,
+ * FOR TABLE).
*/
bool
-is_table_publication(Oid pubid)
+is_table_publication(Form_pg_publication pubform)
{
Relation pubrelsrel;
ScanKeyData scankey;
SysScanDesc scan;
HeapTuple tup;
- bool result = false;
+ bool result;
+
+ /* FOR TABLE cannot coexist with FOR ALL TABLES. */
+ if (pubform->puballtables)
+ return false;
pubrelsrel = table_open(PublicationRelRelationId, AccessShareLock);
ScanKeyInit(&scankey,
Anum_pg_publication_rel_prpubid,
BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(pubid));
+ ObjectIdGetDatum(pubform->oid));
scan = systable_beginscan(pubrelsrel,
PublicationRelPrpubidIndexId,
true, NULL, 1, &scankey);
tup = systable_getnext(scan);
- if (HeapTupleIsValid(tup))
- {
- Form_pg_publication_rel pubrel;
-
- pubrel = (Form_pg_publication_rel) GETSTRUCT(tup);
-
- /*
- * For any publication, pg_publication_rel contains either only EXCEPT
- * entries or only explicitly included tables. Therefore, examining
- * the first tuple is sufficient to determine table inclusion.
- */
- result = !pubrel->prexcept;
- }
+ result = HeapTupleIsValid(tup);
systable_endscan(scan);
table_close(pubrelsrel, AccessShareLock);
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 440adb356ad..3685c711c49 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -1261,7 +1261,7 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
{
TransformPubWhereClauses(rels, queryString, pubform->pubviaroot);
- publish_schema |= is_schema_publication(pubid);
+ publish_schema |= is_schema_publication(pubform);
CheckPubRelationColumnList(stmt->pubname, rels, publish_schema,
pubform->pubviaroot);
@@ -1585,8 +1585,7 @@ CheckAlterPublication(AlterPublicationStmt *stmt, HeapTuple tup,
* If the publication already contains specific tables or schemas, we
* prevent switching to a ALL state.
*/
- if (is_table_publication(pubform->oid) ||
- is_schema_publication(pubform->oid))
+ if (is_table_publication(pubform) || is_schema_publication(pubform))
{
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
@@ -2202,7 +2201,7 @@ AlterPublicationOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
if (!superuser_arg(newOwnerId))
{
if (form->puballtables || form->puballsequences ||
- is_schema_publication(form->oid))
+ is_schema_publication(form))
ereport(ERROR,
errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to change owner of publication \"%s\"",
diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h
index 89b4bb14f62..448373363fc 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -194,8 +194,8 @@ extern Oid GetTopMostAncestorInPublication(Oid puboid, List *ancestors,
int *ancestor_level);
extern bool is_publishable_relation(Relation rel);
-extern bool is_schema_publication(Oid pubid);
-extern bool is_table_publication(Oid pubid);
+extern bool is_schema_publication(Form_pg_publication pubform);
+extern bool is_table_publication(Form_pg_publication pubform);
extern bool check_and_fetch_column_list(Publication *pub, Oid relid,
MemoryContext mcxt, Bitmapset **cols);
extern ObjectAddress publication_add_relation(Oid pubid, PublicationRelInfo *pri,
--
2.47.3
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]
Subject: Re: Logical Replication - revisit `is_table_publication` function implementation
In-Reply-To: <CAHut+Pti83yGaV5-DZU=AvJHxFDuoKW8_pjSedRham8SgZxLYA@mail.gmail.com>
* 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