On 2025-May-22, jian he wrote:
> 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)
> ""
Oh, interesting. I agree with the rough fix, but I think it's better if
we keep the contype comparisons rather than removing them, relaxing to
allow for one more char.
I didn't like the idea of stashing the not-null constraint in the same
array as the check constraints; it feels a bit dirty (especially because
of the need to scan the array in order to find the not-null one). I
opted to add a separate TypeInfo->notnull pointer instead. This feels
more natural. This works because we know a domain has only one not-null
constraint. Note that this means we don't need your proposed 0002
anymore.
I wonder to what extent we promise ABI compatibility of pg_dump.h
structs in stable branches. If that's an issue, we could easily use
your original patch for 17, and use the TypeInfo->notnull addition only
in 18, but I'm starting to lean on not bothering (i.e., use the same
patch in all branches). Compare commit 7418767f1 which was backpatched
all the way and modified struct StatsExtInfo; I don't think we got any
complaints.
I also modified the Perl test so that the COMMENT ON CONSTRAINT
statement is checked in a separate stanza. This works fine: the comment
is created in the create_sql of one stanza (the same where you had it),
then checked in the 'regexp' entry of another. I opted to move the
regexp for both constraints out of the main one.
The attached applies on top of your patch. Opinions?
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
>From 5d29cbcee5ad62083f232db7fa0a3a7cf76455dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@kurilemu.de>
Date: Mon, 14 Jul 2025 18:25:48 +0200
Subject: [PATCH] fix tweak
---
src/bin/pg_dump/pg_dump.c | 137 +++++++++++++++++++------------
src/bin/pg_dump/pg_dump.h | 4 +-
src/bin/pg_dump/pg_dump_sort.c | 10 ++-
src/bin/pg_dump/t/002_pg_dump.pl | 26 +++++-
4 files changed, 119 insertions(+), 58 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7849770fdee..2253f772fcc 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -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_proc_d.h"
@@ -6122,6 +6123,7 @@ getTypes(Archive *fout)
*/
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]));
@@ -8247,7 +8249,6 @@ addConstrChildIdxDeps(DumpableObject *dobj, const
IndxInfo *refidx)
static void
getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
{
- int i;
ConstraintInfo *constrinfo;
PQExpBuffer query = createPQExpBuffer();
PGresult *res;
@@ -8255,29 +8256,26 @@ getDomainConstraints(Archive *fout, TypeInfo *tyinfo)
i_oid,
i_conname,
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, "
- "contype "
- "FROM
pg_catalog.pg_constraint "
- "WHERE contypid = $1
");
-
/*
- * only PG17 or later versions, not-null domain constraint
catalog
- * information is stored in the pg_constraint.
+ * 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.
*/
- if (fout->remoteVersion < 170000)
- appendPQExpBufferStr(query, "AND contype = 'c' ");
-
- appendPQExpBufferStr(query, "ORDER BY conname");
+ 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);
@@ -8296,34 +8294,49 @@ 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++)
+ for (int i = 0, j = 0; i < ntups; i++)
{
- bool validated = PQgetvalue(res, i, 4)[0] == 't';
+ bool validated = PQgetvalue(res, i,
i_convalidated)[0] == 't';
+ char contype = (PQgetvalue(res, i, i_contype))[0];
+ ConstraintInfo *constraint;
- 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 = *(PQgetvalue(res, i, i_contype));
- 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;
+ 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;
+ }
- constrinfo[i].separate = !validated;
+ 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 = *(PQgetvalue(res, i, i_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
@@ -8332,8 +8345,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);
@@ -12528,29 +12540,32 @@ 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')
{
if (fout->remoteVersion < 170000)
appendPQExpBufferStr(q, " NOT NULL");
else
{
- for (i = 0; i < tyinfo->nDomChecks; i++)
+ ConstraintInfo *notnull = tyinfo->notnull;
+
+ if (!notnull->separate)
{
- ConstraintInfo *domcheck =
&(tyinfo->domChecks[i]);
+ char *default_name;
- if (!domcheck->separate && domcheck->contype ==
'n')
- {
- char *default_name;
+ /* XXX should match ChooseConstraintName better
*/
+ default_name = psprintf("%s_not_null",
qtypname);
- /* 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 (strcmp(default_name,
fmtId(notnull->dobj.name)) == 0)
+ appendPQExpBufferStr(q, " NOT NULL");
+ else
+ appendPQExpBuffer(q, " CONSTRAINT %s
%s",
+
fmtId(notnull->dobj.name), notnull->condef);
}
}
}
@@ -12632,6 +12647,22 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
destroyPQExpBuffer(conprefix);
}
+ /* And a comment on the not-null constraint, if there's one */
+ if (tyinfo->notnull != NULL)
+ {
+ 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);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 39eef1d6617..2370c98d192 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -222,7 +222,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;
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index d3eec6b2598..7b42f4882cc 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -1173,10 +1173,12 @@ repairDependencyLoop(DumpableObject **loop,
}
}
- /* Domain and CHECK, NOT NULL 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 == 'n') &&
((ConstraintInfo *) loop[1])->condomain == (TypeInfo *) loop[0])
{
repairDomainConstraintLoop(loop[0], loop[1]);
@@ -1185,13 +1187,15 @@ repairDependencyLoop(DumpableObject **loop,
if (nLoop == 2 &&
loop[1]->objType == DO_TYPE &&
loop[0]->objType == DO_CONSTRAINT &&
+ (((ConstraintInfo *) loop[1])->contype == 'c' ||
+ ((ConstraintInfo *) loop[1])->contype == 'n') &&
((ConstraintInfo *) loop[0])->condomain == (TypeInfo *) loop[1])
{
repairDomainConstraintLoop(loop[1], loop[0]);
return;
}
- /* Indirect loop involving domain and CHECK OR NOT NULL constraint */
+ /* Indirect loop involving domain and CHECK or NOT NULL constraint */
if (nLoop > 2)
{
for (i = 0; i < nLoop; i++)
@@ -1201,6 +1205,8 @@ repairDependencyLoop(DumpableObject **loop,
for (j = 0; j < nLoop; j++)
{
if (loop[j]->objType == DO_CONSTRAINT &&
+ (((ConstraintInfo *)
loop[1])->contype == 'c' ||
+ ((ConstraintInfo *)
loop[1])->contype == 'n') &&
((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 90d1067f3e2..771cdcecb60 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2390,8 +2390,6 @@ my %tests = (
\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 =>
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1,
},
@@ -2401,6 +2399,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()
--
2.39.5
