From abdc729ab5e880543e4702d65e8ea66533325d71 Mon Sep 17 00:00:00 2001 From: Hou Zhijie Date: Tue, 3 Dec 2024 12:19:50 +0800 Subject: [PATCH] improvements --- doc/src/sgml/ref/create_publication.sgml | 9 ++++++++ src/backend/commands/publicationcmds.c | 13 +++++------- src/backend/executor/execReplication.c | 27 ++++++++++++------------ 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml index f8e217d661..ae21018697 100644 --- a/doc/src/sgml/ref/create_publication.sgml +++ b/doc/src/sgml/ref/create_publication.sgml @@ -311,6 +311,15 @@ CREATE PUBLICATION name system columns. + + If tables added to a publication include generated columns that are part of + the REPLICA IDENTITY, it is essential to publish these + columns by explicitly listing them in the column list or by enabling the + publish_generated_columns option. Otherwise, + UPDATE or DELETE operations will be + disallowed on those tables. + + The row filter on a table becomes redundant if FOR TABLES IN SCHEMA is specified and the table diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index d2b4c8e9a6..323f59fae1 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -427,16 +427,13 @@ pub_contains_invalid_column(Oid pubid, Relation relation, List *ancestors, if (columns == NULL) { /* - * Check if pubgencols is false and generated column is part of - * REPLICA IDENTITY + * Break the loop if an unpublished generated column is part of the + * REPLICA IDENTITY. */ - if (!pubgencols) + if (!pubgencols && att->attgenerated) { - *unpublished_gen_col |= att->attgenerated; - - /* Break the loop if unpublished generated columns exist. */ - if (*unpublished_gen_col) - break; + *unpublished_gen_col = true; + break; } /* Skip validating the column list since it is not defined */ diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 227a8aeea0..f66ff21159 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -785,27 +785,26 @@ CheckCmdReplicaIdentity(Relation rel, CmdType cmd) return; /* - * It is only safe to execute UPDATE/DELETE when: + * It is only safe to execute UPDATE/DELETE if the relation does not + * publish UPDATEs or DELETEs, or all the following conditions are + * satisfied: * * 1. All columns, referenced in the row filters from publications which - * the relation is in, are valid - i.e. when all referenced columns are - * part of REPLICA IDENTITY or the table does not publish UPDATEs or - * DELETEs. + * the relation is in, are valid - i.e. when all referenced columns are + * part of REPLICA IDENTITY. * * 2. All columns, referenced in the column lists from publications which - * the relation is in, are valid - i.e. when all referenced columns are - * part of REPLICA IDENTITY or the table does not publish UPDATEs or - * DELETEs. + * the relation is in, are valid - i.e. when all columns referenced in + * the REPLICA IDENTITY are covered by the column list. * - * 3. All generated columns in REPLICA IDENTITY of the relation, for all - * the publications which the relation is in, are valid - i.e. when - * unpublished generated columns are not part of REPLICA IDENTITY or the - * table does not publish UPDATEs or DELETEs. + * 3. All generated columns in REPLICA IDENTITY of the relation, are valid + * - i.e. when all these generated columns are published. * * XXX We could optimize it by first checking whether any of the - * publications have a row filter for this relation. If not and relation - * has replica identity then we can avoid building the descriptor but as - * this happens only one time it doesn't seem worth the additional + * publications have a row filter or column list for this relation, or if + * the relation contains a generated column. If none of these exist and the + * relation has replica identity then we can avoid building the descriptor + * but as this happens only one time it doesn't seem worth the additional * complexity. */ RelationBuildPublicationDesc(rel, &pubdesc); -- 2.30.0.windows.2