Skip to content

Commit 0e13b13

Browse files
Álvaro Herrerajianhe-fun
Álvaro Herrera
and
jianhe-fun
committed
Fix pg_dump for inherited validated not-null constraints
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
1 parent c061000 commit 0e13b13

File tree

6 files changed

+149
-9
lines changed

6 files changed

+149
-9
lines changed

‎src/bin/pg_dump/common.c

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,8 +453,8 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
453453
* What we need to do here is:
454454
*
455455
* - Detect child columns that inherit NOT NULL bits from their parents, so
456-
* that we needn't specify that again for the child. (Versions >= 18 no
457-
* longer need this.)
456+
* that we needn't specify that again for the child. For versions 18 and
457+
* up, this is needed when the parent is NOT VALID and the child isn't.
458458
*
459459
* - Detect child columns that have DEFAULT NULL when their parents had some
460460
* 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
516516
bool foundDefault; /* Found a default in a parent */
517517
bool foundSameGenerated; /* Found matching GENERATED */
518518
bool foundDiffGenerated; /* Found non-matching GENERATED */
519+
bool allNotNullsInvalid = true; /* is NOT NULL NOT VALID
520+
* on all parents? */
519521

520522
/* no point in examining dropped columns */
521523
if (tbinfo->attisdropped[j])
@@ -546,6 +548,18 @@ flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables
546548
parent->notnull_constrs[inhAttrInd] != NULL)
547549
foundNotNull = true;
548550

551+
/*
552+
* Keep track of whether all the parents that have a
553+
* not-null constraint on this column have it as NOT
554+
* VALID; if they all are, arrange to have it printed for
555+
* this column. If at least one parent has it as valid,
556+
* there's no need.
557+
*/
558+
if (fout->remoteVersion >= 180000 &&
559+
parent->notnull_constrs[inhAttrInd] &&
560+
!parent->notnull_invalid[inhAttrInd])
561+
allNotNullsInvalid = false;
562+
549563
foundDefault |= (parentDef != NULL &&
550564
strcmp(parentDef->adef_expr, "NULL") != 0 &&
551565
!parent->attgenerated[inhAttrInd]);
@@ -571,6 +585,14 @@ flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables
571585
if (fout->remoteVersion < 180000 && foundNotNull)
572586
tbinfo->notnull_islocal[j] = false;
573587

588+
/*
589+
* For versions >18, we must print the not-null constraint locally
590+
* for this table even if it isn't really locally defined, but is
591+
* valid for the child and no parent has it as valid.
592+
*/
593+
if (fout->remoteVersion >= 180000 && allNotNullsInvalid)
594+
tbinfo->notnull_islocal[j] = true;
595+
574596
/*
575597
* Manufacture a DEFAULT NULL clause if necessary. This breaks
576598
* the advice given above to avoid changing state that might get

‎src/bin/pg_dump/pg_dump.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9099,8 +9099,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
90999099
*
91009100
* We track in notnull_islocal whether the constraint was defined directly
91019101
* in this table or via an ancestor, for binary upgrade. flagInhAttrs
9102-
* might modify this later for servers older than 18; it's also in charge
9103-
* of determining the correct inhcount.
9102+
* might modify this later; that routine is also in charge of determining
9103+
* the correct inhcount.
91049104
*/
91059105
if (fout->remoteVersion >= 180000)
91069106
appendPQExpBufferStr(q,
@@ -9255,6 +9255,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
92559255
tbinfo->attfdwoptions = (char **) pg_malloc(numatts * sizeof(char *));
92569256
tbinfo->attmissingval = (char **) pg_malloc(numatts * sizeof(char *));
92579257
tbinfo->notnull_constrs = (char **) pg_malloc(numatts * sizeof(char *));
9258+
tbinfo->notnull_invalid = (bool *) pg_malloc(numatts * sizeof(bool));
92589259
tbinfo->notnull_noinh = (bool *) pg_malloc(numatts * sizeof(bool));
92599260
tbinfo->notnull_islocal = (bool *) pg_malloc(numatts * sizeof(bool));
92609261
tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *));
@@ -9708,8 +9709,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
97089709
* constraints and the columns in the child don't have their own NOT NULL
97099710
* declarations, we suppress printing constraints in the child: the
97109711
* constraints are acquired at the point where the child is attached to the
9711-
* parent. This is tracked in ->notnull_islocal (which is set in flagInhAttrs
9712-
* for servers pre-18).
9712+
* parent. This is tracked in ->notnull_islocal; for servers pre-18 this is
9713+
* set not here but in flagInhAttrs. That flag is also used when the
9714+
* constraint was validated in a child but all its parent have it as NOT
9715+
* VALID.
97139716
*
97149717
* Any of these constraints might have the NO INHERIT bit. If so we set
97159718
* ->notnull_noinh and NO INHERIT will be printed by dumpTableSchema.
@@ -9756,17 +9759,24 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
97569759
else
97579760
appendPQExpBuffer(*invalidnotnulloids, ",%s", constroid);
97589761

9762+
/*
9763+
* Track when a parent constraint is invalid for the cases where a
9764+
* child constraint has been validated independenly.
9765+
*/
9766+
tbinfo->notnull_invalid[j] = true;
9767+
97599768
/* nothing else to do */
97609769
tbinfo->notnull_constrs[j] = NULL;
97619770
return;
97629771
}
97639772

97649773
/*
97659774
* notnull_noinh is straight from the query result. notnull_islocal also,
9766-
* though flagInhAttrs may change that one later in versions < 18.
9775+
* though flagInhAttrs may change that one later.
97679776
*/
97689777
tbinfo->notnull_noinh[j] = PQgetvalue(res, r, i_notnull_noinherit)[0] == 't';
97699778
tbinfo->notnull_islocal[j] = PQgetvalue(res, r, i_notnull_islocal)[0] == 't';
9779+
tbinfo->notnull_invalid[j] = false;
97709780

97719781
/*
97729782
* Determine a constraint name to use. If the column is not marked not-

‎src/bin/pg_dump/pg_dump.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ typedef struct _tableInfo
365365
* there isn't one on this column. If
366366
* empty string, unnamed constraint
367367
* (pre-v17) */
368+
bool *notnull_invalid; /* true for NOT NULL NOT VALID */
368369
bool *notnull_noinh; /* NOT NULL is NO INHERIT */
369370
bool *notnull_islocal; /* true if NOT NULL has local definition */
370371
struct _attrDefInfo **attrdefs; /* DEFAULT expressions */

‎src/bin/pg_dump/t/002_pg_dump.pl

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,10 +1118,21 @@
11181118
},
11191119
},
11201120

1121-
'CONSTRAINT NOT NULL / INVALID' => {
1121+
'CONSTRAINT NOT NULL / NOT VALID' => {
11221122
create_sql => 'CREATE TABLE dump_test.test_table_nn (
11231123
col1 int);
1124-
ALTER TABLE dump_test.test_table_nn ADD CONSTRAINT nn NOT NULL col1 NOT VALID;',
1124+
CREATE TABLE dump_test.test_table_nn_2 (
1125+
col1 int NOT NULL);
1126+
CREATE TABLE dump_test.test_table_nn_chld1 (
1127+
) INHERITS (dump_test.test_table_nn);
1128+
CREATE TABLE dump_test.test_table_nn_chld2 (
1129+
col1 int
1130+
) INHERITS (dump_test.test_table_nn);
1131+
CREATE TABLE dump_test.test_table_nn_chld3 (
1132+
) INHERITS (dump_test.test_table_nn, dump_test.test_table_nn_2);
1133+
ALTER TABLE dump_test.test_table_nn ADD CONSTRAINT nn NOT NULL col1 NOT VALID;
1134+
ALTER TABLE dump_test.test_table_nn_chld1 VALIDATE CONSTRAINT nn;
1135+
ALTER TABLE dump_test.test_table_nn_chld2 VALIDATE CONSTRAINT nn;',
11251136
regexp => qr/^
11261137
\QALTER TABLE dump_test.test_table_nn\E \n^\s+
11271138
\QADD CONSTRAINT nn NOT NULL col1 NOT VALID;\E
@@ -1135,6 +1146,50 @@
11351146
},
11361147
},
11371148

1149+
'CONSTRAINT NOT NULL / NOT VALID (child1)' => {
1150+
regexp => qr/^
1151+
\QCREATE TABLE dump_test.test_table_nn_chld1 (\E\n
1152+
^\s+\QCONSTRAINT nn NOT NULL col1\E$
1153+
/xm,
1154+
like => {
1155+
%full_runs, %dump_test_schema_runs, section_pre_data => 1,
1156+
},
1157+
unlike => {
1158+
exclude_dump_test_schema => 1,
1159+
only_dump_measurement => 1,
1160+
binary_upgrade => 1,
1161+
},
1162+
},
1163+
1164+
'CONSTRAINT NOT NULL / NOT VALID (child2)' => {
1165+
regexp => qr/^
1166+
\QCREATE TABLE dump_test.test_table_nn_chld2 (\E\n
1167+
^\s+\Qcol1 integer CONSTRAINT nn NOT NULL\E$
1168+
/xm,
1169+
like => {
1170+
%full_runs, %dump_test_schema_runs, section_pre_data => 1,
1171+
},
1172+
unlike => {
1173+
exclude_dump_test_schema => 1,
1174+
only_dump_measurement => 1,
1175+
},
1176+
},
1177+
1178+
'CONSTRAINT NOT NULL / NOT VALID (child3)' => {
1179+
regexp => qr/^
1180+
\QCREATE TABLE dump_test.test_table_nn_chld3 (\E\n
1181+
^\Q)\E$
1182+
/xm,
1183+
like => {
1184+
%full_runs, %dump_test_schema_runs, section_pre_data => 1,
1185+
},
1186+
unlike => {
1187+
exclude_dump_test_schema => 1,
1188+
only_dump_measurement => 1,
1189+
binary_upgrade => 1,
1190+
},
1191+
},
1192+
11381193
'CONSTRAINT PRIMARY KEY / WITHOUT OVERLAPS' => {
11391194
create_sql => 'CREATE TABLE dump_test.test_table_tpk (
11401195
col1 int4range,

‎src/test/regress/expected/constraints.out

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,6 +1625,40 @@ EXECUTE get_nnconstraint_info('{notnull_part1_upg, notnull_part1_1_upg, notnull_
16251625
notnull_part1_upg | notnull_con | f | t | 0
16261626
(4 rows)
16271627

1628+
-- Inheritance test tables for pg_upgrade
1629+
create table constr_parent (a int);
1630+
create table constr_child (a int) inherits (constr_parent);
1631+
NOTICE: merging column "a" with inherited definition
1632+
alter table constr_parent add not null a not valid;
1633+
alter table constr_child validate constraint constr_parent_a_not_null;
1634+
EXECUTE get_nnconstraint_info('{constr_parent, constr_child}');
1635+
tabname | conname | convalidated | conislocal | coninhcount
1636+
---------------+--------------------------+--------------+------------+-------------
1637+
constr_child | constr_parent_a_not_null | t | f | 1
1638+
constr_parent | constr_parent_a_not_null | f | t | 0
1639+
(2 rows)
1640+
1641+
create table constr_parent2 (a int);
1642+
create table constr_child2 () inherits (constr_parent2);
1643+
alter table constr_parent2 add not null a not valid;
1644+
alter table constr_child2 validate constraint constr_parent2_a_not_null;
1645+
EXECUTE get_nnconstraint_info('{constr_parent2, constr_child2}');
1646+
tabname | conname | convalidated | conislocal | coninhcount
1647+
----------------+---------------------------+--------------+------------+-------------
1648+
constr_child2 | constr_parent2_a_not_null | t | f | 1
1649+
constr_parent2 | constr_parent2_a_not_null | f | t | 0
1650+
(2 rows)
1651+
1652+
create table constr_parent3 (a int not null);
1653+
create table constr_child3 () inherits (constr_parent2, constr_parent3);
1654+
NOTICE: merging multiple inherited definitions of column "a"
1655+
EXECUTE get_nnconstraint_info('{constr_parent3, constr_child3}');
1656+
tabname | conname | convalidated | conislocal | coninhcount
1657+
----------------+---------------------------+--------------+------------+-------------
1658+
constr_child3 | constr_parent2_a_not_null | t | f | 2
1659+
constr_parent3 | constr_parent3_a_not_null | t | t | 0
1660+
(2 rows)
1661+
16281662
DEALLOCATE get_nnconstraint_info;
16291663
-- end NOT NULL NOT VALID
16301664
-- Comments

‎src/test/regress/sql/constraints.sql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,24 @@ INSERT INTO notnull_part1_3_upg values(NULL,1);
979979
ALTER TABLE notnull_part1_3_upg add CONSTRAINT nn3 NOT NULL a NOT VALID;
980980
ALTER TABLE notnull_part1_upg ATTACH PARTITION notnull_part1_3_upg FOR VALUES IN (NULL,5);
981981
EXECUTE get_nnconstraint_info('{notnull_part1_upg, notnull_part1_1_upg, notnull_part1_2_upg, notnull_part1_3_upg}');
982+
983+
-- Inheritance test tables for pg_upgrade
984+
create table constr_parent (a int);
985+
create table constr_child (a int) inherits (constr_parent);
986+
alter table constr_parent add not null a not valid;
987+
alter table constr_child validate constraint constr_parent_a_not_null;
988+
EXECUTE get_nnconstraint_info('{constr_parent, constr_child}');
989+
990+
create table constr_parent2 (a int);
991+
create table constr_child2 () inherits (constr_parent2);
992+
alter table constr_parent2 add not null a not valid;
993+
alter table constr_child2 validate constraint constr_parent2_a_not_null;
994+
EXECUTE get_nnconstraint_info('{constr_parent2, constr_child2}');
995+
996+
create table constr_parent3 (a int not null);
997+
create table constr_child3 () inherits (constr_parent2, constr_parent3);
998+
EXECUTE get_nnconstraint_info('{constr_parent3, constr_child3}');
999+
9821000
DEALLOCATE get_nnconstraint_info;
9831001

9841002
-- end NOT NULL NOT VALID

0 commit comments

Comments
 (0)