public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Smith <[email protected]>
To: shveta malik <[email protected]>
Cc: vignesh C <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Logical Replication - revisit `is_table_publication` function implementation
Date: Wed, 8 Apr 2026 16:27:49 +1000
Message-ID: <CAHut+PsjrPOcs=ePWm+N-q=rdmhDeM-FE05gPgDoN647Jb3RaQ@mail.gmail.com> (raw)
In-Reply-To: <CAHut+PtfHzxHFkHJWYoxOFpgpSH5HNAGmP5sSrwh1d+R0Ab-BQ@mail.gmail.com>
References: <CAHut+Pti83yGaV5-DZU=AvJHxFDuoKW8_pjSedRham8SgZxLYA@mail.gmail.com>
<CALDaNm0nLdBKJVHVvvOnY_5mkVg20=OL18fdjA5+KZ3GhPB=TQ@mail.gmail.com>
<CAHut+Pv+a-7-NRrZv4v6RfaaUo5b21RXe0tGOu8CfKrxPjE=tw@mail.gmail.com>
<CAJpy0uAHe88hL5MX3q9tyGyx_gCKKHcWnmETXvXM2CqnE8jrmA@mail.gmail.com>
<CAHut+PtfHzxHFkHJWYoxOFpgpSH5HNAGmP5sSrwh1d+R0Ab-BQ@mail.gmail.com>
On Wed, Apr 8, 2026 at 4:04 PM Peter Smith <[email protected]> wrote:
>
> On Wed, Apr 8, 2026 at 3:25 PM shveta malik <[email protected]> wrote:
> >
> > On Wed, Apr 8, 2026 at 10:24 AM Peter Smith <[email protected]> wrote:
> > >
> > > On Wed, Apr 8, 2026 at 1:45 PM vignesh C <[email protected]> wrote:
> > > >
...
> > > > And also why just check for puballtables why not to check for puballsequences
> > >
> > > I think function is_schema_publication() is unrelated to 'puballsequences'.
> > >
> > > e.g. all the following will still need to check
> > > pg_publication_namespace, regardless of the 'puballsequences' value.
> > >
> > > ex1. CREATE PUBLICATION ... FOR ALL SEQUENCES;
> > > ex2. CREATE PUBLICATION ... FOR ALL SEQUENCES, FOR TABLES IN SCHEMA s1;
> > > ex3. CREATE PUBLICATION ... FOR TABLES IN SCHEMA s1;
> > >
> >
> > IIUC, we don't support mix of ALL SEQUENCES and TABLES IN SCHEMA s1.
> > So I could not understand your point, why FOR ALL SEQ still need to
> > check pg_publication_namespace?
> >
>
> Oh! You are right.
>
> (Sorry, Vignesh, I did not recognise that combination as unsupported).
>
> I'll post a patch update to handle it.
>
PSA patch v2.
Same as before, but now also doing a quick return false from both
functions if `puballsequences` is true.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
[application/octet-stream] v2-0001-rewrite-is_table_publication.patch (5.2K, 2-v2-0001-rewrite-is_table_publication.patch)
download | inline diff:
From f09ebae190716e440a6ce7dd135056a6343cd9d0 Mon Sep 17 00:00:00 2001
From: Peter Smith <[email protected]>
Date: Wed, 8 Apr 2026 16:22:37 +1000
Subject: [PATCH v2] rewrite is_table_publication
---
src/backend/catalog/pg_publication.c | 44 ++++++++++++++------------
src/backend/commands/publicationcmds.c | 7 ++--
src/include/catalog/pg_publication.h | 4 +--
3 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index a43d385c605..e2003cd9203 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -275,19 +275,26 @@ 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.
+ * FOR TABLES IN SCHEMA cannot coexist with FOR ALL SEQUENCES.
+ */
+ if (pubform->puballtables || pubform->puballsequences)
+ 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 +309,36 @@ 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.
+ * FOR TABLE cannot coexist with FOR ALL SEQUENCES.
+ */
+ if (pubform->puballtables || pubform->puballsequences)
+ 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], [email protected], [email protected]
Subject: Re: Logical Replication - revisit `is_table_publication` function implementation
In-Reply-To: <CAHut+PsjrPOcs=ePWm+N-q=rdmhDeM-FE05gPgDoN647Jb3RaQ@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