]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix pg_dump for inherited validated not-null constraints
authorÁlvaro Herrera <alvherre@kurilemu.de>
Mon, 28 Apr 2025 14:25:06 +0000 (16:25 +0200)
committerÁlvaro Herrera <alvherre@kurilemu.de>
Mon, 28 Apr 2025 14:25:06 +0000 (16:25 +0200)
When a child constraint is validated and the parent constraint it
derives from isn't, pg_dump must be coerced into printing the child
constraint; failing to do would result in a dump that restores the
constraint as not valid, which would be incorrect.

Co-authored-by: jian he <jian.universality@gmail.com>
Co-authored-by: Álvaro Herrera <alvherre@kurilemu.de>
Reported-by: jian he <jian.universality@gmail.com>
Message-id: https://postgr.es/m/CACJufxGHNNMc0E2JphUqJMzD3=bwRSuAEVBF5ekgkG8uY0Q3hg@mail.gmail.com

src/bin/pg_dump/common.c
src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/pg_dump.h
src/bin/pg_dump/t/002_pg_dump.pl
src/test/regress/expected/constraints.out
src/test/regress/sql/constraints.sql

index 56b6c368acf89139f4dda363b2232ee6a2954b12..aa1589e3331d2dd3f976c36294b69b58b6554895 100644 (file)
@@ -453,8 +453,8 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
  * What we need to do here is:
  *
  * - Detect child columns that inherit NOT NULL bits from their parents, so
- *   that we needn't specify that again for the child. (Versions >= 18 no
- *   longer need this.)
+ *   that we needn't specify that again for the child.  For versions 18 and
+ *   up, this is needed when the parent is NOT VALID and the child isn't.
  *
  * - Detect child columns that have DEFAULT NULL when their parents had some
  *   non-null default.  In this case, we make up a dummy AttrDefInfo object so
@@ -516,6 +516,8 @@ flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables
                        bool            foundDefault;   /* Found a default in a parent */
                        bool            foundSameGenerated; /* Found matching GENERATED */
                        bool            foundDiffGenerated; /* Found non-matching GENERATED */
+                       bool            allNotNullsInvalid = true;      /* is NOT NULL NOT VALID
+                                                                                                        * on all parents? */
 
                        /* no point in examining dropped columns */
                        if (tbinfo->attisdropped[j])
@@ -546,6 +548,18 @@ flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables
                                                parent->notnull_constrs[inhAttrInd] != NULL)
                                                foundNotNull = true;
 
+                                       /*
+                                        * Keep track of whether all the parents that have a
+                                        * not-null constraint on this column have it as NOT
+                                        * VALID; if they all are, arrange to have it printed for
+                                        * this column.  If at least one parent has it as valid,
+                                        * there's no need.
+                                        */
+                                       if (fout->remoteVersion >= 180000 &&
+                                               parent->notnull_constrs[inhAttrInd] &&
+                                               !parent->notnull_invalid[inhAttrInd])
+                                               allNotNullsInvalid = false;
+
                                        foundDefault |= (parentDef != NULL &&
                                                                         strcmp(parentDef->adef_expr, "NULL") != 0 &&
                                                                         !parent->attgenerated[inhAttrInd]);
@@ -571,6 +585,14 @@ flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables
                        if (fout->remoteVersion < 180000 && foundNotNull)
                                tbinfo->notnull_islocal[j] = false;
 
+                       /*
+                        * For versions >18, we must print the not-null constraint locally
+                        * for this table even if it isn't really locally defined, but is
+                        * valid for the child and no parent has it as valid.
+                        */
+                       if (fout->remoteVersion >= 180000 && allNotNullsInvalid)
+                               tbinfo->notnull_islocal[j] = true;
+
                        /*
                         * Manufacture a DEFAULT NULL clause if necessary.  This breaks
                         * the advice given above to avoid changing state that might get
index 105e917aa7b9fc21e5ef91864a3b4534bb3d632d..e2e7975b34e025eeda3d481e0e261bd47bfd42a9 100644 (file)
@@ -9099,8 +9099,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
         *
         * We track in notnull_islocal whether the constraint was defined directly
         * in this table or via an ancestor, for binary upgrade.  flagInhAttrs
-        * might modify this later for servers older than 18; it's also in charge
-        * of determining the correct inhcount.
+        * might modify this later; that routine is also in charge of determining
+        * the correct inhcount.
         */
        if (fout->remoteVersion >= 180000)
                appendPQExpBufferStr(q,
@@ -9255,6 +9255,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
                tbinfo->attfdwoptions = (char **) pg_malloc(numatts * sizeof(char *));
                tbinfo->attmissingval = (char **) pg_malloc(numatts * sizeof(char *));
                tbinfo->notnull_constrs = (char **) pg_malloc(numatts * sizeof(char *));
+               tbinfo->notnull_invalid = (bool *) pg_malloc(numatts * sizeof(bool));
                tbinfo->notnull_noinh = (bool *) pg_malloc(numatts * sizeof(bool));
                tbinfo->notnull_islocal = (bool *) pg_malloc(numatts * sizeof(bool));
                tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *));
@@ -9708,8 +9709,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
  * constraints and the columns in the child don't have their own NOT NULL
  * declarations, we suppress printing constraints in the child: the
  * constraints are acquired at the point where the child is attached to the
- * parent.  This is tracked in ->notnull_islocal (which is set in flagInhAttrs
- * for servers pre-18).
+ * parent.  This is tracked in ->notnull_islocal; for servers pre-18 this is
+ * set not here but in flagInhAttrs.  That flag is also used when the
+ * constraint was validated in a child but all its parent have it as NOT
+ * VALID.
  *
  * Any of these constraints might have the NO INHERIT bit.  If so we set
  * ->notnull_noinh and NO INHERIT will be printed by dumpTableSchema.
@@ -9756,6 +9759,12 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
                else
                        appendPQExpBuffer(*invalidnotnulloids, ",%s", constroid);
 
+               /*
+                * Track when a parent constraint is invalid for the cases where a
+                * child constraint has been validated independenly.
+                */
+               tbinfo->notnull_invalid[j] = true;
+
                /* nothing else to do */
                tbinfo->notnull_constrs[j] = NULL;
                return;
@@ -9763,10 +9772,11 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
 
        /*
         * notnull_noinh is straight from the query result. notnull_islocal also,
-        * though flagInhAttrs may change that one later in versions < 18.
+        * though flagInhAttrs may change that one later.
         */
        tbinfo->notnull_noinh[j] = PQgetvalue(res, r, i_notnull_noinherit)[0] == 't';
        tbinfo->notnull_islocal[j] = PQgetvalue(res, r, i_notnull_islocal)[0] == 't';
+       tbinfo->notnull_invalid[j] = false;
 
        /*
         * Determine a constraint name to use.  If the column is not marked not-
index b426b5e47361d97ccbc0592bc332fb6ad5c10e7d..7417eab6aefa601fd7402d7e5077893da98d04f7 100644 (file)
@@ -365,6 +365,7 @@ typedef struct _tableInfo
                                                                         * there isn't one on this column. If
                                                                         * empty string, unnamed constraint
                                                                         * (pre-v17) */
+       bool       *notnull_invalid;    /* true for NOT NULL NOT VALID */
        bool       *notnull_noinh;      /* NOT NULL is NO INHERIT */
        bool       *notnull_islocal;    /* true if NOT NULL has local definition */
        struct _attrDefInfo **attrdefs; /* DEFAULT expressions */
index 6c03eca8e50121f4113648d515106bdbc43b461e..55d892d9c162287fab60042cb479156262d9255d 100644 (file)
@@ -1118,10 +1118,21 @@ my %tests = (
                },
        },
 
-       'CONSTRAINT NOT NULL / INVALID' => {
+       'CONSTRAINT NOT NULL / NOT VALID' => {
                create_sql => 'CREATE TABLE dump_test.test_table_nn (
                                                        col1 int);
-                       ALTER TABLE dump_test.test_table_nn ADD CONSTRAINT nn NOT NULL col1 NOT VALID;',
+                                                       CREATE TABLE dump_test.test_table_nn_2 (
+                                                       col1 int NOT NULL);
+                                                       CREATE TABLE dump_test.test_table_nn_chld1 (
+                                                       ) INHERITS (dump_test.test_table_nn);
+                                                       CREATE TABLE dump_test.test_table_nn_chld2 (
+                                                               col1 int
+                                                       ) INHERITS (dump_test.test_table_nn);
+                                                       CREATE TABLE dump_test.test_table_nn_chld3 (
+                                                       ) INHERITS (dump_test.test_table_nn, dump_test.test_table_nn_2);
+                       ALTER TABLE dump_test.test_table_nn ADD CONSTRAINT nn NOT NULL col1 NOT VALID;
+                       ALTER TABLE dump_test.test_table_nn_chld1 VALIDATE CONSTRAINT nn;
+                       ALTER TABLE dump_test.test_table_nn_chld2 VALIDATE CONSTRAINT nn;',
                regexp => qr/^
                        \QALTER TABLE dump_test.test_table_nn\E \n^\s+
                        \QADD CONSTRAINT nn NOT NULL col1 NOT VALID;\E
@@ -1135,6 +1146,50 @@ my %tests = (
                },
        },
 
+       'CONSTRAINT NOT NULL / NOT VALID (child1)' => {
+               regexp => qr/^
+               \QCREATE TABLE dump_test.test_table_nn_chld1 (\E\n
+               ^\s+\QCONSTRAINT nn NOT NULL col1\E$
+               /xm,
+               like => {
+                       %full_runs, %dump_test_schema_runs, section_pre_data => 1,
+               },
+               unlike => {
+                       exclude_dump_test_schema => 1,
+                       only_dump_measurement => 1,
+                       binary_upgrade => 1,
+               },
+       },
+
+       'CONSTRAINT NOT NULL / NOT VALID (child2)' => {
+               regexp => qr/^
+               \QCREATE TABLE dump_test.test_table_nn_chld2 (\E\n
+               ^\s+\Qcol1 integer CONSTRAINT nn NOT NULL\E$
+               /xm,
+               like => {
+                       %full_runs, %dump_test_schema_runs, section_pre_data => 1,
+               },
+               unlike => {
+                       exclude_dump_test_schema => 1,
+                       only_dump_measurement => 1,
+               },
+       },
+
+       'CONSTRAINT NOT NULL / NOT VALID (child3)' => {
+               regexp => qr/^
+               \QCREATE TABLE dump_test.test_table_nn_chld3 (\E\n
+               ^\Q)\E$
+               /xm,
+               like => {
+                       %full_runs, %dump_test_schema_runs, section_pre_data => 1,
+               },
+               unlike => {
+                       exclude_dump_test_schema => 1,
+                       only_dump_measurement => 1,
+                       binary_upgrade => 1,
+               },
+       },
+
        'CONSTRAINT PRIMARY KEY / WITHOUT OVERLAPS' => {
                create_sql => 'CREATE TABLE dump_test.test_table_tpk (
                                                        col1 int4range,
index eba4c0be0d86f05ebff8e0a8db7d2f66a3164a88..ad6aaab738538a1fec4c4d322a4688bdaa90f5ec 100644 (file)
@@ -1625,6 +1625,40 @@ EXECUTE get_nnconstraint_info('{notnull_part1_upg, notnull_part1_1_upg, notnull_
  notnull_part1_upg   | notnull_con | f            | t          |           0
 (4 rows)
 
+-- Inheritance test tables for pg_upgrade
+create table constr_parent (a int);
+create table constr_child (a int) inherits (constr_parent);
+NOTICE:  merging column "a" with inherited definition
+alter table constr_parent add not null a not valid;
+alter table constr_child validate constraint constr_parent_a_not_null;
+EXECUTE get_nnconstraint_info('{constr_parent, constr_child}');
+    tabname    |         conname          | convalidated | conislocal | coninhcount 
+---------------+--------------------------+--------------+------------+-------------
+ constr_child  | constr_parent_a_not_null | t            | f          |           1
+ constr_parent | constr_parent_a_not_null | f            | t          |           0
+(2 rows)
+
+create table constr_parent2 (a int);
+create table constr_child2 () inherits (constr_parent2);
+alter table constr_parent2 add not null a not valid;
+alter table constr_child2 validate constraint constr_parent2_a_not_null;
+EXECUTE get_nnconstraint_info('{constr_parent2, constr_child2}');
+    tabname     |          conname          | convalidated | conislocal | coninhcount 
+----------------+---------------------------+--------------+------------+-------------
+ constr_child2  | constr_parent2_a_not_null | t            | f          |           1
+ constr_parent2 | constr_parent2_a_not_null | f            | t          |           0
+(2 rows)
+
+create table constr_parent3 (a int not null);
+create table constr_child3 () inherits (constr_parent2, constr_parent3);
+NOTICE:  merging multiple inherited definitions of column "a"
+EXECUTE get_nnconstraint_info('{constr_parent3, constr_child3}');
+    tabname     |          conname          | convalidated | conislocal | coninhcount 
+----------------+---------------------------+--------------+------------+-------------
+ constr_child3  | constr_parent2_a_not_null | t            | f          |           2
+ constr_parent3 | constr_parent3_a_not_null | t            | t          |           0
+(2 rows)
+
 DEALLOCATE get_nnconstraint_info;
 -- end NOT NULL NOT VALID
 -- Comments
index 5d6d749c1502aa5dcad41e3e31233184bc291963..337baab7ced93723f060069a6090f885a59e820f 100644 (file)
@@ -979,6 +979,24 @@ INSERT INTO notnull_part1_3_upg values(NULL,1);
 ALTER TABLE notnull_part1_3_upg add CONSTRAINT nn3 NOT NULL a NOT VALID;
 ALTER TABLE notnull_part1_upg ATTACH PARTITION notnull_part1_3_upg FOR VALUES IN (NULL,5);
 EXECUTE get_nnconstraint_info('{notnull_part1_upg, notnull_part1_1_upg, notnull_part1_2_upg, notnull_part1_3_upg}');
+
+-- Inheritance test tables for pg_upgrade
+create table constr_parent (a int);
+create table constr_child (a int) inherits (constr_parent);
+alter table constr_parent add not null a not valid;
+alter table constr_child validate constraint constr_parent_a_not_null;
+EXECUTE get_nnconstraint_info('{constr_parent, constr_child}');
+
+create table constr_parent2 (a int);
+create table constr_child2 () inherits (constr_parent2);
+alter table constr_parent2 add not null a not valid;
+alter table constr_child2 validate constraint constr_parent2_a_not_null;
+EXECUTE get_nnconstraint_info('{constr_parent2, constr_child2}');
+
+create table constr_parent3 (a int not null);
+create table constr_child3 () inherits (constr_parent2, constr_parent3);
+EXECUTE get_nnconstraint_info('{constr_parent3, constr_child3}');
+
 DEALLOCATE get_nnconstraint_info;
 
 -- end NOT NULL NOT VALID