v39-0004-PS-Row-filter-validation-of-replica-identity.patch

From 0bbe943e5d986ff3f3d84019e9b1d5648c0cafa5 Mon Sep 17 00:00:00 2001
From: Ajin Cherian <ajinc@fast.au.fujitsu.com>
Date: Thu, 11 Nov 2021 22:27:49 -0500
Subject: [PATCH v39 4/6] PS - Row filter validation of replica identity.

This patch introduces some additional row filter validation. Currently it is
implemented only for the publish mode "delete" and it validates that any columns
referenced in the filter expression must be part of REPLICA IDENTITY or Primary
Key.

Also updated test code and PG docs.

Author: Peter Smith
Discussion: https://www.postgresql.org/message-id/CAA4eK1Kyax-qnVPcXzODu3JmA4vtgAjUSYPUK1Pm3vBL5gC81g%40mail.gmail.com
---
 doc/src/sgml/ref/create_publication.sgml  |   5 +-
 src/backend/catalog/pg_publication.c      |  97 ++++++++++++++++++++++++++-
 src/test/regress/expected/publication.out | 105 +++++++++++++++++++++++++++---
 src/test/regress/sql/publication.sql      |  83 ++++++++++++++++++++++-
 src/test/subscription/t/025_row_filter.pl |   7 +-
 5 files changed, 277 insertions(+), 20 deletions(-)

diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml
index 52f6a1c..03cc956 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -231,8 +231,9 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
 
   <para>
    The <literal>WHERE</literal> clause should contain only columns that are
-   part of the primary key or be covered  by <literal>REPLICA
-   IDENTITY</literal> otherwise, <command>DELETE</command> operations will not
+   covered  by <literal>REPLICA IDENTITY</literal>, or are part of the primary
+   key (when <literal>REPLICA IDENTITY</literal> is not set), otherwise
+   <command>DELETE</command> operations will not
    be replicated. That's because old row is used and it only contains primary
    key or columns that are part of the <literal>REPLICA IDENTITY</literal>; the
    remaining columns are <literal>NULL</literal>. For <command>INSERT</command>
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 57d08e7..eadb6d0 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -36,6 +36,7 @@
 #include "commands/publicationcmds.h"
 #include "funcapi.h"
 #include "miscadmin.h"
+#include "nodes/nodeFuncs.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_collate.h"
 #include "parser/parse_relation.h"
@@ -213,10 +214,99 @@ pg_relation_is_publishable(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(result);
 }
 
+typedef struct {
+	Relation	rel;
+	Bitmapset  *bms_replident;
+}
+rf_context;
+
 /*
- * Gets the relations based on the publication partition option for a specified
- * relation.
+ * Walk the row-filter expression to find check that all the referenced columns
+ * are permitted, else error.
  */
+static bool
+rowfilter_expr_replident_walker(Node *node, rf_context *context)
+{
+	if (node == NULL)
+		return false;
+
+	if (IsA(node, Var))
+	{
+		Oid			relid = RelationGetRelid(context->rel);
+		Var		   *var = (Var *) node;
+		AttrNumber	attnum = var->varattno;
+
+		if (!bms_is_member(attnum - FirstLowInvalidHeapAttributeNumber, context->bms_replident))
+		{
+			const char *colname = get_attname(relid, attnum, false);
+
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+					errmsg("cannot add relation \"%s\" to publication",
+						   RelationGetRelationName(context->rel)),
+					errdetail("Row filter column \"%s\" is not part of the REPLICA IDENTITY",
+							  colname)));
+		}
+
+		return true;
+	}
+
+	return expression_tree_walker(node, rowfilter_expr_replident_walker,
+								  (void *) context);
+}
+
+/*
+ * Decide if the row-filter is valid according to the following rules:
+ *
+ * Rule 1. If the publish operation contains "delete" then only columns that
+ * are allowed by the REPLICA IDENTITY rules are permitted to be used in the
+ * row-filter WHERE clause.
+ *
+ * Rule 2. TODO
+ */
+static void
+rowfilter_expr_checker(Publication *pub, Node *rfnode, Relation rel)
+{
+	/*
+	 * Rule 1: For "delete", check that filter cols are also valid replica
+	 * identity cols.
+	 *
+	 * TODO - check later for publish "update" case.
+	 */
+	if (pub->pubactions.pubdelete)
+	{
+		char replica_identity = rel->rd_rel->relreplident;
+
+		if (replica_identity == REPLICA_IDENTITY_FULL)
+		{
+			/*
+			 * FULL means all cols are in the REPLICA IDENTITY, so all cols are
+			 * allowed in the row-filter too.
+			 */
+		}
+		else
+		{
+			Bitmapset *bms_okcols;
+			rf_context context = {0};
+
+			/*
+			 * Find what are the cols that are part of the REPLICA IDENTITY.
+			 * Note that REPLICA IDENTIY DEFAULT means primary key or nothing.
+			 */
+			if (replica_identity == REPLICA_IDENTITY_DEFAULT)
+				bms_okcols = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_PRIMARY_KEY);
+			else
+				bms_okcols = RelationGetIndexAttrBitmap(rel, INDEX_ATTR_BITMAP_IDENTITY_KEY);
+
+			context.rel = rel;
+			context.bms_replident = bms_okcols;
+			(void) rowfilter_expr_replident_walker(rfnode, &context);
+
+			bms_free(bms_okcols);
+		}
+	}
+}
+
 List *
 GetPubPartitionOptionRelations(List *result, PublicationPartOpt pub_partopt,
 							   Oid relid)
@@ -315,6 +405,9 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
 
 		/* Fix up collation information */
 		assign_expr_collations(pstate, whereclause);
+
+		/* Validate the row-filter. */
+		rowfilter_expr_checker(pub, whereclause, targetrel);
 	}
 
 	/* Form a tuple. */
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 9e7f81d..5e7fe01 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -248,13 +248,15 @@ CREATE TABLE testpub_rf_myschema.testpub_rf_tbl5(h integer);
 CREATE SCHEMA testpub_rf_myschema1;
 CREATE TABLE testpub_rf_myschema1.testpub_rf_tb16(i integer);
 SET client_min_messages = 'ERROR';
-CREATE PUBLICATION testpub5 FOR TABLE testpub_rf_tbl1, testpub_rf_tbl2 WHERE (c <> 'test' AND d < 5);
+-- Firstly, test using the option publish="insert" because the row filter
+-- validation of referenced columns is less strict than for delete/update.
+CREATE PUBLICATION testpub5 FOR TABLE testpub_rf_tbl1, testpub_rf_tbl2 WHERE (c <> 'test' AND d < 5) WITH (publish = "insert");
 RESET client_min_messages;
 \dRp+ testpub5
                                     Publication testpub5
           Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
 --------------------------+------------+---------+---------+---------+-----------+----------
- regress_publication_user | f          | t       | t       | t       | t         | f
+ regress_publication_user | f          | t       | f       | f       | f         | f
 Tables:
     "public.testpub_rf_tbl1"
     "public.testpub_rf_tbl2" WHERE (((c <> 'test'::text) AND (d < 5)))
@@ -264,7 +266,7 @@ ALTER PUBLICATION testpub5 ADD TABLE testpub_rf_tbl3 WHERE (e > 1000 AND e < 200
                                     Publication testpub5
           Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
 --------------------------+------------+---------+---------+---------+-----------+----------
- regress_publication_user | f          | t       | t       | t       | t         | f
+ regress_publication_user | f          | t       | f       | f       | f         | f
 Tables:
     "public.testpub_rf_tbl1"
     "public.testpub_rf_tbl2" WHERE (((c <> 'test'::text) AND (d < 5)))
@@ -275,7 +277,7 @@ ALTER PUBLICATION testpub5 DROP TABLE testpub_rf_tbl2;
                                     Publication testpub5
           Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
 --------------------------+------------+---------+---------+---------+-----------+----------
- regress_publication_user | f          | t       | t       | t       | t         | f
+ regress_publication_user | f          | t       | f       | f       | f         | f
 Tables:
     "public.testpub_rf_tbl1"
     "public.testpub_rf_tbl3" WHERE (((e > 1000) AND (e < 2000)))
@@ -286,7 +288,7 @@ ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (e > 300 AND e < 500)
                                     Publication testpub5
           Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
 --------------------------+------------+---------+---------+---------+-----------+----------
- regress_publication_user | f          | t       | t       | t       | t         | f
+ regress_publication_user | f          | t       | f       | f       | f         | f
 Tables:
     "public.testpub_rf_tbl3" WHERE (((e > 300) AND (e < 500)))
 
@@ -310,26 +312,26 @@ Publications:
 DROP PUBLICATION testpub5a, testpub5b, testpub5c;
 -- some more syntax tests to exercise other parser pathways
 SET client_min_messages = 'ERROR';
-CREATE PUBLICATION testpub_syntax1 FOR TABLE testpub_rf_tbl1, ONLY testpub_rf_tbl3 WHERE (e < 999);
+CREATE PUBLICATION testpub_syntax1 FOR TABLE testpub_rf_tbl1, ONLY testpub_rf_tbl3 WHERE (e < 999) WITH (publish = "insert");
 RESET client_min_messages;
 \dRp+ testpub_syntax1
                                 Publication testpub_syntax1
           Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
 --------------------------+------------+---------+---------+---------+-----------+----------
- regress_publication_user | f          | t       | t       | t       | t         | f
+ regress_publication_user | f          | t       | f       | f       | f         | f
 Tables:
     "public.testpub_rf_tbl1"
     "public.testpub_rf_tbl3" WHERE ((e < 999))
 
 DROP PUBLICATION testpub_syntax1;
 SET client_min_messages = 'ERROR';
-CREATE PUBLICATION testpub_syntax2 FOR TABLE testpub_rf_tbl1, testpub_rf_myschema.testpub_rf_tbl5 WHERE (h < 999);
+CREATE PUBLICATION testpub_syntax2 FOR TABLE testpub_rf_tbl1, testpub_rf_myschema.testpub_rf_tbl5 WHERE (h < 999) WITH (publish = "insert");
 RESET client_min_messages;
 \dRp+ testpub_syntax2
                                 Publication testpub_syntax2
           Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
 --------------------------+------------+---------+---------+---------+-----------+----------
- regress_publication_user | f          | t       | t       | t       | t         | f
+ regress_publication_user | f          | t       | f       | f       | f         | f
 Tables:
     "public.testpub_rf_tbl1"
     "testpub_rf_myschema.testpub_rf_tbl5" WHERE ((h < 999))
@@ -387,6 +389,91 @@ DROP PUBLICATION testpub5;
 DROP PUBLICATION testpub7;
 DROP OPERATOR =#>(integer, integer);
 DROP FUNCTION testpub_rf_func(integer, integer);
+-- ======================================================
+-- More row filter tests for validating column references
+CREATE TABLE rf_tbl_abcd_nopk(a int, b int, c int, d int);
+CREATE TABLE rf_tbl_abcd_pk(a int, b int, c int, d int, PRIMARY KEY(a,b));
+-- Case 1. REPLICA IDENTITY DEFAULT (means use primary key or nothing)
+-- 1a. REPLICA IDENTITY is DEFAULT and table has a PK.
+-- ok - "a" is a PK col
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (a > 99);
+RESET client_min_messages;
+DROP PUBLICATION testpub6;
+-- ok - "b" is a PK col
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (b > 99);
+RESET client_min_messages;
+DROP PUBLICATION testpub6;
+-- fail - "c" is not part of the PK
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_pk" to publication
+DETAIL:  Row filter column "c" is not part of the REPLICA IDENTITY
+-- fail - "d" is not part of the PK
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (d > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_pk" to publication
+DETAIL:  Row filter column "d" is not part of the REPLICA IDENTITY
+-- 1b. REPLICA IDENTITY is DEFAULT and table has no PK
+-- fail - "a" is not part of REPLICA IDENTITY
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_nopk" to publication
+DETAIL:  Row filter column "a" is not part of the REPLICA IDENTITY
+-- Case 2. REPLICA IDENTITY FULL
+ALTER TABLE rf_tbl_abcd_pk REPLICA IDENTITY FULL;
+ALTER TABLE rf_tbl_abcd_nopk REPLICA IDENTITY FULL;
+-- ok - "c" is in REPLICA IDENTITY now even though not in PK
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+-- ok - "a" is in REPLICA IDENTITY now
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+-- Case 3. REPLICA IDENTITY NOTHING
+ALTER TABLE rf_tbl_abcd_pk REPLICA IDENTITY NOTHING;
+ALTER TABLE rf_tbl_abcd_nopk REPLICA IDENTITY NOTHING;
+-- fail - "a" is in PK but it is not part of REPLICA IDENTITY NOTHING
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (a > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_pk" to publication
+DETAIL:  Row filter column "a" is not part of the REPLICA IDENTITY
+-- fail - "c" is not in PK and not in REPLICA IDENTITY NOTHING
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_pk" to publication
+DETAIL:  Row filter column "c" is not part of the REPLICA IDENTITY
+-- fail - "a" is not in REPLICA IDENTITY NOTHING
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_nopk" to publication
+DETAIL:  Row filter column "a" is not part of the REPLICA IDENTITY
+-- Case 4. REPLICA IDENTITY INDEX
+ALTER TABLE rf_tbl_abcd_pk ALTER COLUMN c SET NOT NULL;
+CREATE UNIQUE INDEX idx_abcd_pk_c ON rf_tbl_abcd_pk(c);
+ALTER TABLE rf_tbl_abcd_pk REPLICA IDENTITY USING INDEX idx_abcd_pk_c;
+ALTER TABLE rf_tbl_abcd_nopk ALTER COLUMN c SET NOT NULL;
+CREATE UNIQUE INDEX idx_abcd_nopk_c ON rf_tbl_abcd_nopk(c);
+ALTER TABLE rf_tbl_abcd_nopk REPLICA IDENTITY USING INDEX idx_abcd_nopk_c;
+-- fail - "a" is in PK but it is not part of REPLICA IDENTITY INDEX
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (a > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_pk" to publication
+DETAIL:  Row filter column "a" is not part of the REPLICA IDENTITY
+-- ok - "c" is not in PK but it is part of REPLICA IDENTITY INDEX
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+-- fail - "a" is not in REPLICA IDENTITY INDEX
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+ERROR:  cannot add relation "rf_tbl_abcd_nopk" to publication
+DETAIL:  Row filter column "a" is not part of the REPLICA IDENTITY
+-- ok - "c" is part of REPLICA IDENTITY INDEX
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (c > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+DROP TABLE rf_tbl_abcd_pk;
+DROP TABLE rf_tbl_abcd_nopk;
+-- ======================================================
 -- Test cache invalidation FOR ALL TABLES publication
 SET client_min_messages = 'ERROR';
 CREATE TABLE testpub_tbl4(a int);
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 21cc923..b127605 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -143,7 +143,9 @@ CREATE TABLE testpub_rf_myschema.testpub_rf_tbl5(h integer);
 CREATE SCHEMA testpub_rf_myschema1;
 CREATE TABLE testpub_rf_myschema1.testpub_rf_tb16(i integer);
 SET client_min_messages = 'ERROR';
-CREATE PUBLICATION testpub5 FOR TABLE testpub_rf_tbl1, testpub_rf_tbl2 WHERE (c <> 'test' AND d < 5);
+-- Firstly, test using the option publish="insert" because the row filter
+-- validation of referenced columns is less strict than for delete/update.
+CREATE PUBLICATION testpub5 FOR TABLE testpub_rf_tbl1, testpub_rf_tbl2 WHERE (c <> 'test' AND d < 5) WITH (publish = "insert");
 RESET client_min_messages;
 \dRp+ testpub5
 ALTER PUBLICATION testpub5 ADD TABLE testpub_rf_tbl3 WHERE (e > 1000 AND e < 2000);
@@ -163,12 +165,12 @@ RESET client_min_messages;
 DROP PUBLICATION testpub5a, testpub5b, testpub5c;
 -- some more syntax tests to exercise other parser pathways
 SET client_min_messages = 'ERROR';
-CREATE PUBLICATION testpub_syntax1 FOR TABLE testpub_rf_tbl1, ONLY testpub_rf_tbl3 WHERE (e < 999);
+CREATE PUBLICATION testpub_syntax1 FOR TABLE testpub_rf_tbl1, ONLY testpub_rf_tbl3 WHERE (e < 999) WITH (publish = "insert");
 RESET client_min_messages;
 \dRp+ testpub_syntax1
 DROP PUBLICATION testpub_syntax1;
 SET client_min_messages = 'ERROR';
-CREATE PUBLICATION testpub_syntax2 FOR TABLE testpub_rf_tbl1, testpub_rf_myschema.testpub_rf_tbl5 WHERE (h < 999);
+CREATE PUBLICATION testpub_syntax2 FOR TABLE testpub_rf_tbl1, testpub_rf_myschema.testpub_rf_tbl5 WHERE (h < 999) WITH (publish = "insert");
 RESET client_min_messages;
 \dRp+ testpub_syntax2
 DROP PUBLICATION testpub_syntax2;
@@ -209,6 +211,81 @@ DROP PUBLICATION testpub7;
 DROP OPERATOR =#>(integer, integer);
 DROP FUNCTION testpub_rf_func(integer, integer);
 
+-- ======================================================
+-- More row filter tests for validating column references
+CREATE TABLE rf_tbl_abcd_nopk(a int, b int, c int, d int);
+CREATE TABLE rf_tbl_abcd_pk(a int, b int, c int, d int, PRIMARY KEY(a,b));
+
+-- Case 1. REPLICA IDENTITY DEFAULT (means use primary key or nothing)
+-- 1a. REPLICA IDENTITY is DEFAULT and table has a PK.
+-- ok - "a" is a PK col
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (a > 99);
+RESET client_min_messages;
+DROP PUBLICATION testpub6;
+-- ok - "b" is a PK col
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (b > 99);
+RESET client_min_messages;
+DROP PUBLICATION testpub6;
+-- fail - "c" is not part of the PK
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+-- fail - "d" is not part of the PK
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (d > 99);
+-- 1b. REPLICA IDENTITY is DEFAULT and table has no PK
+-- fail - "a" is not part of REPLICA IDENTITY
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+
+-- Case 2. REPLICA IDENTITY FULL
+ALTER TABLE rf_tbl_abcd_pk REPLICA IDENTITY FULL;
+ALTER TABLE rf_tbl_abcd_nopk REPLICA IDENTITY FULL;
+-- ok - "c" is in REPLICA IDENTITY now even though not in PK
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+-- ok - "a" is in REPLICA IDENTITY now
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+
+-- Case 3. REPLICA IDENTITY NOTHING
+ALTER TABLE rf_tbl_abcd_pk REPLICA IDENTITY NOTHING;
+ALTER TABLE rf_tbl_abcd_nopk REPLICA IDENTITY NOTHING;
+-- fail - "a" is in PK but it is not part of REPLICA IDENTITY NOTHING
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (a > 99);
+-- fail - "c" is not in PK and not in REPLICA IDENTITY NOTHING
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+-- fail - "a" is not in REPLICA IDENTITY NOTHING
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+
+-- Case 4. REPLICA IDENTITY INDEX
+ALTER TABLE rf_tbl_abcd_pk ALTER COLUMN c SET NOT NULL;
+CREATE UNIQUE INDEX idx_abcd_pk_c ON rf_tbl_abcd_pk(c);
+ALTER TABLE rf_tbl_abcd_pk REPLICA IDENTITY USING INDEX idx_abcd_pk_c;
+ALTER TABLE rf_tbl_abcd_nopk ALTER COLUMN c SET NOT NULL;
+CREATE UNIQUE INDEX idx_abcd_nopk_c ON rf_tbl_abcd_nopk(c);
+ALTER TABLE rf_tbl_abcd_nopk REPLICA IDENTITY USING INDEX idx_abcd_nopk_c;
+-- fail - "a" is in PK but it is not part of REPLICA IDENTITY INDEX
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (a > 99);
+-- ok - "c" is not in PK but it is part of REPLICA IDENTITY INDEX
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_pk WHERE (c > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+-- fail - "a" is not in REPLICA IDENTITY INDEX
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (a > 99);
+-- ok - "c" is part of REPLICA IDENTITY INDEX
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub6 FOR TABLE rf_tbl_abcd_nopk WHERE (c > 99);
+DROP PUBLICATION testpub6;
+RESET client_min_messages;
+
+DROP TABLE rf_tbl_abcd_pk;
+DROP TABLE rf_tbl_abcd_nopk;
+-- ======================================================
+
 -- Test cache invalidation FOR ALL TABLES publication
 SET client_min_messages = 'ERROR';
 CREATE TABLE testpub_tbl4(a int);
diff --git a/src/test/subscription/t/025_row_filter.pl b/src/test/subscription/t/025_row_filter.pl
index e806b5d..dff55c2 100644
--- a/src/test/subscription/t/025_row_filter.pl
+++ b/src/test/subscription/t/025_row_filter.pl
@@ -19,6 +19,8 @@ $node_subscriber->start;
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab_rowfilter_1 (a int primary key, b text)");
 $node_publisher->safe_psql('postgres',
+	"ALTER TABLE tab_rowfilter_1 REPLICA IDENTITY FULL;");
+$node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab_rowfilter_2 (c int primary key)");
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab_rowfilter_3 (a int primary key, b boolean)");
@@ -223,9 +225,7 @@ $node_publisher->wait_for_catchup($appname);
 # - INSERT (1700, 'test 1700') YES, because 1700 > 1000 and 'test 1700' <> 'filtered'
 # - UPDATE (1600, NULL)        NO, row filter evaluates to false because NULL is not <> 'filtered'
 # - UPDATE (1601, 'test 1601 updated') YES, because 1601 > 1000 and 'test 1601 updated' <> 'filtered'
-# - DELETE (1700)              NO, row filter contains column b that is not part of
-# the PK or REPLICA IDENTITY and old tuple contains b = NULL, hence, row filter
-# evaluates to false
+# - DELETE (1700)              YES, because 1700 > 1000 and 'test 1700' <> 'filtered'
 #
 $result =
   $node_subscriber->safe_psql('postgres',
@@ -234,7 +234,6 @@ is($result, qq(1001|test 1001
 1002|test 1002
 1600|test 1600
 1601|test 1601 updated
-1700|test 1700
 1980|not filtered), 'check replicated rows to table tab_rowfilter_1');
 
 # Publish using root partitioned table
-- 
1.8.3.1