On Wed, May 7, 2025 at 5:25 PM Álvaro Herrera <alvhe...@kurilemu.de> wrote: > > On 2025-May-07, jian he wrote: > > > in PG17 and master, pg_dump (--schema=test --no-owner) > > [...] > > didn't produce > > COMMENT ON CONSTRAINT nn ON DOMAIN test.d1 IS 'not null constraint on > > domain d1'; > > we should make pg_dump to produce it too? > > Yes, this is clearly a pg17 bug whose fix should be backpatched. > > > The attached patch tries to make it produce comments on not-null > > constraints on domains. > > Thanks, I'll have a look. > > > I aslo renamed struct TypeInfo fields, nDomChecks will be renamed to > > nDomConstrs; domChecks will be renamed to domConstrs. > > TypeInfo->domConstrs will also include not-null constraint > > information, changing from domChecks to domConstrs makes sense, IMHO. > > Hmm, for a backpatch I would leave the field names alone since they are > publicly visible; we can rename separately in pg19 once it opens. Can > you resubmit splitting the renaming out to a 0002 patch? > > -- hi.
i didn't fully understand pg_dump perl dump test, I have added some changes on src/bin/pg_dump/t/002_pg_dump.pl, but it fails the tests.... v2-0001 ensures pg_dump dump domain not-null constraint's comments v2-0002 change some variable/argument/struct element name because of v2-0001.
From e1f94b2b1f21ca8f285b40efe87e1ed64e52b0d1 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Wed, 7 May 2025 22:21:52 +0800 Subject: [PATCH v2 1/2] pg_dump does not dump domain not-null constraint's comments 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. discussion: https://postgr.es/m/CACJufxF-0bqVR=j4jons6n2ka6hhupfyu3_3twknhow_4yf...@mail.gmail.com --- src/bin/pg_dump/pg_dump.c | 41 ++++++++++++++++++++++++-------- src/bin/pg_dump/pg_dump_sort.c | 9 +++---- src/bin/pg_dump/t/002_pg_dump.pl | 5 ++++ 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index e2e7975b34e..ea5e2abb03f 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 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; @@ -12479,7 +12490,8 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo) appendPQExpBuffer(q, " COLLATE %s", fmtQualifiedDumpable(coll)); } - if (typnotnull[0] == 't') + /* PG17 or later versions is handled in below */ + if (typnotnull[0] == 't' && fout->remoteVersion < 170000) appendPQExpBufferStr(q, " NOT NULL"); if (typdefault != NULL) @@ -12494,7 +12506,7 @@ 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++) { @@ -18370,14 +18382,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 +18417,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
From d931c5e16b20bce05a84df7bf435763c255aa7f9 Mon Sep 17 00:00:00 2001 From: jian he <jian.universal...@gmail.com> Date: Wed, 7 May 2025 22:28:32 +0800 Subject: [PATCH v2 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 | 22 +++++++++++----------- src/bin/pg_dump/pg_dump.h | 4 ++-- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index ea5e2abb03f..d5a12e675ce 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,9 +12508,9 @@ 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 *domcheck = &(tyinfo->domConstrs[i]); if (!domcheck->separate) appendPQExpBuffer(q, "\n\tCONSTRAINT %s %s", @@ -12554,19 +12554,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
