v3-0001-Disallow-NULLS-NOT-DISTINCT-indexes-for-primary-k.patch

From 6baea59b788b915c23a9c934cad6c0db5b1ff62e Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Fri, 16 Dec 2022 16:21:38 +0100
Subject: [PATCH v3] Disallow NULLS NOT DISTINCT indexes for primary keys

A unique index which is created with non-distinct NULLS cannot be
used for backing a primary key constraint.  Make sure to disallow
such table alterations and add a check to pg_upgrade to warn when
finding these in an old cluster.

Bug: 17720
Reported-by: Reiner Peterke <zedaardv@drizzle.com>
Discussion: https://postgr.es/m/17720-dab8ee0fa85d316d@postgresql.org
---
 src/backend/catalog/index.c                | 13 +++
 src/bin/pg_upgrade/check.c                 | 93 ++++++++++++++++++++++
 src/test/regress/expected/create_index.out |  6 ++
 src/test/regress/sql/create_index.sql      |  6 ++
 4 files changed, 118 insertions(+)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 61f1d3926a..6ff769911e 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -225,6 +225,19 @@ index_check_primary_key(Relation heapRel,
 						RelationGetRelationName(heapRel))));
 	}
 
+	/*
+	 * Indexes created with NULLS NOT DISTINCT cannot be used for primary key
+	 * constraints. While there is no direct syntax to reach here, it can be
+	 * done by creating a separate index and attaching it via ALTER TABLE ..
+	 * USING INDEX.
+	 */
+	if (indexInfo->ii_NullsNotDistinct)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+				 errmsg("primary keys cannot use NULLS NOT DISTINCT indexes")));
+	}
+
 	/*
 	 * Check that all of the attributes in a primary key are marked as not
 	 * null.  (We don't really expect to see that; it'd mean the parser messed
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index d4a3691438..09828b09e7 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -33,6 +33,7 @@ static void check_for_jsonb_9_4_usage(ClusterInfo *cluster);
 static void check_for_pg_role_prefix(ClusterInfo *cluster);
 static void check_for_new_tablespace_dir(ClusterInfo *new_cluster);
 static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
+static void check_for_nulls_not_distinct_pk(ClusterInfo *cluster);
 static char *get_canonical_locale_name(int category, const char *locale);
 
 
@@ -176,6 +177,13 @@ check_and_dump_old_cluster(bool live_check)
 	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 903)
 		old_9_3_check_for_line_data_type_usage(&old_cluster);
 
+	/*
+	 * Pre-PG 16 allowed indexes defined with NULLS NOT DISTINCT to be used
+	 * for primary key constraints.
+	 */
+	if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1500)
+		check_for_nulls_not_distinct_pk(&old_cluster);
+
 	/*
 	 * While not a check option, we do this now because this is the only time
 	 * the old server is running.
@@ -1525,6 +1533,91 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster)
 		check_ok();
 }
 
+/*
+ * check_for_nulls_not_distinct_pk
+ *
+ * In versions before v16 it was possible to create a primary key constraint
+ * with non-distinct nulls by attaching the index via alter table:
+ *
+ * CREATE UNIQUE INDEX idx ON <rel> (<column>) NULLS NOT DISTINCT;
+ * ALTER TABLE <rel> ADD PRIMARY KEY USING INDEX idx;
+ *
+ * This schema will be dumped by pg_dump but cannot be restored, so make sure
+ * the old cluster doesn't have any such constraints.
+ */
+static void
+check_for_nulls_not_distinct_pk(ClusterInfo *cluster)
+{
+	int			dbnum;
+	FILE	   *script = NULL;
+	char		output_path[MAXPGPATH];
+
+	prep_status("Checking for primary keys with non-distinct null indexes");
+
+	snprintf(output_path, sizeof(output_path), "%s/%s",
+			 log_opts.basedir,
+			 "nulls_not_distinct_pk.txt");
+
+	for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)
+	{
+		PGresult   *res;
+		bool		db_used = false;
+		int			ntups;
+		int			i_relname;
+		int			i_nspname;
+		DbInfo	   *active_db = &cluster->dbarr.dbs[dbnum];
+		PGconn	   *conn = connectToServer(cluster, active_db->db_name);
+
+		res = executeQueryOrDie(conn,
+								"SELECT n.nspname, cl.relname "
+								"FROM pg_catalog.pg_constraint cn "
+								"JOIN pg_catalog.pg_class cl ON "
+								"     (cn.conrelid = cl.oid AND cn.contype = 'p') "
+								"JOIN pg_catalog.pg_namespace n ON "
+								"     (cl.relnamespace = n.oid) "
+								"JOIN pg_catalog.pg_index i ON "
+								"     (cn.conindid = i.indexrelid AND "
+								"	  i.indnullsnotdistinct) "
+								"WHERE cn.contype = 'p'");
+		ntups = PQntuples(res);
+		if (!ntups)
+			continue;
+		i_nspname = PQfnumber(res, "nspname");
+		i_relname = PQfnumber(res, "relname");
+		for (int rowno = 0; rowno < ntups; rowno++)
+		{
+			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
+				pg_fatal("could not open file \"%s\": %s",
+						 output_path, strerror(errno));
+			if (!db_used)
+			{
+				fprintf(script, "In database: %s\n", active_db->db_name);
+				db_used = true;
+			}
+
+			fprintf(script, " %s.%s\n",
+					PQgetvalue(res, rowno, i_nspname),
+					PQgetvalue(res, rowno, i_relname));
+		}
+
+		PQclear(res);
+
+		PQfinish(conn);
+	}
+
+	if (script)
+	{
+		fclose(script);
+		pg_log(PG_REPORT, "fatal");
+		pg_fatal("Your installation contains primary keys which use an index\n"
+				 "defined with NULLS NOT DISTINCT which cannot be restored.\n"
+				 "These constraints must be dropped and recreated before\n"
+				 "upgrading. A list of the problematic tables is in the file:\n"
+				 "    %s", output_path);
+	}
+	else
+		check_ok();
+}
 
 /*
  * get_canonical_locale_name
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 6cd57e3eaa..acfd9d1f4f 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1595,6 +1595,12 @@ create unique index on cwi_test (a);
 alter table cwi_test add primary key using index cwi_test_a_idx ;
 ERROR:  ALTER TABLE / ADD CONSTRAINT USING INDEX is not supported on partitioned tables
 DROP TABLE cwi_test;
+-- PRIMARY KEY constraint cannot be backed by a NULLS NOT DISTINCT index
+CREATE TABLE cwi_test(a int, b int);
+CREATE UNIQUE INDEX cwi_a_nnd ON cwi_test (a) NULLS NOT DISTINCT;
+ALTER TABLE cwi_test ADD PRIMARY KEY USING INDEX cwi_a_nnd;
+ERROR:  primary keys cannot use NULLS NOT DISTINCT indexes
+DROP TABLE cwi_test;
 --
 -- Check handling of indexes on system columns
 --
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index a3738833b2..d49ce9f300 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -617,6 +617,12 @@ create unique index on cwi_test (a);
 alter table cwi_test add primary key using index cwi_test_a_idx ;
 DROP TABLE cwi_test;
 
+-- PRIMARY KEY constraint cannot be backed by a NULLS NOT DISTINCT index
+CREATE TABLE cwi_test(a int, b int);
+CREATE UNIQUE INDEX cwi_a_nnd ON cwi_test (a) NULLS NOT DISTINCT;
+ALTER TABLE cwi_test ADD PRIMARY KEY USING INDEX cwi_a_nnd;
+DROP TABLE cwi_test;
+
 --
 -- Check handling of indexes on system columns
 --
-- 
2.32.1 (Apple Git-133)