From adb17f448aafe908aa3501bd2388dd6c9230b72c Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Sun, 21 Dec 2025 19:46:01 +0530 Subject: [PATCH v15 4/6] Add shared index for conflict log table lookup Introduce a dedicated shared unique index on pg_subscription.subconflictlogrelid to make conflict log table detection efficient, and index-backed. Previously, IsConflictLogTable() relied on a full catalog scan of pg_subscription, which was inefficient. This change adds pg_subscription_conflictrel_index and marks it as a shared index, matching the shared pg_subscription table, and rewrites conflict log table detection to use an indexed systable scan. --- src/backend/catalog/catalog.c | 1 + src/backend/catalog/pg_publication.c | 12 +---------- src/backend/catalog/pg_subscription.c | 23 +++++++++++++++++++++ src/backend/commands/subscriptioncmds.c | 20 +++++++++++------- src/backend/replication/logical/conflict.c | 4 +--- src/backend/replication/pgoutput/pgoutput.c | 15 +++++++++++--- src/bin/psql/describe.c | 4 +++- src/include/catalog/pg_proc.dat | 7 +++++++ src/include/catalog/pg_subscription.h | 1 + src/test/regress/expected/subscription.out | 7 ++++--- src/test/regress/sql/subscription.sql | 5 +++-- 11 files changed, 69 insertions(+), 30 deletions(-) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 59caae8f1bc..b95a0a36cd0 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -336,6 +336,7 @@ IsSharedRelation(Oid relationId) relationId == SharedSecLabelObjectIndexId || relationId == SubscriptionNameIndexId || relationId == SubscriptionObjectIndexId || + relationId == SubscriptionConflictrelIndexId || relationId == TablespaceNameIndexId || relationId == TablespaceOidIndexId) return true; diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 9f84e02b7ef..187eb351f3f 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -86,15 +86,6 @@ check_publication_add_relation(Relation targetrel) errmsg("cannot add relation \"%s\" to publication", RelationGetRelationName(targetrel)), errdetail("This operation is not supported for unlogged tables."))); - - /* Can't be conflict log table */ - if (IsConflictLogTable(RelationGetRelid(targetrel))) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("cannot add relation \"%s.%s\" to publication", - get_namespace_name(RelationGetNamespace(targetrel)), - RelationGetRelationName(targetrel)), - errdetail("This operation is not supported for conflict log tables."))); } /* @@ -188,8 +179,7 @@ pg_relation_is_publishable(PG_FUNCTION_ARGS) PG_RETURN_NULL(); /* Subscription conflict log tables are not published */ - result = is_publishable_class(relid, (Form_pg_class) GETSTRUCT(tuple)) && - !IsConflictLogTable(relid); + result = is_publishable_class(relid, (Form_pg_class) GETSTRUCT(tuple)); ReleaseSysCache(tuple); PG_RETURN_BOOL(result); } diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index 27a9aee1c56..dae44c659f8 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -22,6 +22,7 @@ #include "catalog/pg_subscription.h" #include "catalog/pg_subscription_rel.h" #include "catalog/pg_type.h" +#include "commands/subscriptioncmds.h" #include "miscadmin.h" #include "storage/lmgr.h" #include "utils/array.h" @@ -156,6 +157,28 @@ GetSubscription(Oid subid, bool missing_ok) return sub; } +/* + * pg_relation_is_conflict_log_table + * + * Returns true if the given relation OID is used as a conflict log table + * by any subscription, else returns false. + */ +Datum +pg_relation_is_conflict_log_table(PG_FUNCTION_ARGS) +{ + Oid relid = PG_GETARG_OID(0); + HeapTuple tuple; + bool result; + + tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tuple)) + PG_RETURN_NULL(); + + result = IsConflictLogTable(relid); + ReleaseSysCache(tuple); + PG_RETURN_BOOL(result); +} + /* * Return number of subscriptions defined in given database. * Used by dropdb() to check if database can indeed be dropped. diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 05dcb5e3fe4..b3694375191 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -52,6 +52,7 @@ #include "storage/lmgr.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/fmgroids.h" #include "utils/guc.h" #include "utils/lsyscache.h" #include "utils/memutils.h" @@ -3494,27 +3495,32 @@ bool IsConflictLogTable(Oid relid) { Relation rel; - TableScanDesc scan; + ScanKeyData scankey; + SysScanDesc scan; HeapTuple tup; bool is_clt = false; rel = table_open(SubscriptionRelationId, AccessShareLock); - scan = table_beginscan_catalog(rel, 0, NULL); - while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection))) + ScanKeyInit(&scankey, + Anum_pg_subscription_subconflictlogrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(relid)); + + scan = systable_beginscan(rel, SubscriptionConflictrelIndexId, + true, NULL, 1, &scankey); + while (HeapTupleIsValid(tup = systable_getnext(scan))) { Form_pg_subscription subform = (Form_pg_subscription) GETSTRUCT(tup); - /* Direct Oid comparison from catalog */ - if (OidIsValid(subform->subconflictlogrelid) && - subform->subconflictlogrelid == relid) + if (OidIsValid(subform->subconflictlogrelid)) { is_clt = true; break; } } - table_endscan(scan); + systable_endscan(scan); table_close(rel, AccessShareLock); return is_clt; diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index 7cc07a64427..0b6e3f4a2c8 100644 --- a/src/backend/replication/logical/conflict.c +++ b/src/backend/replication/logical/conflict.c @@ -294,13 +294,11 @@ GetConflictLogTableInfo(ConflictLogDest *log_dest) void InsertConflictLogTuple(Relation conflictlogrel) { - int options = HEAP_INSERT_NO_LOGICAL; - /* A valid tuple must be prepared and stored in MyLogicalRepWorker. */ Assert(MyLogicalRepWorker->conflict_log_tuple != NULL); heap_insert(conflictlogrel, MyLogicalRepWorker->conflict_log_tuple, - GetCurrentCommandId(true), options, NULL); + GetCurrentCommandId(true), 0, NULL); /* Free conflict log tuple. */ heap_freetuple(MyLogicalRepWorker->conflict_log_tuple); diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 787998abb8a..fe37b7fc284 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -131,6 +131,8 @@ typedef struct RelationSyncEntry bool schema_sent; + bool conflictlogrel; /* is this relation used for conflict logging? */ + /* * This will be PUBLISH_GENCOLS_STORED if the relation contains generated * columns and the 'publish_generated_columns' parameter is set to @@ -2067,6 +2069,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) { entry->replicate_valid = false; entry->schema_sent = false; + entry->conflictlogrel = false; entry->include_gencols_type = PUBLISH_GENCOLS_NONE; entry->streamed_txns = NIL; entry->pubactions.pubinsert = entry->pubactions.pubupdate = @@ -2117,6 +2120,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) * earlier definition. */ entry->schema_sent = false; + entry->conflictlogrel = IsConflictLogTable(relid); entry->include_gencols_type = PUBLISH_GENCOLS_NONE; list_free(entry->streamed_txns); entry->streamed_txns = NIL; @@ -2199,7 +2203,7 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) * If this is a FOR ALL TABLES publication, pick the partition * root and set the ancestor level accordingly. */ - if (pub->alltables) + if (pub->alltables && !entry->conflictlogrel) { publish = true; if (pub->pubviaroot && am_partition) @@ -2225,8 +2229,12 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) { Oid ancestor; int level; - List *ancestors = get_partition_ancestors(relid); + List *ancestors; + + /* Conflict log table cannot be a partition */ + Assert(entry->conflictlogrel == false); + ancestors = get_partition_ancestors(relid); ancestor = GetTopMostAncestorInPublication(pub->oid, ancestors, &level); @@ -2243,7 +2251,8 @@ get_rel_sync_entry(PGOutputData *data, Relation relation) } if (list_member_oid(pubids, pub->oid) || - list_member_oid(schemaPubids, pub->oid) || + (list_member_oid(schemaPubids, pub->oid) && + !entry->conflictlogrel) || ancestor_published) publish = true; } diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index cc80f0f661c..2d612448241 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -3063,6 +3063,7 @@ describeOneTableDetails(const char *schemaname, " JOIN pg_catalog.pg_publication_namespace pn ON p.oid = pn.pnpubid\n" " JOIN pg_catalog.pg_class pc ON pc.relnamespace = pn.pnnspid\n" "WHERE pc.oid ='%s' and pg_catalog.pg_relation_is_publishable('%s')\n" + "AND NOT pg_catalog.pg_relation_is_conflict_log_table('%s'::oid)\n" "UNION\n" "SELECT pubname\n" " , pg_get_expr(pr.prqual, c.oid)\n" @@ -3082,8 +3083,9 @@ describeOneTableDetails(const char *schemaname, " , NULL\n" "FROM pg_catalog.pg_publication p\n" "WHERE p.puballtables AND pg_catalog.pg_relation_is_publishable('%s')\n" + "AND NOT pg_catalog.pg_relation_is_conflict_log_table('%s'::oid)\n" "ORDER BY 1;", - oid, oid, oid, oid); + oid, oid, oid, oid, oid, oid); } else { diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index fd9448ec7b9..98fe8eee012 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -12321,6 +12321,13 @@ prorettype => 'bool', proargtypes => 'regclass', prosrc => 'pg_relation_is_publishable' }, +# subscriptions +{ oid => '6123', + descr => 'returns whether a relation is a subscription conflict log table', + proname => 'pg_relation_is_conflict_log_table', provolatile => 's', + prorettype => 'bool', proargtypes => 'regclass', + prosrc => 'pg_relation_is_conflict_log_table' }, + # rls { oid => '3298', descr => 'row security for current context active on table by table oid', diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h index 55f4bfa0419..46c446eaf8b 100644 --- a/src/include/catalog/pg_subscription.h +++ b/src/include/catalog/pg_subscription.h @@ -123,6 +123,7 @@ DECLARE_TOAST_WITH_MACRO(pg_subscription, 4183, 4184, PgSubscriptionToastTable, DECLARE_UNIQUE_INDEX_PKEY(pg_subscription_oid_index, 6114, SubscriptionObjectIndexId, pg_subscription, btree(oid oid_ops)); DECLARE_UNIQUE_INDEX(pg_subscription_subname_index, 6115, SubscriptionNameIndexId, pg_subscription, btree(subdbid oid_ops, subname name_ops)); +DECLARE_UNIQUE_INDEX(pg_subscription_conflictrel_index, 6122, SubscriptionConflictrelIndexId, pg_subscription, btree(oid oid_ops, subconflictlogrelid oid_ops)); MAKE_SYSCACHE(SUBSCRIPTIONOID, pg_subscription_oid_index, 4); MAKE_SYSCACHE(SUBSCRIPTIONNAME, pg_subscription_subname_index, 4); diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 4ab58d90925..92423c83197 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -635,13 +635,14 @@ EXCEPTION WHEN dependent_objects_still_exist THEN RAISE NOTICE 'captured expected error: dependent_objects_still_exist'; END $$; NOTICE: captured expected error: dependent_objects_still_exist --- PUBLICATION: Verify internal tables are not publishable --- pg_relation_is_publishable should return false for internal conflict log tables +-- PUBLICATION: Verify internal tables are publishable +-- pg_relation_is_publishable should return true for internal conflict log +-- tables, as it can be published using TABLE publication. SELECT pg_relation_is_publishable(subconflictlogrelid) FROM pg_subscription WHERE subname = 'regress_conflict_test1'; pg_relation_is_publishable ---------------------------- - f + t (1 row) -- CLEANUP: Proper drop reaps the table diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 3359ff8be5c..b4b98c9a178 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -443,8 +443,9 @@ EXCEPTION WHEN dependent_objects_still_exist THEN RAISE NOTICE 'captured expected error: dependent_objects_still_exist'; END $$; --- PUBLICATION: Verify internal tables are not publishable --- pg_relation_is_publishable should return false for internal conflict log tables +-- PUBLICATION: Verify internal tables are publishable +-- pg_relation_is_publishable should return true for internal conflict log +-- tables, as it can be published using TABLE publication. SELECT pg_relation_is_publishable(subconflictlogrelid) FROM pg_subscription WHERE subname = 'regress_conflict_test1'; -- 2.43.0