From: Álvaro Herrera Date: Fri, 13 Dec 2024 06:38:49 +0000 (+0100) Subject: Dump not-null constraints on inherited columns correctly X-Git-Tag: REL_18_BETA1~1296 X-Git-Url: http://git.postgresql.org/gitweb/?a=commitdiff_plain;h=fd41ba93e4630921a72ed5127cd0d552a8f3f8fc;p=postgresql.git Dump not-null constraints on inherited columns correctly With not-null constraints defined in child tables for columns that are coming from their parent tables, we were printing ALTER TABLE SET NOT NULL commands that were missing the constraint name, so the original constraint name was being lost, which is bogus. Fix by instead adding a table-constraint constraint declaration with the correct constraint name in the CREATE TABLE instead. Oversight in commit 14e87ffa5c54. We could have fixed it by changing the ALTER TABLE SET NOT NULL to ALTER TABLE ADD CONSTRAINT, but I'm not sure that's any better. A potential problem here might be that if sent to a non-Postgres server, the new pg_dump output would fail because the "CONSTRAINT foo NOT NULL colname" syntax isn't SQL-conforming. However, Postgres' implementation of inheritance is already non-SQL-conforming, so that'd likely fail anyway. This problem was only noticed by Ashutosh's proposed test framework for pg_dump, https://postgr.es/m/CAExHW5uF5V=Cjecx3_Z=7xfh4rg2Wf61PT+hfquzjBqouRzQJQ@mail.gmail.com Author: Ashutosh Bapat Reported-by: Ashutosh Bapat Reviewed-by: Alvaro Herrera Discussion: https://postgr.es/m/CAExHW5tbdgAKDfqjDJ-7Fk6PJtHg8D4zUF6FQ4H2Pq8zK38Nyw@mail.gmail.com --- diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 89276524ae0..d34b8ed4bb1 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -16220,6 +16220,38 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) fmtQualifiedDumpable(coll)); } } + + /* + * On the other hand, if we choose not to print a column + * (likely because it is created by inheritance), but the + * column has a locally-defined not-null constraint, we need + * to dump the constraint as a standalone object. + * + * This syntax isn't SQL-conforming, but if you wanted + * standard output you wouldn't be creating non-standard + * objects to begin with. + */ + if (!shouldPrintColumn(dopt, tbinfo, j) && + !tbinfo->attisdropped[j] && + tbinfo->notnull_constrs[j] != NULL && + tbinfo->notnull_islocal[j]) + { + /* Format properly if not first attr */ + if (actual_atts == 0) + appendPQExpBufferStr(q, " ("); + else + appendPQExpBufferChar(q, ','); + appendPQExpBufferStr(q, "\n "); + actual_atts++; + + if (tbinfo->notnull_constrs[j][0] == '\0') + appendPQExpBuffer(q, "NOT NULL %s", + fmtId(tbinfo->attnames[j])); + else + appendPQExpBuffer(q, "CONSTRAINT %s NOT NULL %s", + tbinfo->notnull_constrs[j], + fmtId(tbinfo->attnames[j])); + } } /* @@ -16661,29 +16693,6 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo) if (tbinfo->attisdropped[j]) continue; - /* - * If we didn't dump the column definition explicitly above, and - * it is not-null and did not inherit that property from a parent, - * we have to mark it separately. - */ - if (!shouldPrintColumn(dopt, tbinfo, j) && - tbinfo->notnull_constrs[j] != NULL && - (tbinfo->notnull_islocal[j] && !tbinfo->ispartition && !dopt->binary_upgrade)) - { - /* No constraint name desired? */ - if (tbinfo->notnull_constrs[j][0] == '\0') - appendPQExpBuffer(q, - "ALTER %sTABLE ONLY %s ALTER COLUMN %s SET NOT NULL;\n", - foreign, qualrelname, - fmtId(tbinfo->attnames[j])); - else - appendPQExpBuffer(q, - "ALTER %sTABLE ONLY %s ADD CONSTRAINT %s NOT NULL %s;\n", - foreign, qualrelname, - tbinfo->notnull_constrs[j], - fmtId(tbinfo->attnames[j])); - } - /* * Dump per-column statistics information. We only issue an ALTER * TABLE statement if the attstattarget entry for this column is