On Fri, 19 Dec 2025 at 10:05, Peter Smith <[email protected]> wrote: > > Hi Vignesh. > > I had a quick look at this patch. > > ====== > > - * Instead, we invalidate only the relsyncache. > + * Instead, invalidate the relation sync cache for publications that > + * include tables. Invalidation is not required for sequences only > + * publications. > */ > - InvalidatePubRelSyncCache(pub->oid, pub->puballtables); > + if (!pub->puballsequences || pub->puballtables) > + InvalidatePubRelSyncCache(pub->oid, pub->puballtables); > > I felt all these "sequence only" conditions are becoming difficult to read. > > It wonder if it would be better to introduce some function like: > > bool > PubHasSequencesOnly(pub) > { > return pub->puballsequences && !pub->puballtables; > }
I added a macro following a similar pattern. However, since the function calls use different data types, I modified it to pass individual members instead of the structure. > IIUC the current patch code only works because publication syntax like > below is not yet supported: > > CREATE PUBLICATION pub1 FOR ALL SEQUENCES, TABLE t1; > > But when that syntax does become possible, then all these conditions > will be broken. > > Should we prepare for that eventuality by introducing some function > right now, so as to contain all the future broken code? I believe no specific handling is required. These flows will be naturally addressed as part of the feature implementation. The attached patch has the changes for the comments. Regards, Vignesh
From 0fe834fbc5794bb43fbaffc912564f9bc6e6ebb1 Mon Sep 17 00:00:00 2001 From: Vignesh C <[email protected]> Date: Thu, 18 Dec 2025 14:59:44 +0530 Subject: [PATCH v2] Skip table specific handling for sequences only publications Skip trying to get the list of tables, publish_via_partition_root handling, and invalidation where the publication does not publish tables. This avoids unnecessary work and ensures table specific logic is applied only to relevant publications. --- src/backend/catalog/pg_publication.c | 11 ++++++++--- src/backend/commands/alter.c | 6 ++++-- src/backend/commands/publicationcmds.c | 6 +++--- src/include/catalog/pg_publication.h | 6 ++++++ 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 7aa3f179924..6ce745f379b 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -1170,7 +1170,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) if (pub_elem->alltables) pub_elem_tables = GetAllPublicationRelations(RELKIND_RELATION, pub_elem->pubviaroot); - else + else if (!pub_elem->allsequences) { List *relids, *schemarelids; @@ -1203,8 +1203,13 @@ pg_get_publication_tables(PG_FUNCTION_ARGS) table_infos = lappend(table_infos, table_info); } - /* At least one publication is using publish_via_partition_root. */ - if (pub_elem->pubviaroot) + /* + * At least one publication is using publish_via_partition_root. + * Skip sequences only publications, as publish_via_partition_root + * is applicable only to table publications. + */ + if (pub_elem->pubviaroot && !PUB_HAS_SEQUENCES_ONLY(pub_elem->allsequences, + pub_elem->alltables)) viaroot = true; } diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index cb75e11fced..a6ce56b2544 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -349,9 +349,11 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) * Unlike ALTER PUBLICATION ADD/SET/DROP commands, renaming a * publication does not impact the publication status of tables. So, * we don't need to invalidate relcache to rebuild the rd_pubdesc. - * Instead, we invalidate only the relsyncache. + * Instead, we invalidate only the relsyncache. Invalidation is not + * required for sequences only publications. */ - InvalidatePubRelSyncCache(pub->oid, pub->puballtables); + if (!PUB_HAS_SEQUENCES_ONLY(pub->puballsequences, pub->puballtables)) + InvalidatePubRelSyncCache(pub->oid, pub->puballtables); } /* Release memory */ diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index a1983508950..e128789a673 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -1028,8 +1028,8 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt, * disallow using WHERE clause and column lists on partitioned table in * this case. */ - if (!pubform->puballtables && publish_via_partition_root_given && - !publish_via_partition_root) + if (!pubform->puballtables && !pubform->puballsequences && + publish_via_partition_root_given && !publish_via_partition_root) { /* * Lock the publication so nobody else can do anything with it. This @@ -1149,7 +1149,7 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt, { CacheInvalidateRelcacheAll(); } - else + else if (!pubform->puballsequences) { List *relids = NIL; List *schemarelids = NIL; diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h index 22f48bb8975..cabc509e659 100644 --- a/src/include/catalog/pg_publication.h +++ b/src/include/catalog/pg_publication.h @@ -148,6 +148,12 @@ typedef struct PublicationRelInfo List *columns; } PublicationRelInfo; +/* + * Publication is defined for all sequences only (no tables included). + */ +#define PUB_HAS_SEQUENCES_ONLY(allsequences, alltables) \ + ((allsequences) && !(alltables)) + extern Publication *GetPublication(Oid pubid); extern Publication *GetPublicationByName(const char *pubname, bool missing_ok); extern List *GetRelationPublications(Oid relid); -- 2.43.0
