On Friday, November 29, 2024 9:08 PM Shlok Kyal <shlok.kyal....@gmail.com>
wrote:
>
>
> Thanks for reviewing the patch. I have fixed the issue and updated the patch.
Thanks for updating the patch. I have reviewed and have few suggestions.
Please check the attached diff which includes:
1) Comments in CheckCmdReplicaIdentity()
* Removed some duplicate descriptions.
* Fixed the comments for the column list which I think is not correct.
* Mentioned the column list and generated column in XXX part as well.
2) Doc
* Since we mentioned the restriction for UPDATEs and DELTEs when row filter or
column list is defined in the create_publication.sgml, I feel we may need to
mention the generated part as well. So, added in the diff.
3) pub_contains_invalid_column
* Simplified one condition a bit.
Please check and merge if it looks good to you.
Best Regards,
Hou zj
From abdc729ab5e880543e4702d65e8ea66533325d71 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.f...@cn.fujitsu.com>
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 <replaceable
class="parameter">name</replaceable>
system columns.
</para>
+ <para>
+ If tables added to a publication include generated columns that are part of
+ the <literal>REPLICA IDENTITY</literal>, it is essential to publish these
+ columns by explicitly listing them in the column list or by enabling the
+ <literal>publish_generated_columns</literal> option. Otherwise,
+ <command>UPDATE</command> or <command>DELETE</command> operations will be
+ disallowed on those tables.
+ </para>
+
<para>
The row filter on a table becomes redundant if
<literal>FOR TABLES IN SCHEMA</literal> 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
