From 69769779608d462b162d04c912a59c1601bf963e Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Mon, 20 Dec 2021 09:28:08 +1100 Subject: [PATCH v1] 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. --- 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 | 20 +++++++-------- src/include/catalog/pg_publication.h | 13 +++++----- src/include/utils/relcache.h | 4 +-- 7 files changed, 51 insertions(+), 63 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..c6cb2e5 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -5524,14 +5524,14 @@ 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 @@ -5541,8 +5541,7 @@ GetRelationPublicationActions(Relation relation) return pubactions; if (relation->rd_pubactions) - return memcpy(pubactions, relation->rd_pubactions, - sizeof(PublicationActions)); + return *relation->rd_pubactions; /* Fetch the publication membership info. */ puboids = GetRelationPublications(RelationGetRelid(relation)); @@ -5581,10 +5580,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,8 +5591,7 @@ 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; } @@ -5606,7 +5604,7 @@ GetRelationPublicationActions(Relation relation) /* 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)); + *relation->rd_pubactions = pubactions; MemoryContextSwitchTo(oldcxt); return pubactions; diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h index 902f2f2..376e49c 100644 --- a/src/include/catalog/pg_publication.h +++ b/src/include/catalog/pg_publication.h @@ -66,13 +66,12 @@ 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_ALL (PUBACTION_INSERT | PUBACTION_UPDATE | PUBACTION_DELETE | PUBACTION_TRUNCATE) +typedef unsigned PublicationActions; typedef struct Publication { diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h index 82316bb..9bd8b9b 100644 --- a/src/include/utils/relcache.h +++ b/src/include/utils/relcache.h @@ -74,8 +74,8 @@ extern void RelationGetExclusionInfo(Relation indexRelation, extern void RelationInitIndexAccessInfo(Relation relation); /* caller must include pg_publication.h */ -struct PublicationActions; -extern struct PublicationActions *GetRelationPublicationActions(Relation relation); +typedef unsigned PublicationActions; +extern PublicationActions GetRelationPublicationActions(Relation relation); extern void RelationInitTableAccessMethod(Relation relation); -- 1.8.3.1