diff options
| author | Alvaro Herrera <alvherre@alvh.no-ip.org> | 2023-04-12 19:29:21 +0200 |
|---|---|---|
| committer | Alvaro Herrera <alvherre@alvh.no-ip.org> | 2023-04-12 19:29:21 +0200 |
| commit | 9ce04b50e120275afbc03ca0b80839dde3da8308 (patch) | |
| tree | c9b815953349fc5bf689799108a2966fec755bf7 /src/bin | |
| parent | 8e82db97b0a474008d8212a63f34e449a8c50bcd (diff) | |
| download | postgresql-9ce04b50e120275afbc03ca0b80839dde3da8308.tar.gz | |
Revert "Catalog NOT NULL constraints" and fallout
This reverts commit e056c557aef4 and minor later fixes thereof.
There's a few problems in this new feature -- most notably regarding
pg_upgrade behavior, but others as well. This new feature is not in any
way critical on its own, so instead of scrambling to fix it we revert it
and try again in early 17 with these issues in mind.
Discussion: https://postgr.es/m/3801207.1681057430@sss.pgh.pa.us
Diffstat (limited to 'src/bin')
| -rw-r--r-- | src/bin/pg_dump/common.c | 15 | ||||
| -rw-r--r-- | src/bin/pg_dump/pg_backup_archiver.c | 2 | ||||
| -rw-r--r-- | src/bin/pg_dump/pg_dump.c | 209 | ||||
| -rw-r--r-- | src/bin/pg_dump/pg_dump.h | 2 | ||||
| -rw-r--r-- | src/bin/pg_dump/t/002_pg_dump.pl | 6 |
5 files changed, 37 insertions, 197 deletions
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index ed95dc8dc6..5d988986ed 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -82,8 +82,7 @@ static catalogid_hash *catalogIdHash = NULL; static void flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables, InhInfo *inhinfo, int numInherits); static void flagInhIndexes(Archive *fout, TableInfo *tblinfo, int numTables); -static void flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, - int numTables); +static void flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables); static int strInArray(const char *pattern, char **arr, int arr_size); static IndxInfo *findIndexByOid(Oid oid); @@ -227,7 +226,7 @@ getSchemaData(Archive *fout, int *numTablesPtr) getTableAttrs(fout, tblinfo, numTables); pg_log_info("flagging inherited columns in subtables"); - flagInhAttrs(fout, fout->dopt, tblinfo, numTables); + flagInhAttrs(fout->dopt, tblinfo, numTables); pg_log_info("reading partitioning data"); getPartitioningInfo(fout); @@ -472,8 +471,7 @@ 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 >= 16 no - * longer need this.) + * that we needn't specify that again for the child. * * - 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 @@ -493,7 +491,7 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables) * modifies tblinfo */ static void -flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables) +flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables) { int i, j, @@ -574,9 +572,8 @@ flagInhAttrs(Archive *fout, DumpOptions *dopt, TableInfo *tblinfo, int numTables } } - /* In versions < 16, remember if we found inherited NOT NULL */ - if (fout->remoteVersion < 160000) - tbinfo->localNotNull[j] = !foundNotNull; + /* Remember if we found inherited NOT NULL */ + tbinfo->inhNotNull[j] = foundNotNull; /* * Manufacture a DEFAULT NULL clause if necessary. This breaks diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 5b858b2348..d518349e10 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -601,7 +601,6 @@ RestoreArchive(Archive *AHX) if (strcmp(te->desc, "CONSTRAINT") == 0 || strcmp(te->desc, "CHECK CONSTRAINT") == 0 || - strcmp(te->desc, "NOT NULL CONSTRAINT") == 0 || strcmp(te->desc, "FK CONSTRAINT") == 0) strcpy(buffer, "DROP CONSTRAINT"); else @@ -3512,7 +3511,6 @@ _getObjectDescription(PQExpBuffer buf, const TocEntry *te) /* these object types don't have separate owners */ else if (strcmp(type, "CAST") == 0 || strcmp(type, "CHECK CONSTRAINT") == 0 || - strcmp(type, "NOT NULL CONSTRAINT") == 0 || strcmp(type, "CONSTRAINT") == 0 || strcmp(type, "DATABASE PROPERTIES") == 0 || strcmp(type, "DEFAULT") == 0 || diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 967ced4eed..7a504dfe25 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -8372,7 +8372,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) PQExpBuffer q = createPQExpBuffer(); PQExpBuffer tbloids = createPQExpBuffer(); PQExpBuffer checkoids = createPQExpBuffer(); - PQExpBuffer defaultoids = createPQExpBuffer(); PGresult *res; int ntups; int curtblindx; @@ -8390,7 +8389,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) int i_attalign; int i_attislocal; int i_attnotnull; - int i_localnotnull; int i_attoptions; int i_attcollation; int i_attcompression; @@ -8400,17 +8398,16 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) /* * We want to perform just one query against pg_attribute, and then just - * one against pg_attrdef (for DEFAULTs) and two against pg_constraint - * (for CHECK constraints and for NOT NULL constraints). However, we - * mustn't try to select every row of those catalogs and then sort it out - * on the client side, because some of the server-side functions we need - * would be unsafe to apply to tables we don't have lock on. Hence, we - * build an array of the OIDs of tables we care about (and now have lock - * on!), and use a WHERE clause to constrain which rows are selected. + * one against pg_attrdef (for DEFAULTs) and one against pg_constraint + * (for CHECK constraints). However, we mustn't try to select every row + * of those catalogs and then sort it out on the client side, because some + * of the server-side functions we need would be unsafe to apply to tables + * we don't have lock on. Hence, we build an array of the OIDs of tables + * we care about (and now have lock on!), and use a WHERE clause to + * constrain which rows are selected. */ appendPQExpBufferChar(tbloids, '{'); appendPQExpBufferChar(checkoids, '{'); - appendPQExpBufferChar(defaultoids, '{'); for (int i = 0; i < numTables; i++) { TableInfo *tbinfo = &tblinfo[i]; @@ -8454,6 +8451,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) "a.attstattarget,\n" "a.attstorage,\n" "t.typstorage,\n" + "a.attnotnull,\n" "a.atthasdef,\n" "a.attisdropped,\n" "a.attlen,\n" @@ -8470,21 +8468,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) "ORDER BY option_name" "), E',\n ') AS attfdwoptions,\n"); - /* - * Write out NOT NULL. In 16 and up we have to read pg_constraint, and we - * only print it for constraints that aren't connoinherit. A NULL result - * means there's no contype='n' row for the column, so we mustn't print - * anything then either. We also track conislocal so that we can handle - * the case of partitioned tables and binary upgrade especially. - */ - if (fout->remoteVersion >= 160000) - appendPQExpBufferStr(q, - "co.connoinherit IS NOT NULL AS attnotnull,\n" - "coalesce(co.conislocal, false) AS local_notnull,\n"); - else - appendPQExpBufferStr(q, - "a.attnotnull, false AS local_notnull,\n"); - if (fout->remoteVersion >= 140000) appendPQExpBufferStr(q, "a.attcompression AS attcompression,\n"); @@ -8519,20 +8502,11 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_attribute a ON (src.tbloid = a.attrelid) " "LEFT JOIN pg_catalog.pg_type t " - "ON (a.atttypid = t.oid)\n", + "ON (a.atttypid = t.oid)\n" + "WHERE a.attnum > 0::pg_catalog.int2\n" + "ORDER BY a.attrelid, a.attnum", tbloids->data); - /* in 16, need pg_constraint for NOT NULLs */ - if (fout->remoteVersion >= 160000) - appendPQExpBufferStr(q, - " LEFT JOIN pg_catalog.pg_constraint co ON " - "(a.attrelid = co.conrelid\n" - " AND co.contype = 'n' AND " - "co.conkey = array[a.attnum])\n"); - appendPQExpBufferStr(q, - "WHERE a.attnum > 0::pg_catalog.int2\n" - "ORDER BY a.attrelid, a.attnum"); - res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK); ntups = PQntuples(res); @@ -8551,7 +8525,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) i_attalign = PQfnumber(res, "attalign"); i_attislocal = PQfnumber(res, "attislocal"); i_attnotnull = PQfnumber(res, "attnotnull"); - i_localnotnull = PQfnumber(res, "local_notnull"); i_attoptions = PQfnumber(res, "attoptions"); i_attcollation = PQfnumber(res, "attcollation"); i_attcompression = PQfnumber(res, "attcompression"); @@ -8560,8 +8533,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) i_atthasdef = PQfnumber(res, "atthasdef"); /* Within the next loop, we'll accumulate OIDs of tables with defaults */ - resetPQExpBuffer(defaultoids); - appendPQExpBufferChar(defaultoids, '{'); + resetPQExpBuffer(tbloids); + appendPQExpBufferChar(tbloids, '{'); /* * Outer loop iterates once per table, not once per row. Incrementing of @@ -8617,7 +8590,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 = (bool *) pg_malloc(numatts * sizeof(bool)); - tbinfo->localNotNull = (bool *) pg_malloc(numatts * sizeof(bool)); + tbinfo->inhNotNull = (bool *) pg_malloc(numatts * sizeof(bool)); tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(numatts * sizeof(AttrDefInfo *)); hasdefaults = false; @@ -8639,7 +8612,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attalign[j] = *(PQgetvalue(res, r, i_attalign)); tbinfo->attislocal[j] = (PQgetvalue(res, r, i_attislocal)[0] == 't'); tbinfo->notnull[j] = (PQgetvalue(res, r, i_attnotnull)[0] == 't'); - tbinfo->localNotNull[j] = (PQgetvalue(res, r, i_localnotnull)[0] == 't'); tbinfo->attoptions[j] = pg_strdup(PQgetvalue(res, r, i_attoptions)); tbinfo->attcollation[j] = atooid(PQgetvalue(res, r, i_attcollation)); tbinfo->attcompression[j] = *(PQgetvalue(res, r, i_attcompression)); @@ -8648,14 +8620,16 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) tbinfo->attrdefs[j] = NULL; /* fix below */ if (PQgetvalue(res, r, i_atthasdef)[0] == 't') hasdefaults = true; + /* these flags will be set in flagInhAttrs() */ + tbinfo->inhNotNull[j] = false; } if (hasdefaults) { /* Collect OIDs of interesting tables that have defaults */ - if (defaultoids->len > 1) /* do we have more than the '{'? */ - appendPQExpBufferChar(defaultoids, ','); - appendPQExpBuffer(defaultoids, "%u", tbinfo->dobj.catId.oid); + if (tbloids->len > 1) /* do we have more than the '{'? */ + appendPQExpBufferChar(tbloids, ','); + appendPQExpBuffer(tbloids, "%u", tbinfo->dobj.catId.oid); } } @@ -8665,7 +8639,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) * Now get info about column defaults. This is skipped for a data-only * dump, as it is only needed for table schemas. */ - if (!dopt->dataOnly && defaultoids->len > 1) + if (!dopt->dataOnly && tbloids->len > 1) { AttrDefInfo *attrdefs; int numDefaults; @@ -8673,14 +8647,14 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) pg_log_info("finding table default expressions"); - appendPQExpBufferChar(defaultoids, '}'); + appendPQExpBufferChar(tbloids, '}'); printfPQExpBuffer(q, "SELECT a.tableoid, a.oid, adrelid, adnum, " "pg_catalog.pg_get_expr(adbin, adrelid) AS adsrc\n" "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_attrdef a ON (src.tbloid = a.adrelid)\n" "ORDER BY a.adrelid, a.adnum", - defaultoids->data); + tbloids->data); res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK); @@ -8923,112 +8897,9 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables) PQclear(res); } - /* - * Get info about table NOT NULL constraints. This is skipped for a - * data-only dump, as it is only needed for table schemas. - * - * Optimizing for tables that have no NOT NULL constraint seems - * pointless, so we don't try. - */ - if (!dopt->dataOnly) - { - ConstraintInfo *constrs; - int numConstrs; - int i_tableoid; - int i_oid; - int i_conrelid; - int i_conname; - int i_condef; - int i_conislocal; - - pg_log_info("finding table not null constraints"); - - /* - * Only constraints marked connoinherit need to be handled here; - * the normal constraints are instead handled by writing NOT NULL - * when each column is defined. - */ - resetPQExpBuffer(q); - appendPQExpBuffer(q, - "SELECT co.tableoid, co.oid, conrelid, conname, " - "pg_catalog.pg_get_constraintdef(co.oid) AS condef,\n" - " conislocal, coninhcount, connoinherit " - "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" - "JOIN pg_catalog.pg_constraint co ON (src.tbloid = co.conrelid)\n" - "JOIN pg_catalog.pg_class c ON (conrelid = c.oid)\n" - "WHERE contype = 'n' AND connoinherit\n" - "ORDER BY co.conrelid, co.conname", - tbloids->data); - - res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK); - - numConstrs = PQntuples(res); - constrs = (ConstraintInfo *) pg_malloc(numConstrs * sizeof(ConstraintInfo)); - - i_tableoid = PQfnumber(res, "tableoid"); - i_oid = PQfnumber(res, "oid"); - i_conrelid = PQfnumber(res, "conrelid"); - i_conname = PQfnumber(res, "conname"); - i_condef = PQfnumber(res, "condef"); - i_conislocal = PQfnumber(res, "conislocal"); - - /* As above, this loop iterates once per table, not once per row */ - curtblindx = -1; - for (int j = 0; j < numConstrs;) - { - Oid conrelid = atooid(PQgetvalue(res, j, i_conrelid)); - TableInfo *tbinfo = NULL; - int numcons; - - /* Count rows for this table */ - for (numcons = 1; numcons < numConstrs - j; numcons++) - if (atooid(PQgetvalue(res, j + numcons, i_conrelid)) != conrelid) - break; - - /* - * Locate the associated TableInfo; we rely on tblinfo[] being in - * OID order. - */ - while (++curtblindx < numTables) - { - tbinfo = &tblinfo[curtblindx]; - if (tbinfo->dobj.catId.oid == conrelid) - break; - } - if (curtblindx >= numTables) - pg_fatal("unrecognized table OID %u", conrelid); - - for (int c = 0; c < numcons; c++, j++) - { - constrs[j].dobj.objType = DO_CONSTRAINT; - constrs[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_tableoid)); - constrs[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_oid)); - AssignDumpId(&constrs[j].dobj); - constrs[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_conname)); - constrs[j].dobj.namespace = tbinfo->dobj.namespace; - constrs[j].contable = tbinfo; - constrs[j].condomain = NULL; - constrs[j].contype = 'n'; - constrs[j].condef = pg_strdup(PQgetvalue(res, j, i_condef)); - constrs[j].confrelid = InvalidOid; - constrs[j].conindex = 0; - constrs[j].condeferrable = false; - constrs[j].condeferred = false; - constrs[j].conislocal = (PQgetvalue(res, j, i_conislocal)[0] == 't'); - - constrs[j].separate = true; - - constrs[j].dobj.dump = tbinfo->dobj.dump; - } - } - - PQclear(res); - } - destroyPQExpBuffer(q); destroyPQExpBuffer(tbloids); destroyPQExpBuffer(checkoids); - destroyPQExpBuffer(defaultoids); } /* @@ -15701,12 +15572,12 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) !tbinfo->attrdefs[j]->separate); /* - * Not Null constraint --- suppress unless it is locally - * defined, except if partition, or in binary-upgrade case - * where that won't work. + * Not Null constraint --- suppress if inherited, except + * if partition, or in binary-upgrade case where that + * won't work. */ print_notnull = (tbinfo->notnull[j] && - (tbinfo->localNotNull[j] || + (!tbinfo->inhNotNull[j] || tbinfo->ispartition || dopt->binary_upgrade)); /* @@ -16099,8 +15970,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) * we have to mark it separately. */ if (!shouldPrintColumn(dopt, tbinfo, j) && - tbinfo->notnull[j] && tbinfo->localNotNull[j] && - tbinfo->ispartition) + tbinfo->notnull[j] && !tbinfo->inhNotNull[j]) appendPQExpBuffer(q, "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET NOT NULL;\n", foreign, qualrelname, @@ -16925,31 +16795,6 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo) .createStmt = q->data, .dropStmt = delq->data)); } - else if (coninfo->contype == 'n') - { - appendPQExpBuffer(q, "ALTER %sTABLE %s\n", foreign, - fmtQualifiedDumpable(tbinfo)); - appendPQExpBuffer(q, " ADD CONSTRAINT %s %s;\n", - fmtId(coninfo->dobj.name), - coninfo->condef); - - appendPQExpBuffer(delq, "ALTER %sTABLE %s\n", foreign, - fmtQualifiedDumpable(tbinfo)); - appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n", - fmtId(coninfo->dobj.name)); - - tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name); - - if (coninfo->dobj.dump & DUMP_COMPONENT_DEFINITION) - ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId, - ARCHIVE_OPTS(.tag = tag, - .namespace = tbinfo->dobj.namespace->dobj.name, - .owner = tbinfo->rolname, - .description = "NOT NULL CONSTRAINT", - .section = SECTION_POST_DATA, - .createStmt = q->data, - .dropStmt = delq->data)); - } else if (coninfo->contype == 'c' && tbinfo) { /* CHECK constraint on a table */ diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 765fe6399a..ed6ce41ad7 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -345,7 +345,7 @@ typedef struct _tableInfo char **attfdwoptions; /* per-attribute fdw options */ char **attmissingval; /* per attribute missing value */ bool *notnull; /* NOT NULL constraints on attributes */ - bool *localNotNull; /* true if NOT NULL has local definition */ + bool *inhNotNull; /* true if NOT NULL is inherited */ struct _attrDefInfo **attrdefs; /* DEFAULT expressions */ struct _constraintInfo *checkexprs; /* CHECK constraints */ bool needs_override; /* has GENERATED ALWAYS AS IDENTITY */ diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index afd1bb2e9a..93e24d5145 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -3115,7 +3115,7 @@ my %tests = ( );', regexp => qr/^ \QCREATE TABLE dump_test.fk_reference_test_table (\E - \n\s+\Qcol1 integer\E + \n\s+\Qcol1 integer NOT NULL\E \n\); /xm, like => @@ -3507,7 +3507,7 @@ my %tests = ( );', regexp => qr/^ \QCREATE TABLE dump_test.test_table_generated (\E\n - \s+\Qcol1 integer,\E\n + \s+\Qcol1 integer NOT NULL,\E\n \s+\Qcol2 integer GENERATED ALWAYS AS ((col1 * 2)) STORED\E\n \); /xms, @@ -3621,7 +3621,7 @@ my %tests = ( ) INHERITS (dump_test.test_inheritance_parent);', regexp => qr/^ \QCREATE TABLE dump_test.test_inheritance_child (\E\n - \s+\Qcol1 integer NOT NULL,\E\n + \s+\Qcol1 integer,\E\n \s+\QCONSTRAINT test_inheritance_child CHECK ((col2 >= 142857))\E\n \)\n \QINHERITS (dump_test.test_inheritance_parent);\E\n |
