0005-Fix-REFRESH-MATERIALIZED-VIEW-CONCURRENTLY-performance_bug2.patch

From 15fb766ee6cf7245092fabd7c506b880f9f0b6b6 Mon Sep 17 00:00:00 2001
From: spoondla <s_poondla@apple.com>
Date: Mon, 2 Mar 2026 11:23:17 -0800
Subject: [PATCH v5] Fix REFRESH MATERIALIZED VIEW CONCURRENTLY performance
 with nullable indexed columns

When a materialized view has a unique index on a nullable column, the join
condition used to detect changes included NULL = NULL comparisons which
evaluate to NULL (false), causing every row with a NULL indexed value to
appear as changed on every CONCURRENTLY refresh even when the data was
unchanged.

The fix skips nullable columns from the per-column join conditions when the
index has at least one NOT NULL column.  The NOT NULL column(s) are sufficient
to identify matching rows uniquely, so omitting the nullable ones avoids the
unnecessary NULL = NULL comparisons.  The record equality operator (*=),
appended to the join condition unconditionally, handles all columns including
nullable ones and ensures changed rows are still detected correctly.

When all columns of a unique index are nullable, no columns are skipped.
Relying on *= alone in that case would be incorrect: *= treats NULL as equal
to NULL, which can cause join ambiguity or silent data loss when rows contain
all-null values.  Falling back to including all columns preserves the original
(slower but correct) behavior for that edge case.

Added regression tests covering both the optimization path (index with at
least one NOT NULL column) and the fallback path (all-nullable index).
---
 src/backend/commands/matview.c        | 60 +++++++++++++++++++++++++--
 src/test/regress/expected/matview.out | 29 +++++++++++++
 src/test/regress/sql/matview.sql      | 24 +++++++++++
 3 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 81a55a33ef2..b0d7c59444a 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -602,6 +602,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 	char	   *nsp;
 	TupleDesc	tupdesc;
 	bool		foundUniqueIndex;
+	bool		addedAnyQuals;
 	List	   *indexoidlist;
 	ListCell   *indexoidscan;
 	int16		relnatts;
@@ -715,6 +716,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 	tupdesc = matviewRel->rd_att;
 	opUsedForQual = palloc0_array(Oid, relnatts);
 	foundUniqueIndex = false;
+	addedAnyQuals = false;
 
 	indexoidlist = RelationGetIndexList(matviewRel);
 
@@ -731,6 +733,9 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 			oidvector  *indclass;
 			Datum		indclassDatum;
 			int			i;
+			bool		index_has_nonnull;
+
+			foundUniqueIndex = true;
 
 			/* Must get indclass the hard way. */
 			indclassDatum = SysCacheGetAttrNotNull(INDEXRELID,
@@ -738,6 +743,29 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 												   Anum_pg_index_indclass);
 			indclass = (oidvector *) DatumGetPointer(indclassDatum);
 
+			/*
+			 * Pre-scan: check whether this index has at least one NOT NULL
+			 * column.  If it does, we can safely skip nullable columns in
+			 * the join condition below (the NOT NULL column(s) uniquely
+			 * identify rows, so omitting nullable columns does not introduce
+			 * join ambiguity).  If all columns are nullable we must include
+			 * all of them to preserve correctness: skipping everything and
+			 * relying on *= alone can cause join ambiguity or data loss
+			 * because *= treats NULL as equal to NULL.
+			 */
+			index_has_nonnull = false;
+			for (i = 0; i < indnkeyatts; i++)
+			{
+				Form_pg_attribute attr_check =
+					TupleDescAttr(tupdesc, indexStruct->indkey.values[i] - 1);
+
+				if (attr_check->attnotnull)
+				{
+					index_has_nonnull = true;
+					break;
+				}
+			}
+
 			/* Add quals for all columns from this index. */
 			for (i = 0; i < indnkeyatts; i++)
 			{
@@ -753,6 +781,24 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 				const char *leftop;
 				const char *rightop;
 
+				/*
+				 * Skip nullable columns when this index has at least one NOT
+				 * NULL column.  Nullable columns in unique indexes allow
+				 * multiple NULLs and NULL = NULL evaluates to NULL (false) in
+				 * the join, making unchanged NULL-containing rows appear as
+				 * changed on every refresh.  The NOT NULL column(s) are
+				 * sufficient to identify matching rows, so we can safely omit
+				 * nullable ones without introducing join ambiguity.
+				 *
+				 * When all columns are nullable we must not skip any of them:
+				 * with no per-column conditions only *= would remain, which
+				 * can cause join ambiguity or data loss (see pre-scan comment
+				 * above).  We fall back to including all columns so the join
+				 * behaves as in the original code.
+				 */
+				if (index_has_nonnull && !attr->attnotnull)
+					continue;
+
 				/*
 				 * Identify the equality operator associated with this index
 				 * column.  First we need to look up the column's opclass.
@@ -788,7 +834,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 				/*
 				 * Actually add the qual, ANDed with any others.
 				 */
-				if (foundUniqueIndex)
+				if (addedAnyQuals)
 					appendStringInfoString(&querybuf, " AND ");
 
 				leftop = quote_qualified_identifier("newdata",
@@ -801,7 +847,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 										 op,
 										 rightop, attrtype);
 
-				foundUniqueIndex = true;
+				addedAnyQuals = true;
 			}
 		}
 
@@ -826,8 +872,16 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner,
 				errmsg("could not find suitable unique index on materialized view \"%s\"",
 					   RelationGetRelationName(matviewRel)));
 
+	/*
+	 * The record equality operator (*=) is always included in the join
+	 * predicate.  It handles all columns correctly including nullable ones
+	 * that were skipped above, since *=  treats NULL as equal to NULL.
+	 */
+	if (addedAnyQuals)
+		appendStringInfoString(&querybuf, " AND ");
+
 	appendStringInfoString(&querybuf,
-						   " AND newdata.* OPERATOR(pg_catalog.*=) mv.*) "
+						   "newdata.* OPERATOR(pg_catalog.*=) mv.*) "
 						   "WHERE newdata.* IS NULL OR mv.* IS NULL "
 						   "ORDER BY tid");
 
diff --git a/src/test/regress/expected/matview.out b/src/test/regress/expected/matview.out
index 0355720dfc6..98db1e08185 100644
--- a/src/test/regress/expected/matview.out
+++ b/src/test/regress/expected/matview.out
@@ -418,6 +418,35 @@ CREATE MATERIALIZED VIEW mvtest_mv2 AS SELECT * FROM mvtest_mv1
   WHERE col1 = (SELECT LEAST(col1) FROM mvtest_mv1) WITH NO DATA;
 DROP MATERIALIZED VIEW mvtest_mv1 CASCADE;
 NOTICE:  drop cascades to materialized view mvtest_mv2
+-- test that nullable indexed columns are skipped in the join condition when
+-- the index has at least one NOT NULL column.  The NOT NULL column alone
+-- identifies matching rows, so the nullable column does not need to be in
+-- the join and its NULL values do not cause unnecessary DELETE+INSERT churn.
+CREATE TABLE mvtest_foo(a int NOT NULL, b int);
+INSERT INTO mvtest_foo VALUES(1, NULL), (2, NULL), (3, NULL);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);  -- a is NOT NULL, b is nullable
+-- Add a second index on the nullable column alone to make it exercise the
+-- all-nullable fallback path through the pre-scan.
+CREATE UNIQUE INDEX ON mvtest_mv(b);
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;  -- must succeed
+DROP TABLE mvtest_foo CASCADE;
+NOTICE:  drop cascades to materialized view mvtest_mv
+-- test that CONCURRENTLY still works when all indexed columns are nullable
+-- (falls back to including all columns, which is slower but correct).
+CREATE TABLE mvtest_foo(a int, b int);
+INSERT INTO mvtest_foo VALUES(NULL, 1);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);  -- all-nullable index
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;  -- must succeed
+SELECT * FROM mvtest_mv;
+ a | b 
+---+---
+   | 1
+(1 row)
+
+DROP TABLE mvtest_foo CASCADE;
+NOTICE:  drop cascades to materialized view mvtest_mv
 -- make sure that types with unusual equality tests work
 CREATE TABLE mvtest_boxes (id serial primary key, b box);
 INSERT INTO mvtest_boxes (b) VALUES
diff --git a/src/test/regress/sql/matview.sql b/src/test/regress/sql/matview.sql
index 934426b9ae8..624056a4dc5 100644
--- a/src/test/regress/sql/matview.sql
+++ b/src/test/regress/sql/matview.sql
@@ -153,6 +153,30 @@ CREATE MATERIALIZED VIEW mvtest_mv2 AS SELECT * FROM mvtest_mv1
   WHERE col1 = (SELECT LEAST(col1) FROM mvtest_mv1) WITH NO DATA;
 DROP MATERIALIZED VIEW mvtest_mv1 CASCADE;
 
+-- test that nullable indexed columns are skipped in the join condition when
+-- the index has at least one NOT NULL column.  The NOT NULL column alone
+-- identifies matching rows, so the nullable column does not need to be in
+-- the join and its NULL values do not cause unnecessary DELETE+INSERT churn.
+CREATE TABLE mvtest_foo(a int NOT NULL, b int);
+INSERT INTO mvtest_foo VALUES(1, NULL), (2, NULL), (3, NULL);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);  -- a is NOT NULL, b is nullable
+-- Add a second index on the nullable column alone to make it exercise the
+-- all-nullable fallback path through the pre-scan.
+CREATE UNIQUE INDEX ON mvtest_mv(b);
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;  -- must succeed
+DROP TABLE mvtest_foo CASCADE;
+
+-- test that CONCURRENTLY still works when all indexed columns are nullable
+-- (falls back to including all columns, which is slower but correct).
+CREATE TABLE mvtest_foo(a int, b int);
+INSERT INTO mvtest_foo VALUES(NULL, 1);
+CREATE MATERIALIZED VIEW mvtest_mv AS SELECT * FROM mvtest_foo;
+CREATE UNIQUE INDEX ON mvtest_mv(a);  -- all-nullable index
+REFRESH MATERIALIZED VIEW CONCURRENTLY mvtest_mv;  -- must succeed
+SELECT * FROM mvtest_mv;
+DROP TABLE mvtest_foo CASCADE;
+
 -- make sure that types with unusual equality tests work
 CREATE TABLE mvtest_boxes (id serial primary key, b box);
 INSERT INTO mvtest_boxes (b) VALUES
-- 
2.39.5 (Apple Git-154)