hi. "Catalog domain not-null constraints" commit[1] doesn't have a pg_dump test.
I actually found another bug. create schema test; CREATE DOMAIN test.d1 AS integer NOT NULL default 11; pg_dump --schema=test > 1.sql "" pg_dump: warning: could not resolve dependency loop among these items: pg_dump: detail: TYPE d1 (ID 1415 OID 18007) pg_dump: detail: CONSTRAINT d1_not_null (ID 1416 OID 18008) "" Catalog domain not-null constraints is committed in 17, so no need to worry about before 17 branches. The attached patch will work for PG17 and PG18. You can use the following SQL examples to compare with master and see the intended effect of my attached patch. CREATE DOMAIN test.d1 AS integer NOT NULL default 11; CREATE DOMAIN test.d3 AS integer constraint nn NOT NULL default 11; [1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=e5da0fe3c22b34c4433f1729e88495554b5331ed
From aaa19222079d27c554c8b06a2e95b2f2581bccd8 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Thu, 22 May 2025 14:25:58 +0800 Subject: [PATCH v4 1/2] Improve pg_dump handling of domain not-null constraints 1. If requested, we should dump comments for domain not-null constraints. Note: In PostgreSQL 16 and earlier, these constraints do not have entries in pg_constraint. this patch can apply to PG17 and up. discussion: https://postgr.es/m/CACJufxF-0bqVR=j4jons6n2ka6hhupfyu3_3twknhow_4yf...@mail.gmail.com --- src/bin/pg_dump/pg_dump.c | 64 +++++++++++++++++++++++++++----- src/bin/pg_dump/pg_dump_sort.c | 9 ++--- src/bin/pg_dump/t/002_pg_dump.pl | 6 ++- 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index c73e73a87d1..fe60ca874b3 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -8250,7 +8250,8 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo) int i_tableoid, i_oid, i_conname, - i_consrc; + i_consrc, + i_contype; int ntups; if (!fout->is_prepared[PREPQUERY_GETDOMAINCONSTRAINTS]) @@ -8260,10 +8261,19 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo) "PREPARE getDomainConstraints(pg_catalog.oid) AS\n" "SELECT tableoid, oid, conname, " "pg_catalog.pg_get_constraintdef(oid) AS consrc, " - "convalidated " + "convalidated, " + "contype " "FROM pg_catalog.pg_constraint " - "WHERE contypid = $1 AND contype = 'c' " - "ORDER BY conname"); + "WHERE contypid = $1 "); + + /* + * only PG17 or later versions, not-null domain constraint catalog + * information is stored in the pg_constraint. + */ + if (fout->remoteVersion < 170000) + appendPQExpBufferStr(query, "AND contype = 'c' "); + + appendPQExpBufferStr(query, "ORDER BY conname"); ExecuteSqlStatement(fout, query->data); @@ -8282,6 +8292,7 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo) i_oid = PQfnumber(res, "oid"); i_conname = PQfnumber(res, "conname"); i_consrc = PQfnumber(res, "consrc"); + i_contype = PQfnumber(res, "contype"); constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo)); @@ -8300,7 +8311,7 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo) constrinfo[i].dobj.namespace = tyinfo->dobj.namespace; constrinfo[i].contable = NULL; constrinfo[i].condomain = tyinfo; - constrinfo[i].contype = 'c'; + constrinfo[i].contype = *(PQgetvalue(res, i, i_contype)); constrinfo[i].condef = pg_strdup(PQgetvalue(res, i, i_consrc)); constrinfo[i].confrelid = InvalidOid; constrinfo[i].conindex = 0; @@ -12485,7 +12496,31 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) } if (typnotnull[0] == 't') - appendPQExpBufferStr(q, " NOT NULL"); + { + if (fout->remoteVersion < 170000) + appendPQExpBufferStr(q, " NOT NULL"); + else + { + for (i = 0; i < tyinfo->nDomChecks; i++) + { + ConstraintInfo *domcheck = &(tyinfo->domChecks[i]); + + if (!domcheck->separate && domcheck->contype == 'n') + { + char *default_name; + + /* XXX should match ChooseConstraintName better */ + default_name = psprintf("%s_not_null", qtypname); + + if (strcmp(default_name, fmtId(domcheck->dobj.name)) == 0) + appendPQExpBufferStr(q, " NOT NULL"); + else + appendPQExpBuffer(q, " CONSTRAINT %s %s", + fmtId(domcheck->dobj.name), domcheck->condef); + } + } + } + } if (typdefault != NULL) { @@ -12505,7 +12540,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); } @@ -18375,14 +18410,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", @@ -18401,7 +18445,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)); diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index 0b0977788f1..e5506ca788c 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -907,7 +907,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, @@ -1173,11 +1173,10 @@ repairDependencyLoop(DumpableObject **loop, } } - /* Domain and CHECK constraint */ + /* Domain and CHECK, NOT NULL constraint */ if (nLoop == 2 && loop[0]->objType == DO_TYPE && loop[1]->objType == DO_CONSTRAINT && - ((ConstraintInfo *) loop[1])->contype == 'c' && ((ConstraintInfo *) loop[1])->condomain == (TypeInfo *) loop[0]) { repairDomainConstraintLoop(loop[0], loop[1]); @@ -1186,14 +1185,13 @@ repairDependencyLoop(DumpableObject **loop, if (nLoop == 2 && loop[1]->objType == DO_TYPE && loop[0]->objType == DO_CONSTRAINT && - ((ConstraintInfo *) loop[0])->contype == 'c' && ((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++) @@ -1203,7 +1201,6 @@ repairDependencyLoop(DumpableObject **loop, for (j = 0; j < nLoop; j++) { if (loop[j]->objType == DO_CONSTRAINT && - ((ConstraintInfo *) loop[j])->contype == 'c' && ((ConstraintInfo *) loop[j])->condomain == (TypeInfo *) loop[i]) { repairDomainConstraintMultiLoop(loop[i], loop[j]); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index cf34f71ea11..4786dace95e 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -2289,16 +2289,20 @@ 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 nn ON DOMAIN dump_test.us_postal_code IS 'not null';\E(.|\n)* \QCOMMENT ON CONSTRAINT us_postal_code_check ON DOMAIN dump_test.us_postal_code IS 'check it';\E /xm, like => -- 2.34.1
From add8949e09407350f0af2ae49bb09c6de8af28f6 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Thu, 22 May 2025 14:21:10 +0800 Subject: [PATCH v4 2/2] sanitize variable or argument name in pg_dump discussion: https://postgr.es/m/CACJufxF-0bqVR=j4jons6n2ka6hhupfyu3_3twknhow_4yf...@mail.gmail.com --- src/bin/pg_dump/pg_dump.c | 36 ++++++++++++++++++------------------ src/bin/pg_dump/pg_dump.h | 4 ++-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index fe60ca874b3..c8ee393c6ea 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -6118,8 +6118,8 @@ getTypes(Archive *fout) /* * If it's a domain, fetch info about its constraints, if any */ - tyinfo[i].nDomChecks = 0; - tyinfo[i].domChecks = NULL; + tyinfo[i].nDomConstrs = 0; + tyinfo[i].domConstrs = NULL; if ((tyinfo[i].dobj.dump & DUMP_COMPONENT_DEFINITION) && tyinfo[i].typtype == TYPTYPE_DOMAIN) getDomainConstraints(fout, &(tyinfo[i])); @@ -8296,8 +8296,8 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo) constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo)); - tyinfo->nDomChecks = ntups; - tyinfo->domChecks = constrinfo; + tyinfo->nDomConstrs = ntups; + tyinfo->domConstrs = constrinfo; for (i = 0; i < ntups; i++) { @@ -12501,22 +12501,22 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) appendPQExpBufferStr(q, " NOT NULL"); else { - for (i = 0; i < tyinfo->nDomChecks; i++) + for (i = 0; i < tyinfo->nDomConstrs; i++) { - ConstraintInfo *domcheck = &(tyinfo->domChecks[i]); + ConstraintInfo *domconstr = &(tyinfo->domConstrs[i]); - if (!domcheck->separate && domcheck->contype == 'n') + if (!domconstr->separate && domconstr->contype == 'n') { char *default_name; /* XXX should match ChooseConstraintName better */ default_name = psprintf("%s_not_null", qtypname); - if (strcmp(default_name, fmtId(domcheck->dobj.name)) == 0) + if (strcmp(default_name, fmtId(domconstr->dobj.name)) == 0) appendPQExpBufferStr(q, " NOT NULL"); else appendPQExpBuffer(q, " CONSTRAINT %s %s", - fmtId(domcheck->dobj.name), domcheck->condef); + fmtId(domconstr->dobj.name), domconstr->condef); } } } @@ -12536,13 +12536,13 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) /* * Add any CHECK constraints for the domain */ - for (i = 0; i < tyinfo->nDomChecks; i++) + for (i = 0; i < tyinfo->nDomConstrs; i++) { - ConstraintInfo *domcheck = &(tyinfo->domChecks[i]); + ConstraintInfo *domconstr = &(tyinfo->domConstrs[i]); - if (!domcheck->separate && domcheck->contype == 'c') + if (!domconstr->separate && domconstr->contype == 'c') appendPQExpBuffer(q, "\n\tCONSTRAINT %s %s", - fmtId(domcheck->dobj.name), domcheck->condef); + fmtId(domconstr->dobj.name), domconstr->condef); } appendPQExpBufferStr(q, ";\n"); @@ -12582,19 +12582,19 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) NULL, tyinfo->rolname, &tyinfo->dacl); /* Dump any per-constraint comments */ - for (i = 0; i < tyinfo->nDomChecks; i++) + for (i = 0; i < tyinfo->nDomConstrs; i++) { - ConstraintInfo *domcheck = &(tyinfo->domChecks[i]); + ConstraintInfo *domconstr = &(tyinfo->domConstrs[i]); PQExpBuffer conprefix = createPQExpBuffer(); appendPQExpBuffer(conprefix, "CONSTRAINT %s ON DOMAIN", - fmtId(domcheck->dobj.name)); + fmtId(domconstr->dobj.name)); - if (domcheck->dobj.dump & DUMP_COMPONENT_COMMENT) + if (domconstr->dobj.dump & DUMP_COMPONENT_COMMENT) dumpComment(fout, conprefix->data, qtypname, tyinfo->dobj.namespace->dobj.name, tyinfo->rolname, - domcheck->dobj.catId, 0, tyinfo->dobj.dumpId); + domconstr->dobj.catId, 0, tyinfo->dobj.dumpId); destroyPQExpBuffer(conprefix); } diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 7417eab6aef..b5e19ecc2e3 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -223,8 +223,8 @@ typedef struct _typeInfo /* 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: */ - int nDomChecks; - struct _constraintInfo *domChecks; + int nDomConstrs; + struct _constraintInfo *domConstrs; } TypeInfo; typedef struct _shellTypeInfo -- 2.34.1
