From fbe57036acd717f5fa3f8d2ee4de09fa555100ff Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Wed, 29 Dec 2021 14:54:42 +1100 Subject: [PATCH v2] PublicationActions - use bit flags. The current PublicationActions is a struct of boolean representing combinations of the 4 different "publication actions". IMO it is more natural to implement these flag combinations using a bitmask instead of a struct of bools. This patch keeps the PublicationActions typedef but changes it to be unsigned int, so the previous structure members are all replaced now by bit operations. The Relation cache is also replaced by a simple PublicationAction bitmask instead of a pointer to an allocated struct of pubaction flags. --- src/backend/catalog/pg_publication.c | 9 ++++--- src/backend/commands/publicationcmds.c | 38 +++++++++++++--------------- src/backend/executor/execReplication.c | 6 ++--- src/backend/replication/pgoutput/pgoutput.c | 24 ++++++------------ src/backend/utils/cache/relcache.c | 39 ++++++++++------------------- src/include/catalog/pg_publication.h | 14 +++++------ src/include/utils/rel.h | 2 +- src/include/utils/relcache.h | 3 +-- 8 files changed, 56 insertions(+), 79 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 62f10bc..dbc5d7a 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -783,10 +783,11 @@ GetPublication(Oid pubid) pub->oid = pubid; pub->name = pstrdup(NameStr(pubform->pubname)); pub->alltables = pubform->puballtables; - pub->pubactions.pubinsert = pubform->pubinsert; - pub->pubactions.pubupdate = pubform->pubupdate; - pub->pubactions.pubdelete = pubform->pubdelete; - pub->pubactions.pubtruncate = pubform->pubtruncate; + pub->pubactions = 0; + if (pubform->pubinsert) pub->pubactions |= PUBACTION_INSERT; + if (pubform->pubupdate) pub->pubactions |= PUBACTION_UPDATE; + if (pubform->pubdelete) pub->pubactions |= PUBACTION_DELETE; + if (pubform->pubtruncate) pub->pubactions |= PUBACTION_TRUNCATE; pub->pubviaroot = pubform->pubviaroot; ReleaseSysCache(tup); diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 404bb5d..d509e4b 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -73,10 +73,7 @@ parse_publication_options(ParseState *pstate, *publish_via_partition_root_given = false; /* defaults */ - pubactions->pubinsert = true; - pubactions->pubupdate = true; - pubactions->pubdelete = true; - pubactions->pubtruncate = true; + *pubactions = PUBACTION_ALL; *publish_via_partition_root = false; /* Parse options */ @@ -97,10 +94,7 @@ parse_publication_options(ParseState *pstate, * If publish option was given only the explicitly listed actions * should be published. */ - pubactions->pubinsert = false; - pubactions->pubupdate = false; - pubactions->pubdelete = false; - pubactions->pubtruncate = false; + *pubactions = 0; *publish_given = true; publish = defGetString(defel); @@ -116,13 +110,13 @@ parse_publication_options(ParseState *pstate, char *publish_opt = (char *) lfirst(lc); if (strcmp(publish_opt, "insert") == 0) - pubactions->pubinsert = true; + *pubactions |= PUBACTION_INSERT; else if (strcmp(publish_opt, "update") == 0) - pubactions->pubupdate = true; + *pubactions |= PUBACTION_UPDATE; else if (strcmp(publish_opt, "delete") == 0) - pubactions->pubdelete = true; + *pubactions |= PUBACTION_DELETE; else if (strcmp(publish_opt, "truncate") == 0) - pubactions->pubtruncate = true; + *pubactions |= PUBACTION_TRUNCATE; else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -299,13 +293,13 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt) values[Anum_pg_publication_puballtables - 1] = BoolGetDatum(stmt->for_all_tables); values[Anum_pg_publication_pubinsert - 1] = - BoolGetDatum(pubactions.pubinsert); + BoolGetDatum(pubactions & PUBACTION_INSERT); values[Anum_pg_publication_pubupdate - 1] = - BoolGetDatum(pubactions.pubupdate); + BoolGetDatum(pubactions & PUBACTION_UPDATE); values[Anum_pg_publication_pubdelete - 1] = - BoolGetDatum(pubactions.pubdelete); + BoolGetDatum(pubactions & PUBACTION_DELETE); values[Anum_pg_publication_pubtruncate - 1] = - BoolGetDatum(pubactions.pubtruncate); + BoolGetDatum(pubactions & PUBACTION_TRUNCATE); values[Anum_pg_publication_pubviaroot - 1] = BoolGetDatum(publish_via_partition_root); @@ -406,16 +400,20 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt, if (publish_given) { - values[Anum_pg_publication_pubinsert - 1] = BoolGetDatum(pubactions.pubinsert); + values[Anum_pg_publication_pubinsert - 1] = + BoolGetDatum(pubactions & PUBACTION_INSERT); replaces[Anum_pg_publication_pubinsert - 1] = true; - values[Anum_pg_publication_pubupdate - 1] = BoolGetDatum(pubactions.pubupdate); + values[Anum_pg_publication_pubupdate - 1] = + BoolGetDatum(pubactions & PUBACTION_UPDATE); replaces[Anum_pg_publication_pubupdate - 1] = true; - values[Anum_pg_publication_pubdelete - 1] = BoolGetDatum(pubactions.pubdelete); + values[Anum_pg_publication_pubdelete - 1] = + BoolGetDatum(pubactions & PUBACTION_DELETE); replaces[Anum_pg_publication_pubdelete - 1] = true; - values[Anum_pg_publication_pubtruncate - 1] = BoolGetDatum(pubactions.pubtruncate); + values[Anum_pg_publication_pubtruncate - 1] = + BoolGetDatum(pubactions & PUBACTION_TRUNCATE); replaces[Anum_pg_publication_pubtruncate - 1] = true; } diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 574d7d2..4e2f9d5 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -567,7 +567,7 @@ ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo, void CheckCmdReplicaIdentity(Relation rel, CmdType cmd) { - PublicationActions *pubactions; + PublicationActions pubactions; /* We only need to do checks for UPDATE and DELETE. */ if (cmd != CMD_UPDATE && cmd != CMD_DELETE) @@ -584,13 +584,13 @@ CheckCmdReplicaIdentity(Relation rel, CmdType cmd) * Check if the table publishes UPDATES or DELETES. */ pubactions = GetRelationPublicationActions(rel); - if (cmd == CMD_UPDATE && pubactions->pubupdate) + if (cmd == CMD_UPDATE && (pubactions & PUBACTION_UPDATE)) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot update table \"%s\" because it does not have a replica identity and publishes updates", RelationGetRelationName(rel)), errhint("To enable updating the table, set REPLICA IDENTITY using ALTER TABLE."))); - else if (cmd == CMD_DELETE && pubactions->pubdelete) + else if (cmd == CMD_DELETE && (pubactions & PUBACTION_DELETE)) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("cannot delete from table \"%s\" because it does not have a replica identity and publishes deletes", diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c index 6f6a203..11a0277 100644 --- a/src/backend/replication/pgoutput/pgoutput.c +++ b/src/backend/replication/pgoutput/pgoutput.c @@ -653,15 +653,15 @@ pgoutput_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, switch (change->action) { case REORDER_BUFFER_CHANGE_INSERT: - if (!relentry->pubactions.pubinsert) + if (!(relentry->pubactions & PUBACTION_INSERT)) return; break; case REORDER_BUFFER_CHANGE_UPDATE: - if (!relentry->pubactions.pubupdate) + if (!(relentry->pubactions & PUBACTION_UPDATE)) return; break; case REORDER_BUFFER_CHANGE_DELETE: - if (!relentry->pubactions.pubdelete) + if (!(relentry->pubactions & PUBACTION_DELETE)) return; break; default: @@ -796,7 +796,7 @@ pgoutput_truncate(LogicalDecodingContext *ctx, ReorderBufferTXN *txn, relentry = get_rel_sync_entry(data, relid); - if (!relentry->pubactions.pubtruncate) + if (!(relentry->pubactions & PUBACTION_TRUNCATE)) continue; /* @@ -1139,8 +1139,7 @@ get_rel_sync_entry(PGOutputData *data, Oid relid) entry->schema_sent = false; entry->streamed_txns = NIL; entry->replicate_valid = false; - entry->pubactions.pubinsert = entry->pubactions.pubupdate = - entry->pubactions.pubdelete = entry->pubactions.pubtruncate = false; + entry->pubactions = 0; entry->publish_as_relid = InvalidOid; entry->map = NULL; /* will be set by maybe_send_schema() if * needed */ @@ -1239,14 +1238,10 @@ get_rel_sync_entry(PGOutputData *data, Oid relid) if (publish && (relkind != RELKIND_PARTITIONED_TABLE || pub->pubviaroot)) { - entry->pubactions.pubinsert |= pub->pubactions.pubinsert; - entry->pubactions.pubupdate |= pub->pubactions.pubupdate; - entry->pubactions.pubdelete |= pub->pubactions.pubdelete; - entry->pubactions.pubtruncate |= pub->pubactions.pubtruncate; + entry->pubactions |= pub->pubactions; } - if (entry->pubactions.pubinsert && entry->pubactions.pubupdate && - entry->pubactions.pubdelete && entry->pubactions.pubtruncate) + if (entry->pubactions == PUBACTION_ALL) break; } @@ -1387,10 +1382,7 @@ rel_sync_cache_publication_cb(Datum arg, int cacheid, uint32 hashvalue) * There might be some relations dropped from the publication so we * don't need to publish the changes for them. */ - entry->pubactions.pubinsert = false; - entry->pubactions.pubupdate = false; - entry->pubactions.pubdelete = false; - entry->pubactions.pubtruncate = false; + entry->pubactions = 0; } } diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 105d8d4..fed94aa 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -2418,8 +2418,7 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc) bms_free(relation->rd_pkattr); bms_free(relation->rd_idattr); bms_free(relation->rd_hotblockingattr); - if (relation->rd_pubactions) - pfree(relation->rd_pubactions); + relation->rd_pubactions = 0; if (relation->rd_options) pfree(relation->rd_options); if (relation->rd_indextuple) @@ -5524,14 +5523,13 @@ RelationGetExclusionInfo(Relation indexRelation, /* * Get publication actions for the given relation. */ -struct PublicationActions * +PublicationActions GetRelationPublicationActions(Relation relation) { List *puboids; ListCell *lc; - MemoryContext oldcxt; Oid schemaid; - PublicationActions *pubactions = palloc0(sizeof(PublicationActions)); + PublicationActions pubactions = 0; /* * If not publishable, it publishes no actions. (pgoutput_change() will @@ -5540,9 +5538,8 @@ GetRelationPublicationActions(Relation relation) if (!is_publishable_relation(relation)) return pubactions; - if (relation->rd_pubactions) - return memcpy(pubactions, relation->rd_pubactions, - sizeof(PublicationActions)); + if (relation->rd_pubactions & PUBACTION_IN_RELCACHE) + return relation->rd_pubactions & PUBACTION_ALL; /* Fetch the publication membership info. */ puboids = GetRelationPublications(RelationGetRelid(relation)); @@ -5581,10 +5578,10 @@ GetRelationPublicationActions(Relation relation) pubform = (Form_pg_publication) GETSTRUCT(tup); - pubactions->pubinsert |= pubform->pubinsert; - pubactions->pubupdate |= pubform->pubupdate; - pubactions->pubdelete |= pubform->pubdelete; - pubactions->pubtruncate |= pubform->pubtruncate; + if (pubform->pubinsert) pubactions |= PUBACTION_INSERT; + if (pubform->pubupdate) pubactions |= PUBACTION_UPDATE; + if (pubform->pubdelete) pubactions |= PUBACTION_DELETE; + if (pubform->pubtruncate) pubactions |= PUBACTION_TRUNCATE; ReleaseSysCache(tup); @@ -5592,22 +5589,12 @@ GetRelationPublicationActions(Relation relation) * If we know everything is replicated, there is no point to check for * other publications. */ - if (pubactions->pubinsert && pubactions->pubupdate && - pubactions->pubdelete && pubactions->pubtruncate) + if (pubactions == PUBACTION_ALL) break; } - if (relation->rd_pubactions) - { - pfree(relation->rd_pubactions); - relation->rd_pubactions = NULL; - } - - /* Now save copy of the actions in the relcache entry. */ - oldcxt = MemoryContextSwitchTo(CacheMemoryContext); - relation->rd_pubactions = palloc(sizeof(PublicationActions)); - memcpy(relation->rd_pubactions, pubactions, sizeof(PublicationActions)); - MemoryContextSwitchTo(oldcxt); + /* Now save the actions in the relcache entry. */ + relation->rd_pubactions = pubactions | PUBACTION_IN_RELCACHE; return pubactions; } @@ -6162,7 +6149,7 @@ load_relcache_init_file(bool shared) rel->rd_pkattr = NULL; rel->rd_idattr = NULL; rel->rd_hotblockingattr = NULL; - rel->rd_pubactions = NULL; + rel->rd_pubactions = 0; rel->rd_statvalid = false; rel->rd_statlist = NIL; rel->rd_fkeyvalid = false; diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h index 902f2f2..a8b98ed 100644 --- a/src/include/catalog/pg_publication.h +++ b/src/include/catalog/pg_publication.h @@ -66,13 +66,13 @@ typedef FormData_pg_publication *Form_pg_publication; DECLARE_UNIQUE_INDEX_PKEY(pg_publication_oid_index, 6110, PublicationObjectIndexId, on pg_publication using btree(oid oid_ops)); DECLARE_UNIQUE_INDEX(pg_publication_pubname_index, 6111, PublicationNameIndexId, on pg_publication using btree(pubname name_ops)); -typedef struct PublicationActions -{ - bool pubinsert; - bool pubupdate; - bool pubdelete; - bool pubtruncate; -} PublicationActions; +#define PUBACTION_INSERT (1 << 0) +#define PUBACTION_UPDATE (1 << 1) +#define PUBACTION_DELETE (1 << 2) +#define PUBACTION_TRUNCATE (1 << 3) +#define PUBACTION_IN_RELCACHE (1 << 4) +#define PUBACTION_ALL (PUBACTION_INSERT | PUBACTION_UPDATE | PUBACTION_DELETE | PUBACTION_TRUNCATE) +typedef uint8 PublicationActions; typedef struct Publication { diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index 3128127..0c4b592 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -161,7 +161,7 @@ typedef struct RelationData Bitmapset *rd_idattr; /* included in replica identity index */ Bitmapset *rd_hotblockingattr; /* cols blocking HOT update */ - PublicationActions *rd_pubactions; /* publication actions */ + PublicationActions rd_pubactions; /* publication actions (valid if PUBACTION_IN_RELCACHE bit set) */ /* * rd_options is set whenever rd_rel is loaded into the relcache entry. diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 82316bb..1ede853 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -74,8 +74,7 @@ extern void RelationGetExclusionInfo(Relation indexRelation, extern void RelationInitIndexAccessInfo(Relation relation); /* caller must include pg_publication.h */ -struct PublicationActions; -extern struct PublicationActions *GetRelationPublicationActions(Relation relation); +extern uint8 GetRelationPublicationActions(Relation relation); extern void RelationInitTableAccessMethod(Relation relation); -- 1.8.3.1