From 6baea59b788b915c23a9c934cad6c0db5b1ff62e Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson 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 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 () NULLS NOT DISTINCT; + * ALTER TABLE 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)