From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Ali Akbar <the.apaan@gmail.com>
Cc: Michael Paquier <michael.paquier@gmail.com>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: tushar <tushar.ahuja@enterprisedb.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org, Alvaro Herrera <alvherre@alvh.no-ip.org>
Subject: Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
Date: Thu, 18 Apr 2024 15:01:52 -0500
Message-ID: <ZiF8MCeZOAbC6710@pryzbyj2023> (raw)
In-Reply-To: <ZRb9GugoMk74CAn+@pryzbyj2023>
References: <CAB7nPqStji75D1M=DEkpOXsJbF151m=ZLrJ4jXTh-Czk3kKZ1Q@mail.gmail.com>
<CACQjQLrN88BrpUAFv-soW+bCnDEkkWGc+z6cmSEMfGMpd+Aa=g@mail.gmail.com>
<107ccc78-1a10-ff59-e837-e5c99dc8c152@lab.ntt.co.jp>
<CACQjQLoMsE+1pyLe98pi0KvPG2jQQ94LWJ+PTiLgVRK4B=i_jg@mail.gmail.com>
<20171214054902.GC23527@telsasoft.com>
<CAB7nPqRgRbMWLaidxKW+n144kT+3xRfTSqNS8OUaV77FuQo48w@mail.gmail.com>
<CACQjQLqoXTzCV4+-s1XOT62tY8ggkZkH4kY03gm2aQYOMXE+SA@mail.gmail.com>
<ZRXGAaOvamcRjoL+@telsasoft.com>
<ZRYXY3WMVep8yEgq@paquier.xyz>
<ZRb9GugoMk74CAn+@pryzbyj2023>
On Thu, Dec 14, 2017 at 08:51:06AM +0700, Ali Akbar wrote:
> Patch for adding check in pg_upgrade.
[...]
On Fri, Sep 29, 2023 at 11:36:42AM -0500, Justin Pryzby wrote:
> On Fri, Sep 29, 2023 at 09:16:35AM +0900, Michael Paquier wrote:
> > You mean when upgrading from an instance of 9.6 or older as c30f177 is
> > not there, right?
>
> No - while upgrading from v15 to v16. I'm not clear on how we upgraded
> *to* v15 without hitting the issue, nor how the "not null" got
> dropped...
>
> > Anyway, it seems like the patch from [1] has no need to run this check
> > when the old cluster's version is 10 or newer. And perhaps it should
> > mention that this check could be removed from pg_upgrade once v10
> > support is out of scope, in the shape of a comment.
>
> You're still thinking of PRIMARY KEY as the only way to hit this, right?
> But Ali Akbar already pointed out how to reproduce the problem with DROP
> NOT NULL - which still applies to both v16 and v17.
Rebased and amended the forgotten patch.
See also ZiE3NoY6DdvlvFl9@pryzbyj2023 et seq.
Attachments:0001-pg_upgrade-check-for-inconsistent-inherited-not-null.patch (text/x-diff, 4.9K) download
From f804bec39acac0b53e29563be9ef7a10237ba717 Mon Sep 17 00:00:00 2001
From: Ali Akbar <the.apaan@gmail.com>
Date: Thu, 14 Dec 2017 19:18:59 +0700
Subject: [PATCH] pg_upgrade: check for inconsistent inherited not null columns
Otherwise, the upgrade can fail halfway through with error:
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
JTP: updated to handle contype='n' - otherwise, the cnn_grandchild2
added at b0e96f311 triggers the issue this patch tries to avoid ...
Should be backpatched to all versions.
---
src/bin/pg_upgrade/check.c | 101 +++++++++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.cindex ad1f8e3bcb1..79671a9cdb4 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(bool live_check);
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
@@ -646,6 +647,8 @@ check_and_dump_old_cluster(bool live_check)
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
@@ -1832,6 +1835,104 @@ check_new_cluster_subscription_configuration(void)
check_ok();
}
+/*+ * check_for_not_null_inheritance()+ *+ * Currently pg_upgrade will fail if there are any inconsistencies in NOT NULL+ * constraint inheritance: In PostgreSQL, we can have a column that is NOT NULL+ * in parent, but nullable in its children. So check for inconsistent NOT NULL+ * constraints in inheritance tree.+ */+static void+check_for_not_null_inheritance(ClusterInfo *cluster)+{+ int dbnum;+ FILE *script = NULL;+ char output_path[MAXPGPATH];++ prep_status("Checking for NOT NULL inconsistencies in inheritance");++ snprintf(output_path, sizeof(output_path), "not_null_inconsistent_columns.txt");++ for (dbnum = 0; dbnum < cluster->dbarr.ndbs; dbnum++)+ {+ PGresult *res;+ bool db_used = false;+ int ntups;+ int rowno;+ int i_nspname,+ i_relname,+ i_attname;+ DbInfo *active_db = &cluster->dbarr.dbs[dbnum];+ PGconn *conn = connectToServer(cluster, active_db->db_name);++ res = executeQueryOrDie(conn,+ "WITH RECURSIVE parents AS ( "+ " SELECT i.inhrelid, i.inhparent "+ " FROM pg_catalog.pg_inherits i "+ " UNION ALL "+ " SELECT p.inhrelid, i.inhparent "+ " FROM parents p "+ " JOIN pg_catalog.pg_inherits i "+ " ON i.inhrelid = p.inhparent "+ ") "+ "SELECT n.nspname, c.relname, ac.attname "+ "FROM parents p, "+ " pg_catalog.pg_attribute ac, "+ " pg_catalog.pg_attribute ap, "+ " pg_catalog.pg_constraint cc, "+ " pg_catalog.pg_class c, "+ " pg_catalog.pg_namespace n "+ "WHERE NOT ac.attnotnull AND "+ " ac.attrelid = p.inhrelid AND "+ " ap.attrelid = p.inhparent AND "+ " ac.attname = ap.attname AND "+ " ac.attrelid = cc.conrelid AND ac.attnum = ANY(cc.conkey) AND contype='n' AND NOT connoinherit AND"+ " ap.attnotnull AND "+ " c.oid = ac.attrelid AND "+ " c.relnamespace = n.oid");++ ntups = PQntuples(res);+ i_nspname = PQfnumber(res, "nspname");+ i_relname = PQfnumber(res, "relname");+ i_attname = PQfnumber(res, "attname");+ for (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, "Database: %s\n", active_db->db_name);+ db_used = true;+ }+ fprintf(script, " %s.%s.%s\n",+ PQgetvalue(res, rowno, i_nspname),+ PQgetvalue(res, rowno, i_relname),+ PQgetvalue(res, rowno, i_attname));+ }++ PQclear(res);++ PQfinish(conn);+ }++ if (script)+ {+ fclose(script);+ pg_log(PG_REPORT, "fatal");+ pg_fatal("Your installation contains has inconsistencies in NOT NULL\n"+ "constraint inheritance: child column is not marked NOT NULL\n"+ "while parent column has NOT NULL constraint. You can fix the\n"+ "inconsistency by adding NOT NULL constraints and rerunning the\n"+ "upgrade.\n"+ "A list of the problematic columns is in the file:\n"+ " %s", output_path);+ }+ else+ check_ok();+}+
/*
* check_old_cluster_for_valid_slots()
*
--
2.42.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, michael@paquier.xyz, the.apaan@gmail.com, michael.paquier@gmail.com, Langote_Amit_f8@lab.ntt.co.jp, tushar.ahuja@enterprisedb.com, tgl@sss.pgh.pa.us, alvherre@alvh.no-ip.org
Subject: Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
In-Reply-To: <ZiF8MCeZOAbC6710@pryzbyj2023>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox