0005-Fix-REFRESH-MATERIALIZED-VIEW-CONCURRENTLY-to-detect_bug1.patch

From f8419fe83c6e6831ff987b53f2bcae2f3173029b Mon Sep 17 00:00:00 2001
From: spoondla <s_poondla@apple.com>
Date: Mon, 2 Mar 2026 11:22:11 -0800
Subject: [PATCH v5] Fix REFRESH MATERIALIZED VIEW CONCURRENTLY to detect
 duplicate rows

REFRESH MATERIALIZED VIEW CONCURRENTLY was incorrectly skipping
duplicate detection for rows containing NULL values. The pre-check
query used "WHERE newdata.* IS NOT NULL" which caused rows with any
NULL column to bypass duplicate detection entirely.

The fix rebuilds the pre-check using the same per-column equality
operators used by the diff JOIN (one condition per unique-indexed
column).  These operators treat NULL as not equal to NULL, matching
unique index semantics where NULLs are considered distinct.  Only
rows whose indexed columns are non-null and equal are flagged as
duplicates --- the same rows that would cause join ambiguity.

This correctly handles two cases that the old approach got wrong:
  - (test, NULL) x2 with index on a: a='test' is non-null and
    duplicated, so the duplicate is correctly detected and an error
    is raised.
  - (NULL, NULL) x2 with index on a: a=NULL, and unique indexes
    allow multiple NULLs (each is treated as distinct), so the
    refresh correctly succeeds and updates the view to two rows.

Added regression tests covering both cases.


---
 src/backend/commands/matview.c        | 122 ++++++++++++++++++--------
 src/test/regress/expected/matview.out |  33 ++++++-
 src/test/regress/sql/matview.sql      |  22 +++++
 3 files changed, 137 insertions(+), 40 deletions(-)

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 81a55a33ef2..2a1694f3b4e 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -606,6 +606,8 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 	ListCell   *indexoidscan;
 	int16		relnatts;
 	Oid		   *opUsedForQual;
+	StringInfoData precheck_cond_buf;
+	bool		precheck_has_cond;
 
 	initStringInfo(&querybuf);
 	matviewRel = table_open(matviewOid, NoLock);
@@ -634,45 +636,6 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 	if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY)
 		elog(ERROR, "SPI_exec failed: %s", querybuf.data);
 
-	/*
-	 * We need to ensure that there are not duplicate rows without NULLs in
-	 * the new data set before we can count on the "diff" results.  Check for
-	 * that in a way that allows showing the first duplicated row found.  Even
-	 * after we pass this test, a unique index on the materialized view may
-	 * find a duplicate key problem.
-	 *
-	 * Note: here and below, we use "tablename.*::tablerowtype" as a hack to
-	 * keep ".*" from being expanded into multiple columns in a SELECT list.
-	 * Compare ruleutils.c's get_variable().
-	 */
-	resetStringInfo(&querybuf);
-	appendStringInfo(&querybuf,
-					 "SELECT newdata.*::%s FROM %s newdata "
-					 "WHERE newdata.* IS NOT NULL AND EXISTS "
-					 "(SELECT 1 FROM %s newdata2 WHERE newdata2.* IS NOT NULL "
-					 "AND newdata2.* OPERATOR(pg_catalog.*=) newdata.* "
-					 "AND newdata2.ctid OPERATOR(pg_catalog.<>) "
-					 "newdata.ctid)",
-					 tempname, tempname, tempname);
-	if (SPI_execute(querybuf.data, false, 1) != SPI_OK_SELECT)
-		elog(ERROR, "SPI_exec failed: %s", querybuf.data);
-	if (SPI_processed > 0)
-	{
-		/*
-		 * Note that this ereport() is returning data to the user.  Generally,
-		 * we would want to make sure that the user has been granted access to
-		 * this data.  However, REFRESH MAT VIEW is only able to be run by the
-		 * owner of the mat view (or a superuser) and therefore there is no
-		 * need to check for access to data in the mat view.
-		 */
-		ereport(ERROR,
-				(errcode(ERRCODE_CARDINALITY_VIOLATION),
-				 errmsg("new data for materialized view \"%s\" contains duplicate rows without any null columns",
-						RelationGetRelationName(matviewRel)),
-				 errdetail("Row: %s",
-						   SPI_getvalue(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1))));
-	}
-
 	/*
 	 * Create the temporary "diff" table.
 	 *
@@ -715,6 +678,8 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 	tupdesc = matviewRel->rd_att;
 	opUsedForQual = palloc0_array(Oid, relnatts);
 	foundUniqueIndex = false;
+	initStringInfo(&precheck_cond_buf);
+	precheck_has_cond = false;
 
 	indexoidlist = RelationGetIndexList(matviewRel);
 
@@ -802,6 +767,30 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 										 rightop, attrtype);
 
 				foundUniqueIndex = true;
+
+				/*
+				 * Also accumulate the same condition for the duplicate-row
+				 * pre-check, comparing newdata2 against newdata (instead of
+				 * newdata against mv).  This lets us detect rows that are
+				 * duplicates with respect to the unique index semantics ---
+				 * i.e. rows whose indexed columns are non-null and equal ---
+				 * without falsely flagging rows whose indexed columns are
+				 * NULL (which unique indexes treat as distinct).
+				 */
+				if (precheck_has_cond)
+					appendStringInfoString(&precheck_cond_buf, " AND ");
+
+				leftop = quote_qualified_identifier("newdata2",
+														NameStr(attr->attname));
+				rightop = quote_qualified_identifier("newdata",
+														 NameStr(attr->attname));
+
+				generate_operator_clause(&precheck_cond_buf,
+										 leftop, attrtype,
+										 op,
+										 rightop, attrtype);
+
+				precheck_has_cond = true;
 			}
 		}
 
@@ -831,6 +820,61 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 						   "WHERE newdata.* IS NULL OR mv.* IS NULL "
 						   "ORDER BY tid");
 
+	/*
+	 * Before populating the diff table, check for duplicate rows in the
+	 * new data set.  We look for rows that are equal in all unique index
+	 * columns (using the same operators as the join above, where
+	 * NULL = NULL is false) AND are also *=-equal overall.  Such rows
+	 * would both match the same materialized view row in the join,
+	 * producing an ambiguous diff.
+	 *
+	 * Using index column operators rather than *= alone is important: it
+	 * correctly excludes rows whose indexed columns are NULL, because
+	 * unique indexes treat NULLs as distinct so those rows do not cause
+	 * join ambiguity.
+	 *
+	 * Note: here and below, we use "tablename.*::tablerowtype" as a hack
+	 * to keep ".*" from being expanded into multiple columns in a SELECT
+	 * list.  Compare ruleutils.c's get_variable().
+	 *
+	 * Even after we pass this test, a unique index on the materialized
+	 * view may find a duplicate key problem.
+	 */
+	{
+		StringInfoData precheck_querybuf;
+
+		initStringInfo(&precheck_querybuf);
+		appendStringInfo(&precheck_querybuf,
+						 "SELECT newdata.*::%s FROM %s newdata "
+						 "WHERE EXISTS "
+						 "(SELECT 1 FROM %s newdata2 WHERE %s"
+						 " AND newdata2.* OPERATOR(pg_catalog.*=) newdata.*"
+						 " AND newdata2.ctid OPERATOR(pg_catalog.<>) newdata.ctid)",
+						 tempname, tempname, tempname, precheck_cond_buf.data);
+
+		if (SPI_execute(precheck_querybuf.data, false, 1) != SPI_OK_SELECT)
+			elog(ERROR, "SPI_exec failed: %s", precheck_querybuf.data);
+
+		if (SPI_processed > 0)
+		{
+			/*
+			 * Note that this ereport() is returning data to the user.
+			 * Generally, we would want to make sure that the user has been
+			 * granted access to this data.  However, REFRESH MAT VIEW is
+			 * only able to be run by the owner of the mat view (or a
+			 * superuser) and therefore there is no need to check for access
+			 * to data in the mat view.
+			 */
+			ereport(ERROR,
+					(errcode(ERRCODE_CARDINALITY_VIOLATION),
+					 errmsg("new data for materialized view \"%s\" contains duplicate rows",
+							RelationGetRelationName(matviewRel)),
+					 errdetail("Row: %s",
+							   SPI_getvalue(SPI_tuptable->vals[0],
+										   SPI_tuptable->tupdesc, 1))));
+		}
+	}
+
 	/* Populate the temporary "diff" table. */
 	if (SPI_exec(querybuf.data, 0) != SPI_OK_INSERT)
 		elog(ERROR, "SPI_exec failed: %s", querybuf.data);
diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index 0355720dfc6..1dcff26a2e4 100644
--- a/src/test/regress/expected/matview.out
+++ b/src/test/regress/expected/matview.out
@@ -396,8 +396,39 @@ REFRESH MATERIALIZED VIEW mvtest_mv;
 ERROR:  could not create unique index "mvtest_mv_a_idx"
 DETAIL:  Key (a)=(1) is duplicated.
 REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;
-ERROR:  new data for materialized view "mvtest_mv" contains duplicate rows without any null columns
+ERROR:  new data for materialized view "mvtest_mv" contains duplicate rows
 DETAIL:  Row: (1,10)
+DROP TABLE mvtest_foo CASCADE;
+NOTICE:  drop cascades to materialized view mvtest_mv
+-- test that duplicate rows containing NULLs in non-indexed columns are detected
+CREATE TABLE mvtest_foo(a text, b text);
+INSERT INTO mvtest_foo VALUES('test', NULL);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);
+INSERT INTO mvtest_foo VALUES('test', NULL);
+REFRESH MATERIALIZED VIEW mvtest_mv;
+ERROR:  could not create unique index "mvtest_mv_a_idx"
+DETAIL:  Key (a)=(test) is duplicated.
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;
+ERROR:  new data for materialized view "mvtest_mv" contains duplicate rows
+DETAIL:  Row: (test,)
+DROP TABLE mvtest_foo CASCADE;
+NOTICE:  drop cascades to materialized view mvtest_mv
+-- test that rows with NULLs in the indexed column are not false positives:
+-- unique indexes treat NULLs as distinct, so (NULL,NULL)x2 is a valid state
+-- and CONCURRENTLY should succeed and update the view to reflect both rows
+CREATE TABLE mvtest_foo(a int, b int);
+INSERT INTO mvtest_foo VALUES(NULL, NULL);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);
+INSERT INTO mvtest_foo VALUES(NULL, NULL);
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;
+SELECT COUNT(*) FROM mvtest_mv;
+ count 
+-------
+     2
+(1 row)
+
 DROP TABLE mvtest_foo CASCADE;
 NOTICE:  drop cascades to materialized view mvtest_mv
 -- make sure that all columns covered by unique indexes works
diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql
index 934426b9ae8..e65cd4fa94b 100644
--- a/src/test/regress/sql/matview.sql
+++ b/src/test/regress/sql/matview.sql
@@ -135,6 +135,28 @@ REFRESH MATERIALIZED VIEW mvtest_mv;
 REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;
 DROP TABLE mvtest_foo CASCADE;
 
+-- test that duplicate rows containing NULLs in non-indexed columns are detected
+CREATE TABLE mvtest_foo(a text, b text);
+INSERT INTO mvtest_foo VALUES('test', NULL);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);
+INSERT INTO mvtest_foo VALUES('test', NULL);
+REFRESH MATERIALIZED VIEW mvtest_mv;
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;
+DROP TABLE mvtest_foo CASCADE;
+
+-- test that rows with NULLs in the indexed column are not false positives:
+-- unique indexes treat NULLs as distinct, so (NULL,NULL)x2 is a valid state
+-- and CONCURRENTLY should succeed and update the view to reflect both rows
+CREATE TABLE mvtest_foo(a int, b int);
+INSERT INTO mvtest_foo VALUES(NULL, NULL);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);
+INSERT INTO mvtest_foo VALUES(NULL, NULL);
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;
+SELECT COUNT(*) FROM mvtest_mv;
+DROP TABLE mvtest_foo CASCADE;
+
 -- make sure that all columns covered by unique indexes works
 CREATE TABLE mvtest_foo(a, b, c) AS VALUES(1, 2, 3);
 CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
-- 
2.39.5 (Apple Git-154)