]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
pg_dump: include comments on not-null constraints on domains, too
authorÁlvaro Herrera <alvherre@kurilemu.de>
Mon, 21 Jul 2025 09:34:10 +0000 (11:34 +0200)
committerÁlvaro Herrera <alvherre@kurilemu.de>
Mon, 21 Jul 2025 09:34:10 +0000 (11:34 +0200)
Commit e5da0fe3c22b introduced catalog entries for not-null constraints
on domains; but because commit b0e96f311985 (the original work for
catalogued not-null constraints on tables) forgot to teach pg_dump to
process the comments for them, this one also forgot.  Add that now.

We also need to teach repairDependencyLoop() about the new type of
constraints being possible for domains.

Backpatch-through: 17
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>
Discussion: https://postgr.es/m/CACJufxF-0bqVR=j4jonS6N2Ka6hHUpFyu3_3TWKNhOW_4yFSSg@mail.gmail.com

src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/pg_dump.h
src/bin/pg_dump/pg_dump_sort.c
src/bin/pg_dump/t/002_pg_dump.pl

index 06f630a910dc8f749157601e6df4e27a31ec028a..2626dd250c8eb2e0420075c9aeae890ccff380ba 100644 (file)
@@ -47,6 +47,7 @@
 #include "catalog/pg_authid_d.h"
 #include "catalog/pg_cast_d.h"
 #include "catalog/pg_class_d.h"
+#include "catalog/pg_constraint_d.h"
 #include "catalog/pg_default_acl_d.h"
 #include "catalog/pg_largeobject_d.h"
 #include "catalog/pg_largeobject_metadata_d.h"
@@ -5929,6 +5930,7 @@ getTypes(Archive *fout, int *numTypes)
                 */
                tyinfo[i].nDomChecks = 0;
                tyinfo[i].domChecks = NULL;
+               tyinfo[i].notnull = NULL;
                if ((tyinfo[i].dobj.dump & DUMP_COMPONENT_DEFINITION) &&
                        tyinfo[i].typtype == TYPTYPE_DOMAIN)
                        getDomainConstraints(fout, &(tyinfo[i]));
@@ -7949,27 +7951,33 @@ addConstrChildIdxDeps(DumpableObject *dobj, const IndxInfo *refidx)
 static void
 getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
 {
-       int                     i;
        ConstraintInfo *constrinfo;
        PQExpBuffer query = createPQExpBuffer();
        PGresult   *res;
        int                     i_tableoid,
                                i_oid,
                                i_conname,
-                               i_consrc;
+                               i_consrc,
+                               i_convalidated,
+                               i_contype;
        int                     ntups;
 
        if (!fout->is_prepared[PREPQUERY_GETDOMAINCONSTRAINTS])
        {
-               /* Set up query for constraint-specific details */
-               appendPQExpBufferStr(query,
-                                                        "PREPARE getDomainConstraints(pg_catalog.oid) AS\n"
-                                                        "SELECT tableoid, oid, conname, "
-                                                        "pg_catalog.pg_get_constraintdef(oid) AS consrc, "
-                                                        "convalidated "
-                                                        "FROM pg_catalog.pg_constraint "
-                                                        "WHERE contypid = $1 AND contype = 'c' "
-                                                        "ORDER BY conname");
+               /*
+                * Set up query for constraint-specific details.  For servers 17 and
+                * up, domains have constraints of type 'n' as well as 'c', otherwise
+                * just the latter.
+                */
+               appendPQExpBuffer(query,
+                                                 "PREPARE getDomainConstraints(pg_catalog.oid) AS\n"
+                                                 "SELECT tableoid, oid, conname, "
+                                                 "pg_catalog.pg_get_constraintdef(oid) AS consrc, "
+                                                 "convalidated, contype "
+                                                 "FROM pg_catalog.pg_constraint "
+                                                 "WHERE contypid = $1 AND contype IN (%s) "
+                                                 "ORDER BY conname",
+                                                 fout->remoteVersion < 170000 ? "'c'" : "'c', 'n'");
 
                ExecuteSqlStatement(fout, query->data);
 
@@ -7988,33 +7996,50 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
        i_oid = PQfnumber(res, "oid");
        i_conname = PQfnumber(res, "conname");
        i_consrc = PQfnumber(res, "consrc");
+       i_convalidated = PQfnumber(res, "convalidated");
+       i_contype = PQfnumber(res, "contype");
 
        constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
-
-       tyinfo->nDomChecks = ntups;
        tyinfo->domChecks = constrinfo;
 
-       for (i = 0; i < ntups; i++)
+       /* 'i' tracks result rows; 'j' counts CHECK constraints */
+       for (int i = 0, j = 0; i < ntups; i++)
        {
-               bool            validated = PQgetvalue(res, i, 4)[0] == 't';
-
-               constrinfo[i].dobj.objType = DO_CONSTRAINT;
-               constrinfo[i].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
-               constrinfo[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
-               AssignDumpId(&constrinfo[i].dobj);
-               constrinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_conname));
-               constrinfo[i].dobj.namespace = tyinfo->dobj.namespace;
-               constrinfo[i].contable = NULL;
-               constrinfo[i].condomain = tyinfo;
-               constrinfo[i].contype = 'c';
-               constrinfo[i].condef = pg_strdup(PQgetvalue(res, i, i_consrc));
-               constrinfo[i].confrelid = InvalidOid;
-               constrinfo[i].conindex = 0;
-               constrinfo[i].condeferrable = false;
-               constrinfo[i].condeferred = false;
-               constrinfo[i].conislocal = true;
-
-               constrinfo[i].separate = !validated;
+               bool            validated = PQgetvalue(res, i, i_convalidated)[0] == 't';
+               char            contype = (PQgetvalue(res, i, i_contype))[0];
+               ConstraintInfo *constraint;
+
+               if (contype == CONSTRAINT_CHECK)
+               {
+                       constraint = &constrinfo[j++];
+                       tyinfo->nDomChecks++;
+               }
+               else
+               {
+                       Assert(contype == CONSTRAINT_NOTNULL);
+                       Assert(tyinfo->notnull == NULL);
+                       /* use last item in array for the not-null constraint */
+                       tyinfo->notnull = &(constrinfo[ntups - 1]);
+                       constraint = tyinfo->notnull;
+               }
+
+               constraint->dobj.objType = DO_CONSTRAINT;
+               constraint->dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
+               constraint->dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
+               AssignDumpId(&(constraint->dobj));
+               constraint->dobj.name = pg_strdup(PQgetvalue(res, i, i_conname));
+               constraint->dobj.namespace = tyinfo->dobj.namespace;
+               constraint->contable = NULL;
+               constraint->condomain = tyinfo;
+               constraint->contype = contype;
+               constraint->condef = pg_strdup(PQgetvalue(res, i, i_consrc));
+               constraint->confrelid = InvalidOid;
+               constraint->conindex = 0;
+               constraint->condeferrable = false;
+               constraint->condeferred = false;
+               constraint->conislocal = true;
+
+               constraint->separate = !validated;
 
                /*
                 * Make the domain depend on the constraint, ensuring it won't be
@@ -8023,8 +8048,7 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
                 * anyway, so this doesn't matter.
                 */
                if (validated)
-                       addObjectDependency(&tyinfo->dobj,
-                                                               constrinfo[i].dobj.dumpId);
+                       addObjectDependency(&tyinfo->dobj, constraint->dobj.dumpId);
        }
 
        PQclear(res);
@@ -11557,8 +11581,36 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
                        appendPQExpBuffer(q, " COLLATE %s", fmtQualifiedDumpable(coll));
        }
 
+       /*
+        * Print a not-null constraint if there's one.  In servers older than 17
+        * these don't have names, so just print it unadorned; in newer ones they
+        * do, but most of the time it's going to be the standard generated one,
+        * so omit the name in that case also.
+        */
        if (typnotnull[0] == 't')
-               appendPQExpBufferStr(q, " NOT NULL");
+       {
+               if (fout->remoteVersion < 170000 || tyinfo->notnull == NULL)
+                       appendPQExpBufferStr(q, " NOT NULL");
+               else
+               {
+                       ConstraintInfo *notnull = tyinfo->notnull;
+
+                       if (!notnull->separate)
+                       {
+                               char       *default_name;
+
+                               /* XXX should match ChooseConstraintName better */
+                               default_name = psprintf("%s_not_null", tyinfo->dobj.name);
+
+                               if (strcmp(default_name, notnull->dobj.name) == 0)
+                                       appendPQExpBufferStr(q, " NOT NULL");
+                               else
+                                       appendPQExpBuffer(q, " CONSTRAINT %s %s",
+                                                                         fmtId(notnull->dobj.name), notnull->condef);
+                               free(default_name);
+                       }
+               }
+       }
 
        if (typdefault != NULL)
        {
@@ -11578,7 +11630,7 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
        {
                ConstraintInfo *domcheck = &(tyinfo->domChecks[i]);
 
-               if (!domcheck->separate)
+               if (!domcheck->separate && domcheck->contype == 'c')
                        appendPQExpBuffer(q, "\n\tCONSTRAINT %s %s",
                                                          fmtId(domcheck->dobj.name), domcheck->condef);
        }
@@ -11642,6 +11694,25 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
                destroyPQExpBuffer(conprefix);
        }
 
+       /*
+        * And a comment on the not-null constraint, if there's one -- but only if
+        * the constraint itself was dumped here
+        */
+       if (tyinfo->notnull != NULL && !tyinfo->notnull->separate)
+       {
+               PQExpBuffer conprefix = createPQExpBuffer();
+
+               appendPQExpBuffer(conprefix, "CONSTRAINT %s ON DOMAIN",
+                                                 fmtId(tyinfo->notnull->dobj.name));
+
+               if (tyinfo->notnull->dobj.dump & DUMP_COMPONENT_COMMENT)
+                       dumpComment(fout, conprefix->data, qtypname,
+                                               tyinfo->dobj.namespace->dobj.name,
+                                               tyinfo->rolname,
+                                               tyinfo->notnull->dobj.catId, 0, tyinfo->dobj.dumpId);
+               destroyPQExpBuffer(conprefix);
+       }
+
        destroyPQExpBuffer(q);
        destroyPQExpBuffer(delq);
        destroyPQExpBuffer(query);
@@ -17336,14 +17407,23 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo)
                                                                                  .dropStmt = delq->data));
                }
        }
-       else if (coninfo->contype == 'c' && tbinfo == NULL)
+       else if (tbinfo == NULL)
        {
-               /* CHECK constraint on a domain */
+               /* CHECK, NOT NULL constraint on a domain */
                TypeInfo   *tyinfo = coninfo->condomain;
 
+               Assert(coninfo->contype == 'c' || coninfo->contype == 'n');
+
                /* Ignore if not to be dumped separately */
                if (coninfo->separate)
                {
+                       const char *keyword;
+
+                       if (coninfo->contype == 'c')
+                               keyword = "CHECK CONSTRAINT";
+                       else
+                               keyword = "CONSTRAINT";
+
                        appendPQExpBuffer(q, "ALTER DOMAIN %s\n",
                                                          fmtQualifiedDumpable(tyinfo));
                        appendPQExpBuffer(q, "    ADD CONSTRAINT %s %s;\n",
@@ -17362,7 +17442,7 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo)
                                                         ARCHIVE_OPTS(.tag = tag,
                                                                                  .namespace = tyinfo->dobj.namespace->dobj.name,
                                                                                  .owner = tyinfo->rolname,
-                                                                                 .description = "CHECK CONSTRAINT",
+                                                                                 .description = keyword,
                                                                                  .section = SECTION_POST_DATA,
                                                                                  .createStmt = q->data,
                                                                                  .dropStmt = delq->data));
index 1d352fe12d1acf5cd93c157f99326fb3377699a3..243942369ea257e69d9c2f7a43097cc87e6ed7cf 100644 (file)
@@ -215,7 +215,9 @@ typedef struct _typeInfo
        bool            isDefined;              /* true if typisdefined */
        /* If needed, we'll create a "shell type" entry for it; link that here: */
        struct _shellTypeInfo *shellType;       /* shell-type entry, or NULL */
-       /* If it's a domain, we store links to its constraints here: */
+       /* If it's a domain, its not-null constraint is here: */
+       struct _constraintInfo *notnull;
+       /* If it's a domain, we store links to its CHECK constraints here: */
        int                     nDomChecks;
        struct _constraintInfo *domChecks;
 } TypeInfo;
index 31bdb91a585ad2c815648b0802fb93cc1602ca64..d85f3d0e2d833552f34c4af0f4c650d61255334f 100644 (file)
@@ -884,7 +884,7 @@ repairTableAttrDefMultiLoop(DumpableObject *tableobj,
 }
 
 /*
- * CHECK constraints on domains work just like those on tables ...
+ * CHECK, NOT NULL constraints on domains work just like those on tables ...
  */
 static void
 repairDomainConstraintLoop(DumpableObject *domainobj,
@@ -1135,11 +1135,12 @@ repairDependencyLoop(DumpableObject **loop,
                }
        }
 
-       /* Domain and CHECK constraint */
+       /* Domain and CHECK or NOT NULL constraint */
        if (nLoop == 2 &&
                loop[0]->objType == DO_TYPE &&
                loop[1]->objType == DO_CONSTRAINT &&
-               ((ConstraintInfo *) loop[1])->contype == 'c' &&
+               (((ConstraintInfo *) loop[1])->contype == 'c' ||
+                ((ConstraintInfo *) loop[1])->contype == 'n') &&
                ((ConstraintInfo *) loop[1])->condomain == (TypeInfo *) loop[0])
        {
                repairDomainConstraintLoop(loop[0], loop[1]);
@@ -1148,14 +1149,15 @@ repairDependencyLoop(DumpableObject **loop,
        if (nLoop == 2 &&
                loop[1]->objType == DO_TYPE &&
                loop[0]->objType == DO_CONSTRAINT &&
-               ((ConstraintInfo *) loop[0])->contype == 'c' &&
+               (((ConstraintInfo *) loop[0])->contype == 'c' ||
+                ((ConstraintInfo *) loop[0])->contype == 'n') &&
                ((ConstraintInfo *) loop[0])->condomain == (TypeInfo *) loop[1])
        {
                repairDomainConstraintLoop(loop[1], loop[0]);
                return;
        }
 
-       /* Indirect loop involving domain and CHECK constraint */
+       /* Indirect loop involving domain and CHECK or NOT NULL constraint */
        if (nLoop > 2)
        {
                for (i = 0; i < nLoop; i++)
@@ -1165,7 +1167,8 @@ repairDependencyLoop(DumpableObject **loop,
                                for (j = 0; j < nLoop; j++)
                                {
                                        if (loop[j]->objType == DO_CONSTRAINT &&
-                                               ((ConstraintInfo *) loop[j])->contype == 'c' &&
+                                               (((ConstraintInfo *) loop[j])->contype == 'c' ||
+                                                ((ConstraintInfo *) loop[j])->contype == 'n') &&
                                                ((ConstraintInfo *) loop[j])->condomain == (TypeInfo *) loop[i])
                                        {
                                                repairDomainConstraintMultiLoop(loop[i], loop[j]);
index 5bcc2244d583166d7e5e62d260b8068680d4d5c0..58306d307eceea2477bfc0bce2c26e6e98d86810 100644 (file)
@@ -2041,17 +2041,19 @@ my %tests = (
                create_sql => 'CREATE DOMAIN dump_test.us_postal_code AS TEXT
                               COLLATE "C"
                                           DEFAULT \'10014\'
+                                          CONSTRAINT nn NOT NULL
                                           CHECK(VALUE ~ \'^\d{5}$\' OR
                                                         VALUE ~ \'^\d{5}-\d{4}$\');
+                                          COMMENT ON CONSTRAINT nn
+                                                ON DOMAIN dump_test.us_postal_code IS \'not null\';
                                           COMMENT ON CONSTRAINT us_postal_code_check
                                                 ON DOMAIN dump_test.us_postal_code IS \'check it\';',
                regexp => qr/^
-                       \QCREATE DOMAIN dump_test.us_postal_code AS text COLLATE pg_catalog."C" DEFAULT '10014'::text\E\n\s+
+                       \QCREATE DOMAIN dump_test.us_postal_code AS text COLLATE pg_catalog."C" CONSTRAINT nn NOT NULL DEFAULT '10014'::text\E\n\s+
                        \QCONSTRAINT us_postal_code_check CHECK \E
                        \Q(((VALUE ~ '^\d{5}\E
                        \$\Q'::text) OR (VALUE ~ '^\d{5}-\d{4}\E\$
                        \Q'::text)));\E(.|\n)*
-                       \QCOMMENT ON CONSTRAINT us_postal_code_check ON DOMAIN dump_test.us_postal_code IS 'check it';\E
                        /xm,
                like =>
                  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
@@ -2061,6 +2063,30 @@ my %tests = (
                },
        },
 
+       'COMMENT ON CONSTRAINT ON DOMAIN (1)' => {
+               regexp => qr/^
+               \QCOMMENT ON CONSTRAINT nn ON DOMAIN dump_test.us_postal_code IS '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,
+               },
+       },
+
+       'COMMENT ON CONSTRAINT ON DOMAIN (2)' => {
+               regexp => qr/^
+               \QCOMMENT ON CONSTRAINT us_postal_code_check ON DOMAIN dump_test.us_postal_code IS 'check it';\E
+               /xm,
+               like =>
+                 { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+               unlike => {
+                       exclude_dump_test_schema => 1,
+                       only_dump_measurement => 1,
+               },
+       },
+
        'CREATE FUNCTION dump_test.pltestlang_call_handler' => {
                create_order => 17,
                create_sql => 'CREATE FUNCTION dump_test.pltestlang_call_handler()