Hello, We seem to have forgotten about this issue. I think this is even more pressing with the changes to 18 for not-null constraints, though strictly speaking it's always been a problem. Here's an updated patch. I simplified the query (no need for a recursive CTE as far as I can tell) and updated to use the pg_upgrade "task" infrastructure.
I think from 18 on, the problem can no longer be recreated, since removing the attnotnull bit from a child table is not possible. I should add some testing too before pushing, of course. I only tested manually. I'd like to hear from RMT about getting this in 18. Actually I think we should consider backporting to all live versions ... thoughts? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Investigación es lo que hago cuando no sé lo que estoy haciendo" (Wernher von Braun)
>From 91142cc750b041bb8a0ed28004df3b609d70bd0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@kurilemu.de> Date: Thu, 3 Jul 2025 20:14:27 +0200 Subject: [PATCH] pg_upgrade: check for inconsistent inherited not null columns Otherwise, the upgrade can fail halfway through with errors with tables like these: CREATE TABLE ip (id int PRIMARY KEY); CREATE TABLE ic(id int) INHERITS (ip); ALTER TABLE ic ALTER id DROP NOT NULL; ERROR: column "a" in child table must be marked NOT NULL Author: Ali Akbar <the.ap...@gmail.com> Reviewed-by: Justin Pryzby <pry...@telsasoft.com> Discussion: https://postgr.es/m/CACQjQLoMsE+1pyLe98pi0KvPG2jQQ94LWJ+PTiLgVRK4B=i...@mail.gmail.com --- src/bin/pg_upgrade/check.c | 91 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index fb063a2de42..7f23175d662 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -31,6 +31,7 @@ static void check_new_cluster_logical_replication_slots(void); static void check_new_cluster_subscription_configuration(void); static void check_old_cluster_for_valid_slots(void); static void check_old_cluster_subscription_state(void); +static void check_for_not_null_inheritance(ClusterInfo *cluster); /* * DataTypesUsageChecks - definitions of data type checks for the old cluster @@ -672,6 +673,8 @@ check_and_dump_old_cluster(void) if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1100) check_for_tables_with_oids(&old_cluster); + check_for_not_null_inheritance(&old_cluster); + /* * Pre-PG 10 allowed tables with 'unknown' type columns and non WAL logged * hash indexes @@ -1943,6 +1946,94 @@ check_for_unicode_update(ClusterInfo *cluster) check_ok(); } +/* + * Callback function for processing results of query for + * check_for_not_null_inheritance. + */ +static void +process_inconsistent_notnull(DbInfo *dbinfo, PGresult *res, void *arg) +{ + UpgradeTaskReport *report = (UpgradeTaskReport *) arg; + int ntups = PQntuples(res); + int i_nspname = PQfnumber(res, "nspname"); + int i_relname = PQfnumber(res, "relname"); + int i_attname = PQfnumber(res, "attname"); + + AssertVariableIsOfType(&process_inconsistent_notnull, + UpgradeTaskProcessCB); + + if (ntups == 0) + return; + + if (report->file == NULL && + (report->file = fopen_priv(report->path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %m", report->path); + + fprintf(report->file, "In database: %s\n", dbinfo->db_name); + + for (int rowno = 0; rowno < ntups; rowno++) + { + fprintf(report->file, " %s.%s.%s\n", + PQgetvalue(res, rowno, i_nspname), + PQgetvalue(res, rowno, i_relname), + PQgetvalue(res, rowno, i_attname)); + } +} + +/* + * check_for_not_null_inheritance() + * + * An attempt to create child tables lacking not-null constraints that are + * present in their parents errors out. This can no longer occur since 18, + * but previously there were various ways for that to happen. Check that + * the cluster to be upgraded doesn't have any of those problems. + */ +static void +check_for_not_null_inheritance(ClusterInfo *cluster) +{ + UpgradeTaskReport report; + UpgradeTask *task; + const char *query; + + prep_status("Checking for not-null constraint inconsistencies"); + + report.file = NULL; + snprintf(report.path, sizeof(report.path), "%s/%s", + log_opts.basedir, + "not_null_inconsistent_columns.txt"); + + query = "SELECT cc.relnamespace::pg_catalog.regnamespace AS nspname, " + " cc.relname, ac.attname " + "FROM pg_catalog.pg_inherits i, pg_catalog.pg_attribute ac, " + " pg_catalog.pg_attribute ap, pg_catalog.pg_class cc " + "WHERE cc.oid = ac.attrelid AND i.inhrelid = ac.attrelid " + " AND i.inhparent = ap.attrelid AND ac.attname = ap.attname " + " AND ap.attnum > 0 and ap.attnotnull AND NOT ac.attnotnull"; + + task = upgrade_task_create(); + upgrade_task_add_step(task, query, + process_inconsistent_notnull, + true, &report); + upgrade_task_run(task, cluster); + upgrade_task_free(task); + + if (report.file) + { + fclose(report.file); + pg_log(PG_REPORT, "fatal"); + pg_fatal("Your installation contains inconsistent NOT NULL constraints.\n" + "If the parent column(s) are NOT NULL, then the child column must\n" + "also be marked NOT NULL, or the upgrade will fail.\n" + "You can fix this by running\n" + " ALTER TABLE tablename ALTER column SET NOT NULL;\n" + "on each column listed in the file:\n" + " %s", report.path); + } + else + check_ok(); +} + + /* * check_new_cluster_logical_replication_slots() * -- 2.39.5
