On Wed, May 7, 2025 at 10:56 PM Álvaro Herrera <alvhe...@kurilemu.de> wrote: > > Ooh, you know what? I ran your the CREATE DOMAIN statement in your new > test and pg_dump'ed that, and as far as I can tell the _name_ of the > not-null constraint is not dumped either. So it's not just the comment > that we're losing. That's a separate bug, and probably needs to be > fixed first, although I'm not sure if that one is going to be > back-patchable. Which means, I withdraw my earlier assessment that the > comment fix is back-patchable as a whole. >
for PG17. we change the dump of create domain test.d4 as int constraint nn not null; from CREATE DOMAIN test.d4 AS integer NOT NULL; to CREATE DOMAIN test.d4 AS integer CONSTRAINT nn NOT NULL; in PG17 we didn't preserve the constraint name, now if we did, then it's a bug fix, so it can be back-patchable? Anyway, the attachment is for PG18 only now, it will fix the domain constraint name loss and domain not-null comments loss issue together.
From 2db869fc212867e74f61f4e5b2aef1fda017e9f7 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Thu, 8 May 2025 12:48:25 +0800 Subject: [PATCH v3 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 | 32 ++++++++++++++++---------------- src/bin/pg_dump/pg_dump.h | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 7837d4cea36..b3064f1b8d3 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++) { @@ -12508,28 +12508,28 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) /* * Add any CHECK, NOT NULL 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) + if (!domconstr->separate) { - if (domcheck->contype == 'n') + if (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, "\n\tCONSTRAINT %s %s", - fmtId(domcheck->dobj.name), domcheck->condef); + fmtId(domconstr->dobj.name), domconstr->condef); } else appendPQExpBuffer(q, "\n\tCONSTRAINT %s %s", - fmtId(domcheck->dobj.name), domcheck->condef); + fmtId(domconstr->dobj.name), domconstr->condef); } } @@ -12570,19 +12570,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
From c6a2e754a5292b72a7e5e328ca4684f9c39517e8 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Thu, 8 May 2025 12:29:01 +0800 Subject: [PATCH v3 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. 2. Similar to PG18 table not-null constraint, conditaionlly print out not-null constraint name, This only apply to PG18. discussion: https://postgr.es/m/CACJufxF-0bqVR=j4jons6n2ka6hhupfyu3_3twknhow_4yf...@mail.gmail.com --- src/bin/pg_dump/pg_dump.c | 61 +++++++++++++++++++++++++------- src/bin/pg_dump/pg_dump_sort.c | 9 ++--- src/bin/pg_dump/t/002_pg_dump.pl | 5 +++ 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index e2e7975b34e..7837d4cea36 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 "); + + /* + * in PG18 or later versions, not-null domain constraint catalog + * information is stored in the pg_constraint. + */ + if (fout->remoteVersion < 180000) + 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; @@ -12479,7 +12490,8 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) appendPQExpBuffer(q, " COLLATE %s", fmtQualifiedDumpable(coll)); } - if (typnotnull[0] == 't') + /* PG18 or later versions is handled in below */ + if (typnotnull[0] == 't' && fout->remoteVersion < 180000) appendPQExpBufferStr(q, " NOT NULL"); if (typdefault != NULL) @@ -12494,15 +12506,31 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) PQclear(res); /* - * Add any CHECK constraints for the domain + * Add any CHECK, NOT NULL constraints for the domain */ for (i = 0; i < tyinfo->nDomChecks; i++) { ConstraintInfo *domcheck = &(tyinfo->domChecks[i]); if (!domcheck->separate) - appendPQExpBuffer(q, "\n\tCONSTRAINT %s %s", - fmtId(domcheck->dobj.name), domcheck->condef); + { + if (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, "\n\tCONSTRAINT %s %s", + fmtId(domcheck->dobj.name), domcheck->condef); + } + else + appendPQExpBuffer(q, "\n\tCONSTRAINT %s %s", + fmtId(domcheck->dobj.name), domcheck->condef); + } } appendPQExpBufferStr(q, ";\n"); @@ -18370,14 +18398,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", @@ -18396,7 +18433,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 55d892d9c16..88369bc98ae 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -2289,16 +2289,21 @@ 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+ + \QCONSTRAINT nn NOT NULL \E \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 \QCOMMENT ON CONSTRAINT us_postal_code_check ON DOMAIN dump_test.us_postal_code IS 'check it';\E /xm, like => -- 2.34.1
