public inbox for [email protected]  
help / color / mirror / Atom feed
pg_dump: fix NOT NULL constraint name comparison using makeObjectName
2+ messages / 1 participants
[nested] [flat]

* pg_dump: fix NOT NULL constraint name comparison using makeObjectName
@ 2026-03-19 05:40 JoongHyuk Shin <[email protected]>
  2026-03-19 09:12 ` Re: pg_dump: fix NOT NULL constraint name comparison using makeObjectName JoongHyuk Shin <[email protected]>
  0 siblings, 1 reply; 2+ messages in thread

From: JoongHyuk Shin @ 2026-03-19 05:40 UTC (permalink / raw)
  To: [email protected]

pg_dump decides whether a domain's NOT NULL constraint was auto-generated
by comparing the stored constraint name against a simple
'<domainname>_not_null' string.  This comparison is wrong when the domain
name is longer than 54 bytes, because the server generates the name via
ChooseConstraintName -> makeObjectName, which truncates the domain name to
fit within NAMEDATALEN-1 (63 bytes).  pg_dump skips this truncation, so the
names never match, and it emits an explicit CONSTRAINT clause instead of
plain NOT NULL.

The consequence is a round-trip correctness bug.  When the domain name is
55 bytes, pg_dump emits:

  CONSTRAINT "<55bytes>_not_null" NOT NULL

The restore server receives a 64-byte name that exceeds the 63-byte Name
type limit and silently truncates it to "<55bytes>_not_nul" -- a different
name from the original "<54bytes>_not_null" that makeObjectName would have
produced.  Any DDL that references the original constraint name by string
(ALTER DOMAIN ... DROP CONSTRAINT ...) will then fail on the restored
database.

The same XXX comment and the same structural bug exist in the table column
NOT NULL comparison in getTableAttrs(), where the candidate name is built
as '<tablename>_<colname>_not_null'.

The root cause is that makeObjectName lives in src/backend/commands/
indexcmds.c and is not available to frontend code.  This patch fixes the
problem by:

1. Moving pg_encoding_mbcliplen from src/backend/utils/mb/mbutils.c to
   src/common/wchar.c.  Its dependencies (pg_wchar_table,
   pg_encoding_max_length, and a trivial cliplen helper) are already in
   src/common/wchar.c, so no backend infrastructure is required.

2. Moving makeObjectName from src/backend/commands/indexcmds.c to
   src/common/string.c, adding an 'encoding' parameter to replace the
   implicit GetDatabaseEncoding() call.  src/common/ is the right home for
   this function: it is already a collection of string helpers (strtoint,
   pg_clean_ascii, etc.), it links into both backend and frontend, and
   palloc() is available in frontend via the postgres_fe.h wrapper.

3. Updating all five backend call sites to pass GetDatabaseEncoding().

4. Adding an Archive.server_encoding field and populating it in
   setup_connection() via PQparameterStatus(conn, "server_encoding").
   The server encoding is used for truncation, not the client encoding
   stored in Archive.encoding.

5. Replacing the two XXX psprintf comparisons in pg_dump.c with direct
   calls to makeObjectName(name1, name2, "not_null", fout->server_encoding).

makeObjectName has historically lived in indexcmds.c for no particular
architectural reason -- it has no dependency on index infrastructure.
Moving it to src/common/ is a small cleanup that happens to be necessary
for this fix.

The patch is larger than the two-line conceptual fix, but the bulk of the
size is the mechanical function move (deletion from indexcmds.c, addition
to string.c) and the five backend call-site updates.  An inline
reimplementation in pg_dump was considered but rejected: the truncation
must respect multibyte character boundaries via pg_encoding_mbcliplen, and
duplicating that logic without a reference to the canonical implementation
risks silent divergence in future.

I considered whether the numeric-suffix case (_not_null1, _not_null2, ...)
should also be handled.  When a name collision exists in the same namespace,
ChooseConstraintName appends a pass counter to the label.  pg_dump would
still emit CONSTRAINT for those names.  For a round-trip restore into a
database where the same collision condition holds, the name is stored
verbatim and identity is preserved.  Restoring into a database without that
collision would produce a different constraint name, but that is a
pre-existing limitation shared by any collision-renamed object, not specific
to NOT NULL constraints.  This patch does not address that case.

Reproducer:

  CREATE DOMAIN aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa --
55 'a'
      AS integer NOT NULL;

  -- Server stores constraint name: aaaaaa...(54 a's)_not_null  (63 bytes)
  SELECT c.conname, length(c.conname)
  FROM pg_type t JOIN pg_constraint c ON c.contypid = t.oid
  WHERE t.typname LIKE 'aaa%' AND c.contype = 'n';

Before this patch, pg_dump emits:
  CONSTRAINT "aaaa...(55 a's)_not_null" NOT NULL

After this patch, pg_dump emits:
  NOT NULL

TAP tests are included in src/bin/pg_dump/t/002_pg_dump.pl for both
truncation cases: a 55-char domain name (domain_name + "_not_null" exceeds
NAMEDATALEN-1) and a 27-char table with a 27-char column (table_name + "_"
+ column_name + "_not_null" exceeds NAMEDATALEN-1).  Both cases verify that
pg_dump produces output without an explicit CONSTRAINT clause.

Patch attached.


Attachments:

  [application/octet-stream] 0001-Move-makeObjectName-to-src-common-and-fix-pg_dump-NO.patch (19.2K, 3-0001-Move-makeObjectName-to-src-common-and-fix-pg_dump-NO.patch)
  download | inline diff:
From 45ba378d8f818d9a09d01ac13feaef2fbcf1fbb3 Mon Sep 17 00:00:00 2001
From: JoongHyuk Shin <[email protected]>
Date: Tue, 17 Mar 2026 14:48:49 +0900
Subject: [PATCH] Move makeObjectName to src/common/ and fix pg_dump NOT NULL
 constraint name comparison

pg_dump generates a constraint name using psprintf("%s_not_null", ...)
for domain NOT NULL constraints, and psprintf("%s_%s_not_null", ...)
for column NOT NULL constraints.  PostgreSQL itself uses makeObjectName()
which applies NAMEDATALEN truncation.  When the resulting name exceeds
NAMEDATALEN-1 characters, the generated name differs from the stored
catalog name, so pg_dump incorrectly emits CONSTRAINT <wrong_name> NOT NULL
instead of the inline NOT NULL form.

Fix by moving makeObjectName() from src/backend/commands/indexcmds.c
to src/common/string.c (adding an encoding parameter), so pg_dump can
call it directly with the server encoding obtained via the archive.
Also move pg_encoding_mbcliplen() from src/backend/utils/mb/mbutils.c
to src/common/wchar.c for the same reason.

Both domain NOT NULL and column NOT NULL code paths in pg_dump.c are
updated to use makeObjectName() instead of psprintf().

Add regression tests for the truncation case: a 55-char domain name
(where domain_name + "_not_null" > NAMEDATALEN-1) and a 27-char table
with a 27-char column (where table_name + "_" + col_name + "_not_null"
> NAMEDATALEN-1).
---
 src/backend/catalog/pg_constraint.c |  5 +-
 src/backend/catalog/pg_type.c       |  6 +-
 src/backend/commands/indexcmds.c    | 88 ++--------------------------
 src/backend/commands/statscmds.c    |  5 +-
 src/backend/utils/mb/mbutils.c      | 30 +---------
 src/bin/pg_dump/pg_backup.h         |  2 +
 src/bin/pg_dump/pg_dump.c           | 30 ++++++++--
 src/bin/pg_dump/t/002_pg_dump.pl    | 38 ++++++++++++
 src/common/string.c                 | 91 +++++++++++++++++++++++++++++
 src/common/wchar.c                  | 47 +++++++++++++++
 src/include/commands/defrem.h       |  2 -
 src/include/common/string.h         |  2 +
 12 files changed, 225 insertions(+), 121 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index b12765ae691..8219714ec2e 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -29,6 +29,8 @@
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "common/int.h"
+#include "common/string.h"
+#include "mb/pg_wchar.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
@@ -533,7 +535,8 @@ ChooseConstraintName(const char *name1, const char *name2,
 
 	for (;;)
 	{
-		conname = makeObjectName(name1, name2, modlabel);
+		conname = makeObjectName(name1, name2, modlabel,
+								 GetDatabaseEncoding());
 
 		found = false;
 
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index fc369c35aa6..3d4363f82f8 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -28,6 +28,7 @@
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/typecmds.h"
+#include "common/string.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "utils/acl.h"
@@ -856,7 +857,7 @@ makeArrayTypeName(const char *typeName, Oid typeNamespace)
 	 */
 
 	/* First, try with no numeric suffix */
-	arr_name = makeObjectName("", typeName, NULL);
+	arr_name = makeObjectName("", typeName, NULL, GetDatabaseEncoding());
 
 	for (;;)
 	{
@@ -868,7 +869,8 @@ makeArrayTypeName(const char *typeName, Oid typeNamespace)
 		/* That attempt conflicted.  Prepare a new name with some digits. */
 		pfree(arr_name);
 		snprintf(suffix, sizeof(suffix), "%d", ++pass);
-		arr_name = makeObjectName("", typeName, suffix);
+		arr_name = makeObjectName("", typeName, suffix,
+								  GetDatabaseEncoding());
 	}
 
 	return arr_name;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index cbd76066f74..9a8eca628db 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -44,6 +44,7 @@
 #include "commands/progress.h"
 #include "commands/tablecmds.h"
 #include "commands/tablespace.h"
+#include "common/string.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -2519,88 +2520,10 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
 }
 
 /*
- *	makeObjectName()
- *
- *	Create a name for an implicitly created index, sequence, constraint,
- *	extended statistics, etc.
- *
- *	The parameters are typically: the original table name, the original field
- *	name, and a "type" string (such as "seq" or "pkey").    The field name
- *	and/or type can be NULL if not relevant.
- *
- *	The result is a palloc'd string.
- *
- *	The basic result we want is "name1_name2_label", omitting "_name2" or
- *	"_label" when those parameters are NULL.  However, we must generate
- *	a name with less than NAMEDATALEN characters!  So, we truncate one or
- *	both names if necessary to make a short-enough string.  The label part
- *	is never truncated (so it had better be reasonably short).
- *
- *	The caller is responsible for checking uniqueness of the generated
- *	name and retrying as needed; retrying will be done by altering the
- *	"label" string (which is why we never truncate that part).
+ * makeObjectName() is now in src/common/string.c so that frontend code
+ * can use it too.  Backend callers pass GetDatabaseEncoding() as the
+ * encoding argument.
  */
-char *
-makeObjectName(const char *name1, const char *name2, const char *label)
-{
-	char	   *name;
-	int			overhead = 0;	/* chars needed for label and underscores */
-	int			availchars;		/* chars available for name(s) */
-	int			name1chars;		/* chars allocated to name1 */
-	int			name2chars;		/* chars allocated to name2 */
-	int			ndx;
-
-	name1chars = strlen(name1);
-	if (name2)
-	{
-		name2chars = strlen(name2);
-		overhead++;				/* allow for separating underscore */
-	}
-	else
-		name2chars = 0;
-	if (label)
-		overhead += strlen(label) + 1;
-
-	availchars = NAMEDATALEN - 1 - overhead;
-	Assert(availchars > 0);		/* else caller chose a bad label */
-
-	/*
-	 * If we must truncate, preferentially truncate the longer name. This
-	 * logic could be expressed without a loop, but it's simple and obvious as
-	 * a loop.
-	 */
-	while (name1chars + name2chars > availchars)
-	{
-		if (name1chars > name2chars)
-			name1chars--;
-		else
-			name2chars--;
-	}
-
-	name1chars = pg_mbcliplen(name1, name1chars, name1chars);
-	if (name2)
-		name2chars = pg_mbcliplen(name2, name2chars, name2chars);
-
-	/* Now construct the string using the chosen lengths */
-	name = palloc(name1chars + name2chars + overhead + 1);
-	memcpy(name, name1, name1chars);
-	ndx = name1chars;
-	if (name2)
-	{
-		name[ndx++] = '_';
-		memcpy(name + ndx, name2, name2chars);
-		ndx += name2chars;
-	}
-	if (label)
-	{
-		name[ndx++] = '_';
-		strcpy(name + ndx, label);
-	}
-	else
-		name[ndx] = '\0';
-
-	return name;
-}
 
 /*
  * Select a nonconflicting name for a new relation.  This is ordinarily
@@ -2652,7 +2575,8 @@ ChooseRelationName(const char *name1, const char *name2,
 		SysScanDesc scan;
 		bool		collides;
 
-		relname = makeObjectName(name1, name2, modlabel);
+		relname = makeObjectName(name1, name2, modlabel,
+								 GetDatabaseEncoding());
 
 		/* is there any conflicting relation name? */
 		ScanKeyInit(&key[0],
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index c1da79f36ba..201e08d1654 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -27,6 +27,8 @@
 #include "catalog/pg_statistic_ext_data.h"
 #include "commands/comment.h"
 #include "commands/defrem.h"
+#include "common/string.h"
+#include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
@@ -860,7 +862,8 @@ ChooseExtendedStatisticName(const char *name1, const char *name2,
 	{
 		Oid			existingstats;
 
-		stxname = makeObjectName(name1, name2, modlabel);
+		stxname = makeObjectName(name1, name2, modlabel,
+								 GetDatabaseEncoding());
 
 		existingstats = GetSysCacheOid2(STATEXTNAMENSP, Anum_pg_statistic_ext_oid,
 										PointerGetDatum(stxname),
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 78f4d5e202c..fdd2e025a4e 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1215,35 +1215,9 @@ pg_mbcliplen(const char *mbstr, int len, int limit)
 }
 
 /*
- * pg_mbcliplen with specified encoding; string must be valid in encoding
+ * pg_encoding_mbcliplen is now in src/common/wchar.c so that frontend
+ * code can use it too.
  */
-int
-pg_encoding_mbcliplen(int encoding, const char *mbstr,
-					  int len, int limit)
-{
-	mblen_converter mblen_fn;
-	int			clen = 0;
-	int			l;
-
-	/* optimization for single byte encoding */
-	if (pg_encoding_max_length(encoding) == 1)
-		return cliplen(mbstr, len, limit);
-
-	mblen_fn = pg_wchar_table[encoding].mblen;
-
-	while (len > 0 && *mbstr)
-	{
-		l = (*mblen_fn) ((const unsigned char *) mbstr);
-		if ((clen + l) > limit)
-			break;
-		clen += l;
-		if (clen == limit)
-			break;
-		len -= l;
-		mbstr += l;
-	}
-	return clen;
-}
 
 /*
  * Similar to pg_mbcliplen except the limit parameter specifies the
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index fda912ba0a9..11584840d7d 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -242,6 +242,8 @@ typedef struct Archive
 
 	/* info needed for string escaping */
 	int			encoding;		/* libpq code for client_encoding */
+	int			server_encoding;	/* libpq code for server encoding; distinct
+									 * from encoding (client encoding) */
 	bool		std_strings;	/* standard_conforming_strings */
 
 	/* other important stuff */
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b41a3ae3db4..f1c37b65cc3 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -60,6 +60,8 @@
 #include "common/int.h"
 #include "common/relpath.h"
 #include "common/shortest_dec.h"
+#include "common/string.h"
+#include "mb/pg_wchar.h"
 #include "compress_io.h"
 #include "dumputils.h"
 #include "fe_utils/option_utils.h"
@@ -1436,6 +1438,20 @@ setup_connection(Archive *AH, const char *dumpencoding,
 	AH->encoding = PQclientEncoding(conn);
 	setFmtEncoding(AH->encoding);
 
+	/*
+	 * Get the server encoding so we can replicate the server's object name
+	 * truncation logic (which uses server encoding, not client encoding).
+	 */
+	{
+		const char *senc = PQparameterStatus(conn, "server_encoding");
+
+		if (senc == NULL)
+			pg_fatal("could not get server encoding");
+		AH->server_encoding = pg_char_to_encoding(senc);
+		if (AH->server_encoding < 0)
+			pg_fatal("unrecognized server encoding \"%s\"", senc);
+	}
+
 	/*
 	 * Set the role if requested.  In a parallel dump worker, we'll be passed
 	 * use_role == NULL, but AH->use_role is already set (if user specified it
@@ -10208,9 +10224,11 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
 			{
 				char	   *default_name;
 
-				/* XXX should match ChooseConstraintName better */
-				default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
-										tbinfo->attnames[j]);
+				/* Use makeObjectName to match ChooseConstraintName's truncation */
+				default_name = makeObjectName(tbinfo->dobj.name,
+											  tbinfo->attnames[j],
+											  "not_null",
+											  fout->server_encoding);
 				if (strcmp(default_name,
 						   PQgetvalue(res, r, i_notnull_name)) == 0)
 					tbinfo->notnull_constrs[j] = "";
@@ -12903,8 +12921,10 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
 			{
 				char	   *default_name;
 
-				/* XXX should match ChooseConstraintName better */
-				default_name = psprintf("%s_not_null", tyinfo->dobj.name);
+				/* Use makeObjectName to match ChooseConstraintName's truncation */
+				default_name = makeObjectName(tyinfo->dobj.name, NULL,
+											  "not_null",
+											  fout->server_encoding);
 
 				if (strcmp(default_name, notnull->dobj.name) == 0)
 					appendPQExpBufferStr(q, " NOT NULL");
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 051a3d8ea3d..8d0b0ed3df6 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2293,6 +2293,44 @@ my %tests = (
 		},
 	},
 
+	# Test that pg_dump uses makeObjectName truncation logic when deciding
+	# whether to emit CONSTRAINT for an auto-generated NOT NULL constraint name.
+	# A 55-char domain name causes makeObjectName to truncate to 54 chars
+	# before appending "_not_null", so pg_dump must apply the same truncation.
+	'CREATE DOMAIN dump_test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' => {
+		create_order => 101,
+		create_sql =>
+		  'CREATE DOMAIN dump_test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa AS integer NOT NULL;',
+		regexp => qr/^
+			\QCREATE DOMAIN dump_test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa AS integer 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,
+		},
+	},
+
+	# A 27-char table name + 27-char column name (total 54 > 53) causes
+	# makeObjectName to truncate, so pg_dump must apply the same truncation
+	# when deciding whether to emit CONSTRAINT for a column NOT NULL.
+	'CREATE TABLE dump_test.ttttttttttttttttttttttttttt' => {
+		create_order => 102,
+		create_sql =>
+		  'CREATE TABLE dump_test.ttttttttttttttttttttttttttt (ttttttttttttttttttttttttttt integer NOT NULL);',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.ttttttttttttttttttttttttttt (\E\n
+			\s+\Qttttttttttttttttttttttttttt integer 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,
+		},
+	},
+
 	'CREATE FUNCTION dump_test.pltestlang_call_handler' => {
 		create_order => 17,
 		create_sql => 'CREATE FUNCTION dump_test.pltestlang_call_handler()
diff --git a/src/common/string.c b/src/common/string.c
index 41c74a1502d..dd11fcd4a6e 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -22,6 +22,97 @@
 #endif
 
 #include "common/string.h"
+#include "mb/pg_wchar.h"
+
+
+/*
+ * makeObjectName()
+ *
+ *	Create a name for an implicitly created index, sequence, constraint,
+ *	extended statistics, etc.
+ *
+ *	The parameters are typically: the original table name, the original field
+ *	name, and a "type" string (such as "seq" or "pkey").    The field name
+ *	and/or type can be NULL if not relevant.
+ *
+ *	The result is a palloc'd string (pg_malloc'd in frontend code).
+ *
+ *	The basic result we want is "name1_name2_label", omitting "_name2" or
+ *	"_label" when those parameters are NULL.  However, we must generate
+ *	a name with less than NAMEDATALEN characters!  So, we truncate one or
+ *	both names if necessary to make a short-enough string.  The label part
+ *	is never truncated (so it had better be reasonably short).
+ *
+ *	The caller is responsible for checking uniqueness of the generated
+ *	name and retrying as needed; retrying will be done by altering the
+ *	"label" string (which is why we never truncate that part).
+ *
+ *	The encoding parameter is used to avoid splitting multibyte characters
+ *	when truncating.  Backend callers should pass GetDatabaseEncoding();
+ *	frontend callers should pass the server encoding.
+ */
+char *
+makeObjectName(const char *name1, const char *name2, const char *label,
+			   int encoding)
+{
+	char	   *name;
+	int			overhead = 0;	/* chars needed for label and underscores */
+	int			availchars;		/* chars available for name(s) */
+	int			name1chars;		/* chars allocated to name1 */
+	int			name2chars;		/* chars allocated to name2 */
+	int			ndx;
+
+	name1chars = strlen(name1);
+	if (name2)
+	{
+		name2chars = strlen(name2);
+		overhead++;				/* allow for separating underscore */
+	}
+	else
+		name2chars = 0;
+	if (label)
+		overhead += strlen(label) + 1;
+
+	availchars = NAMEDATALEN - 1 - overhead;
+	Assert(availchars > 0);		/* else caller chose a bad label */
+
+	/*
+	 * If we must truncate, preferentially truncate the longer name. This
+	 * logic could be expressed without a loop, but it's simple and obvious as
+	 * a loop.
+	 */
+	while (name1chars + name2chars > availchars)
+	{
+		if (name1chars > name2chars)
+			name1chars--;
+		else
+			name2chars--;
+	}
+
+	name1chars = pg_encoding_mbcliplen(encoding, name1, name1chars, name1chars);
+	if (name2)
+		name2chars = pg_encoding_mbcliplen(encoding, name2, name2chars, name2chars);
+
+	/* Now construct the string using the chosen lengths */
+	name = palloc(name1chars + name2chars + overhead + 1);
+	memcpy(name, name1, name1chars);
+	ndx = name1chars;
+	if (name2)
+	{
+		name[ndx++] = '_';
+		memcpy(name + ndx, name2, name2chars);
+		ndx += name2chars;
+	}
+	if (label)
+	{
+		name[ndx++] = '_';
+		strcpy(name + ndx, label);
+	}
+	else
+		name[ndx] = '\0';
+
+	return name;
+}
 
 
 /*
diff --git a/src/common/wchar.c b/src/common/wchar.c
index e7b6595b042..39e2ab815f1 100644
--- a/src/common/wchar.c
+++ b/src/common/wchar.c
@@ -2244,3 +2244,50 @@ pg_encoding_max_length(int encoding)
 		pg_wchar_table[encoding].maxmblen :
 		pg_wchar_table[PG_SQL_ASCII].maxmblen;
 }
+
+/* cliplen for any single-byte encoding */
+static int
+cliplen(const char *str, int len, int limit)
+{
+	int			l = 0;
+
+	len = Min(len, limit);
+	while (l < len && str[l])
+		l++;
+	return l;
+}
+
+/*
+ * pg_encoding_mbcliplen -- return the byte length of a multibyte string
+ * (not necessarily NULL terminated) that is no longer than limit bytes.
+ * This function does not break multibyte character boundaries.
+ *
+ * The string must be valid in the specified encoding.
+ */
+int
+pg_encoding_mbcliplen(int encoding, const char *mbstr,
+					  int len, int limit)
+{
+	mblen_converter mblen_fn;
+	int			clen = 0;
+	int			l;
+
+	/* optimization for single byte encoding */
+	if (pg_encoding_max_length(encoding) == 1)
+		return cliplen(mbstr, len, limit);
+
+	mblen_fn = pg_wchar_table[encoding].mblen;
+
+	while (len > 0 && *mbstr)
+	{
+		l = (*mblen_fn) ((const unsigned char *) mbstr);
+		if ((clen + l) > limit)
+			break;
+		clen += l;
+		if (clen == limit)
+			break;
+		len -= l;
+		mbstr += l;
+	}
+	return clen;
+}
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 8f4a2d9bbc1..554c01504ec 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -38,8 +38,6 @@ extern ObjectAddress DefineIndex(ParseState *pstate,
 								 bool skip_build,
 								 bool quiet);
 extern void ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel);
-extern char *makeObjectName(const char *name1, const char *name2,
-							const char *label);
 extern char *ChooseRelationName(const char *name1, const char *name2,
 								const char *label, Oid namespaceid,
 								bool isconstraint);
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 2a7c31ea74e..07cc230488b 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -24,6 +24,8 @@ typedef struct PromptInterruptContext
 } PromptInterruptContext;
 
 /* functions in src/common/string.c */
+extern char *makeObjectName(const char *name1, const char *name2,
+							const char *label, int encoding);
 extern bool pg_str_endswith(const char *str, const char *end);
 extern int	strtoint(const char *pg_restrict str, char **pg_restrict endptr,
 					 int base);
-- 
2.52.0



^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: pg_dump: fix NOT NULL constraint name comparison using makeObjectName
  2026-03-19 05:40 pg_dump: fix NOT NULL constraint name comparison using makeObjectName JoongHyuk Shin <[email protected]>
@ 2026-03-19 09:12 ` JoongHyuk Shin <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: JoongHyuk Shin @ 2026-03-19 09:12 UTC (permalink / raw)
  To: [email protected]

v1 failed the meson/Cirrus CI build: libpgtypes.so reported an undefined
reference to palloc.

The cause: v1 placed makeObjectName in string.c.  libpgtypes links against
libpgcommon_shlib, which excludes fe_memutils.c (the frontend palloc
provider).  libpgtypes uses strtoint from string.c, so the linker pulls in
string.c.o -- and with it, the palloc call inside makeObjectName.

v2 moves makeObjectName to its own file, src/common/objectname.c.  Since
nothing in libpgtypes calls makeObjectName, the linker never pulls in
objectname.c.o from the archive, and the palloc symbol stays unresolved
only in an unused object file.

No functional change from v1 otherwise.  The autoconf build and all tests
(regression + pg_dump TAP) pass.

Patch attached.

On Thu, Mar 19, 2026 at 2:40 PM JoongHyuk Shin <[email protected]> wrote:

> pg_dump decides whether a domain's NOT NULL constraint was auto-generated
> by comparing the stored constraint name against a simple
> '<domainname>_not_null' string.  This comparison is wrong when the domain
> name is longer than 54 bytes, because the server generates the name via
> ChooseConstraintName -> makeObjectName, which truncates the domain name to
> fit within NAMEDATALEN-1 (63 bytes).  pg_dump skips this truncation, so the
> names never match, and it emits an explicit CONSTRAINT clause instead of
> plain NOT NULL.
>
> The consequence is a round-trip correctness bug.  When the domain name is
> 55 bytes, pg_dump emits:
>
>   CONSTRAINT "<55bytes>_not_null" NOT NULL
>
> The restore server receives a 64-byte name that exceeds the 63-byte Name
> type limit and silently truncates it to "<55bytes>_not_nul" -- a different
> name from the original "<54bytes>_not_null" that makeObjectName would have
> produced.  Any DDL that references the original constraint name by string
> (ALTER DOMAIN ... DROP CONSTRAINT ...) will then fail on the restored
> database.
>
> The same XXX comment and the same structural bug exist in the table column
> NOT NULL comparison in getTableAttrs(), where the candidate name is built
> as '<tablename>_<colname>_not_null'.
>
> The root cause is that makeObjectName lives in src/backend/commands/
> indexcmds.c and is not available to frontend code.  This patch fixes the
> problem by:
>
> 1. Moving pg_encoding_mbcliplen from src/backend/utils/mb/mbutils.c to
>    src/common/wchar.c.  Its dependencies (pg_wchar_table,
>    pg_encoding_max_length, and a trivial cliplen helper) are already in
>    src/common/wchar.c, so no backend infrastructure is required.
>
> 2. Moving makeObjectName from src/backend/commands/indexcmds.c to
>    src/common/string.c, adding an 'encoding' parameter to replace the
>    implicit GetDatabaseEncoding() call.  src/common/ is the right home for
>    this function: it is already a collection of string helpers (strtoint,
>    pg_clean_ascii, etc.), it links into both backend and frontend, and
>    palloc() is available in frontend via the postgres_fe.h wrapper.
>
> 3. Updating all five backend call sites to pass GetDatabaseEncoding().
>
> 4. Adding an Archive.server_encoding field and populating it in
>    setup_connection() via PQparameterStatus(conn, "server_encoding").
>    The server encoding is used for truncation, not the client encoding
>    stored in Archive.encoding.
>
> 5. Replacing the two XXX psprintf comparisons in pg_dump.c with direct
>    calls to makeObjectName(name1, name2, "not_null",
> fout->server_encoding).
>
> makeObjectName has historically lived in indexcmds.c for no particular
> architectural reason -- it has no dependency on index infrastructure.
> Moving it to src/common/ is a small cleanup that happens to be necessary
> for this fix.
>
> The patch is larger than the two-line conceptual fix, but the bulk of the
> size is the mechanical function move (deletion from indexcmds.c, addition
> to string.c) and the five backend call-site updates.  An inline
> reimplementation in pg_dump was considered but rejected: the truncation
> must respect multibyte character boundaries via pg_encoding_mbcliplen, and
> duplicating that logic without a reference to the canonical implementation
> risks silent divergence in future.
>
> I considered whether the numeric-suffix case (_not_null1, _not_null2, ...)
> should also be handled.  When a name collision exists in the same
> namespace,
> ChooseConstraintName appends a pass counter to the label.  pg_dump would
> still emit CONSTRAINT for those names.  For a round-trip restore into a
> database where the same collision condition holds, the name is stored
> verbatim and identity is preserved.  Restoring into a database without that
> collision would produce a different constraint name, but that is a
> pre-existing limitation shared by any collision-renamed object, not
> specific
> to NOT NULL constraints.  This patch does not address that case.
>
> Reproducer:
>
>   CREATE DOMAIN aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa --
> 55 'a'
>       AS integer NOT NULL;
>
>   -- Server stores constraint name: aaaaaa...(54 a's)_not_null  (63 bytes)
>   SELECT c.conname, length(c.conname)
>   FROM pg_type t JOIN pg_constraint c ON c.contypid = t.oid
>   WHERE t.typname LIKE 'aaa%' AND c.contype = 'n';
>
> Before this patch, pg_dump emits:
>   CONSTRAINT "aaaa...(55 a's)_not_null" NOT NULL
>
> After this patch, pg_dump emits:
>   NOT NULL
>
> TAP tests are included in src/bin/pg_dump/t/002_pg_dump.pl for both
> truncation cases: a 55-char domain name (domain_name + "_not_null" exceeds
> NAMEDATALEN-1) and a 27-char table with a 27-char column (table_name + "_"
> + column_name + "_not_null" exceeds NAMEDATALEN-1).  Both cases verify that
> pg_dump produces output without an explicit CONSTRAINT clause.
>
> Patch attached.
>


Attachments:

  [application/octet-stream] 0001-Move-makeObjectName-to-src-common-and-fix-pg_dump-NO.patch (20.4K, 3-0001-Move-makeObjectName-to-src-common-and-fix-pg_dump-NO.patch)
  download | inline diff:
From fc680ddf2d11ffc211cd5f3c3521564da16592f2 Mon Sep 17 00:00:00 2001
From: JoongHyuk Shin <[email protected]>
Date: Thu, 19 Mar 2026 17:10:52 +0900
Subject: [PATCH] Move makeObjectName to src/common/ and fix pg_dump NOT NULL
 constraint name comparison

pg_dump compared NOT NULL constraint names using a simple
psprintf("%s_not_null", ...) format, which did not account for
NAMEDATALEN truncation.  When a table or column name was long enough
to trigger truncation, the generated name differed from what
ChooseConstraintName() produces via makeObjectName(), causing pg_dump
to emit a redundant CONSTRAINT clause.

Fix by making makeObjectName() available to frontend code.  Move it
from src/backend/commands/indexcmds.c to src/common/objectname.c,
adding an encoding parameter so callers can specify the encoding for
multibyte-aware truncation.  Also move pg_encoding_mbcliplen() to
src/common/wchar.c for the same reason.

Backend callers pass GetDatabaseEncoding(); pg_dump passes the
server encoding obtained via PQparameterStatus().
---
 src/backend/catalog/pg_constraint.c |   5 +-
 src/backend/catalog/pg_type.c       |   6 +-
 src/backend/commands/indexcmds.c    |  88 +---------------------
 src/backend/commands/statscmds.c    |   5 +-
 src/backend/utils/mb/mbutils.c      |  31 --------
 src/bin/pg_dump/pg_backup.h         |   3 +
 src/bin/pg_dump/pg_dump.c           |  36 +++++++--
 src/bin/pg_dump/t/002_pg_dump.pl    |  38 ++++++++++
 src/common/Makefile                 |   1 +
 src/common/meson.build              |   1 +
 src/common/objectname.c             | 112 ++++++++++++++++++++++++++++
 src/common/wchar.c                  |  53 +++++++++++++
 src/include/commands/defrem.h       |   2 -
 src/include/common/string.h         |   4 +
 14 files changed, 258 insertions(+), 127 deletions(-)
 create mode 100644 src/common/objectname.c

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index b12765ae691..8219714ec2e 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -29,6 +29,8 @@
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "common/int.h"
+#include "common/string.h"
+#include "mb/pg_wchar.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
@@ -533,7 +535,8 @@ ChooseConstraintName(const char *name1, const char *name2,
 
 	for (;;)
 	{
-		conname = makeObjectName(name1, name2, modlabel);
+		conname = makeObjectName(name1, name2, modlabel,
+								 GetDatabaseEncoding());
 
 		found = false;
 
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index fc369c35aa6..3d4363f82f8 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -28,6 +28,7 @@
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/typecmds.h"
+#include "common/string.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "utils/acl.h"
@@ -856,7 +857,7 @@ makeArrayTypeName(const char *typeName, Oid typeNamespace)
 	 */
 
 	/* First, try with no numeric suffix */
-	arr_name = makeObjectName("", typeName, NULL);
+	arr_name = makeObjectName("", typeName, NULL, GetDatabaseEncoding());
 
 	for (;;)
 	{
@@ -868,7 +869,8 @@ makeArrayTypeName(const char *typeName, Oid typeNamespace)
 		/* That attempt conflicted.  Prepare a new name with some digits. */
 		pfree(arr_name);
 		snprintf(suffix, sizeof(suffix), "%d", ++pass);
-		arr_name = makeObjectName("", typeName, suffix);
+		arr_name = makeObjectName("", typeName, suffix,
+								  GetDatabaseEncoding());
 	}
 
 	return arr_name;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index cbd76066f74..5b5daa37e6b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -44,6 +44,7 @@
 #include "commands/progress.h"
 #include "commands/tablecmds.h"
 #include "commands/tablespace.h"
+#include "common/string.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -2518,90 +2519,6 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
 						  get_opfamily_name(opfamily, false), get_am_name(amid)));
 }
 
-/*
- *	makeObjectName()
- *
- *	Create a name for an implicitly created index, sequence, constraint,
- *	extended statistics, etc.
- *
- *	The parameters are typically: the original table name, the original field
- *	name, and a "type" string (such as "seq" or "pkey").    The field name
- *	and/or type can be NULL if not relevant.
- *
- *	The result is a palloc'd string.
- *
- *	The basic result we want is "name1_name2_label", omitting "_name2" or
- *	"_label" when those parameters are NULL.  However, we must generate
- *	a name with less than NAMEDATALEN characters!  So, we truncate one or
- *	both names if necessary to make a short-enough string.  The label part
- *	is never truncated (so it had better be reasonably short).
- *
- *	The caller is responsible for checking uniqueness of the generated
- *	name and retrying as needed; retrying will be done by altering the
- *	"label" string (which is why we never truncate that part).
- */
-char *
-makeObjectName(const char *name1, const char *name2, const char *label)
-{
-	char	   *name;
-	int			overhead = 0;	/* chars needed for label and underscores */
-	int			availchars;		/* chars available for name(s) */
-	int			name1chars;		/* chars allocated to name1 */
-	int			name2chars;		/* chars allocated to name2 */
-	int			ndx;
-
-	name1chars = strlen(name1);
-	if (name2)
-	{
-		name2chars = strlen(name2);
-		overhead++;				/* allow for separating underscore */
-	}
-	else
-		name2chars = 0;
-	if (label)
-		overhead += strlen(label) + 1;
-
-	availchars = NAMEDATALEN - 1 - overhead;
-	Assert(availchars > 0);		/* else caller chose a bad label */
-
-	/*
-	 * If we must truncate, preferentially truncate the longer name. This
-	 * logic could be expressed without a loop, but it's simple and obvious as
-	 * a loop.
-	 */
-	while (name1chars + name2chars > availchars)
-	{
-		if (name1chars > name2chars)
-			name1chars--;
-		else
-			name2chars--;
-	}
-
-	name1chars = pg_mbcliplen(name1, name1chars, name1chars);
-	if (name2)
-		name2chars = pg_mbcliplen(name2, name2chars, name2chars);
-
-	/* Now construct the string using the chosen lengths */
-	name = palloc(name1chars + name2chars + overhead + 1);
-	memcpy(name, name1, name1chars);
-	ndx = name1chars;
-	if (name2)
-	{
-		name[ndx++] = '_';
-		memcpy(name + ndx, name2, name2chars);
-		ndx += name2chars;
-	}
-	if (label)
-	{
-		name[ndx++] = '_';
-		strcpy(name + ndx, label);
-	}
-	else
-		name[ndx] = '\0';
-
-	return name;
-}
-
 /*
  * Select a nonconflicting name for a new relation.  This is ordinarily
  * used to choose index names (which is why it's here) but it can also
@@ -2652,7 +2569,8 @@ ChooseRelationName(const char *name1, const char *name2,
 		SysScanDesc scan;
 		bool		collides;
 
-		relname = makeObjectName(name1, name2, modlabel);
+		relname = makeObjectName(name1, name2, modlabel,
+								 GetDatabaseEncoding());
 
 		/* is there any conflicting relation name? */
 		ScanKeyInit(&key[0],
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index c1da79f36ba..201e08d1654 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -27,6 +27,8 @@
 #include "catalog/pg_statistic_ext_data.h"
 #include "commands/comment.h"
 #include "commands/defrem.h"
+#include "common/string.h"
+#include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
@@ -860,7 +862,8 @@ ChooseExtendedStatisticName(const char *name1, const char *name2,
 	{
 		Oid			existingstats;
 
-		stxname = makeObjectName(name1, name2, modlabel);
+		stxname = makeObjectName(name1, name2, modlabel,
+								 GetDatabaseEncoding());
 
 		existingstats = GetSysCacheOid2(STATEXTNAMENSP, Anum_pg_statistic_ext_oid,
 										PointerGetDatum(stxname),
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 78f4d5e202c..116b150bbf8 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1214,37 +1214,6 @@ pg_mbcliplen(const char *mbstr, int len, int limit)
 								 len, limit);
 }
 
-/*
- * pg_mbcliplen with specified encoding; string must be valid in encoding
- */
-int
-pg_encoding_mbcliplen(int encoding, const char *mbstr,
-					  int len, int limit)
-{
-	mblen_converter mblen_fn;
-	int			clen = 0;
-	int			l;
-
-	/* optimization for single byte encoding */
-	if (pg_encoding_max_length(encoding) == 1)
-		return cliplen(mbstr, len, limit);
-
-	mblen_fn = pg_wchar_table[encoding].mblen;
-
-	while (len > 0 && *mbstr)
-	{
-		l = (*mblen_fn) ((const unsigned char *) mbstr);
-		if ((clen + l) > limit)
-			break;
-		clen += l;
-		if (clen == limit)
-			break;
-		len -= l;
-		mbstr += l;
-	}
-	return clen;
-}
-
 /*
  * Similar to pg_mbcliplen except the limit parameter specifies the
  * character length, not the byte length.
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index fda912ba0a9..4d1e4635d5f 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -242,6 +242,9 @@ typedef struct Archive
 
 	/* info needed for string escaping */
 	int			encoding;		/* libpq code for client_encoding */
+	int			server_encoding;	/* libpq code for server encoding;
+									 * distinct from encoding (client
+									 * encoding) */
 	bool		std_strings;	/* standard_conforming_strings */
 
 	/* other important stuff */
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b41a3ae3db4..b344d2e1437 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -60,6 +60,8 @@
 #include "common/int.h"
 #include "common/relpath.h"
 #include "common/shortest_dec.h"
+#include "common/string.h"
+#include "mb/pg_wchar.h"
 #include "compress_io.h"
 #include "dumputils.h"
 #include "fe_utils/option_utils.h"
@@ -1436,6 +1438,20 @@ setup_connection(Archive *AH, const char *dumpencoding,
 	AH->encoding = PQclientEncoding(conn);
 	setFmtEncoding(AH->encoding);
 
+	/*
+	 * Get the server encoding so we can replicate the server's object name
+	 * truncation logic (which uses server encoding, not client encoding).
+	 */
+	{
+		const char *senc = PQparameterStatus(conn, "server_encoding");
+
+		if (senc == NULL)
+			pg_fatal("could not get server encoding");
+		AH->server_encoding = pg_char_to_encoding(senc);
+		if (AH->server_encoding < 0)
+			pg_fatal("unrecognized server encoding \"%s\"", senc);
+	}
+
 	/*
 	 * Set the role if requested.  In a parallel dump worker, we'll be passed
 	 * use_role == NULL, but AH->use_role is already set (if user specified it
@@ -10208,9 +10224,14 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
 			{
 				char	   *default_name;
 
-				/* XXX should match ChooseConstraintName better */
-				default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
-										tbinfo->attnames[j]);
+				/*
+				 * Use makeObjectName to match ChooseConstraintName's
+				 * truncation
+				 */
+				default_name = makeObjectName(tbinfo->dobj.name,
+											  tbinfo->attnames[j],
+											  "not_null",
+											  fout->server_encoding);
 				if (strcmp(default_name,
 						   PQgetvalue(res, r, i_notnull_name)) == 0)
 					tbinfo->notnull_constrs[j] = "";
@@ -12903,8 +12924,13 @@ dumpDomain(Archive *fout, const TypeInfo *tyinfo)
 			{
 				char	   *default_name;
 
-				/* XXX should match ChooseConstraintName better */
-				default_name = psprintf("%s_not_null", tyinfo->dobj.name);
+				/*
+				 * Use makeObjectName to match ChooseConstraintName's
+				 * truncation
+				 */
+				default_name = makeObjectName(tyinfo->dobj.name, NULL,
+											  "not_null",
+											  fout->server_encoding);
 
 				if (strcmp(default_name, notnull->dobj.name) == 0)
 					appendPQExpBufferStr(q, " NOT NULL");
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 051a3d8ea3d..8d0b0ed3df6 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2293,6 +2293,44 @@ my %tests = (
 		},
 	},
 
+	# Test that pg_dump uses makeObjectName truncation logic when deciding
+	# whether to emit CONSTRAINT for an auto-generated NOT NULL constraint name.
+	# A 55-char domain name causes makeObjectName to truncate to 54 chars
+	# before appending "_not_null", so pg_dump must apply the same truncation.
+	'CREATE DOMAIN dump_test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' => {
+		create_order => 101,
+		create_sql =>
+		  'CREATE DOMAIN dump_test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa AS integer NOT NULL;',
+		regexp => qr/^
+			\QCREATE DOMAIN dump_test.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa AS integer 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,
+		},
+	},
+
+	# A 27-char table name + 27-char column name (total 54 > 53) causes
+	# makeObjectName to truncate, so pg_dump must apply the same truncation
+	# when deciding whether to emit CONSTRAINT for a column NOT NULL.
+	'CREATE TABLE dump_test.ttttttttttttttttttttttttttt' => {
+		create_order => 102,
+		create_sql =>
+		  'CREATE TABLE dump_test.ttttttttttttttttttttttttttt (ttttttttttttttttttttttttttt integer NOT NULL);',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.ttttttttttttttttttttttttttt (\E\n
+			\s+\Qttttttttttttttttttttttttttt integer 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,
+		},
+	},
+
 	'CREATE FUNCTION dump_test.pltestlang_call_handler' => {
 		create_order => 17,
 		create_sql => 'CREATE FUNCTION dump_test.pltestlang_call_handler()
diff --git a/src/common/Makefile b/src/common/Makefile
index 2c720caa509..15b48b72016 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -65,6 +65,7 @@ OBJS_COMMON = \
 	kwlookup.o \
 	link-canary.o \
 	md5_common.o \
+	objectname.o \
 	parse_manifest.o \
 	percentrepl.o \
 	pg_get_line.o \
diff --git a/src/common/meson.build b/src/common/meson.build
index 4f9b8b8263d..81064a8b476 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -19,6 +19,7 @@ common_sources = files(
   'kwlookup.c',
   'link-canary.c',
   'md5_common.c',
+  'objectname.c',
   'parse_manifest.c',
   'percentrepl.c',
   'pg_get_line.c',
diff --git a/src/common/objectname.c b/src/common/objectname.c
new file mode 100644
index 00000000000..f6ecde13457
--- /dev/null
+++ b/src/common/objectname.c
@@ -0,0 +1,112 @@
+/*-------------------------------------------------------------------------
+ *
+ * objectname.c
+ *		helper for generating names of implicitly created objects
+ *
+ * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *	  src/common/objectname.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#ifndef FRONTEND
+#include "postgres.h"
+#else
+#include "postgres_fe.h"
+#endif
+
+#include "common/string.h"
+#include "mb/pg_wchar.h"
+
+/*
+ * makeObjectName()
+ *
+ *	Create a name for an implicitly created index, sequence, constraint,
+ *	extended statistics, etc.
+ *
+ *	The parameters are typically: the original table name, the original field
+ *	name, and a "type" string (such as "seq" or "pkey").  The field name
+ *	and/or type can be NULL if not relevant.
+ *
+ *	The result is a palloc'd string (pg_malloc'd in frontend code).
+ *
+ *	The basic result we want is "name1_name2_label", omitting "_name2" or
+ *	"_label" when those parameters are NULL.  However, we must generate
+ *	a name with less than NAMEDATALEN characters!  So, we truncate one or
+ *	both names if necessary to make a short-enough string.  The label part
+ *	is never truncated (so it had better be reasonably short).
+ *
+ *	The caller is responsible for checking uniqueness of the generated
+ *	name and retrying as needed; retrying will be done by altering the
+ *	"label" string (which is why we never truncate that part).
+ *
+ *	The encoding parameter is used to avoid splitting multibyte characters
+ *	when truncating.  Backend callers should pass GetDatabaseEncoding();
+ *	frontend callers should pass the server encoding.
+ */
+char *
+makeObjectName(const char *name1, const char *name2, const char *label,
+			   int encoding)
+{
+	char	   *name;
+	int			overhead = 0;	/* chars needed for label and underscores */
+	int			availchars;		/* chars available for name(s) */
+	int			name1chars;		/* chars allocated to name1 */
+	int			name2chars;		/* chars allocated to name2 */
+	int			ndx;
+
+	name1chars = strlen(name1);
+	if (name2)
+	{
+		name2chars = strlen(name2);
+		overhead++;				/* allow for separating underscore */
+	}
+	else
+		name2chars = 0;
+	if (label)
+		overhead += strlen(label) + 1;
+
+	availchars = NAMEDATALEN - 1 - overhead;
+	Assert(availchars > 0);		/* else caller chose a bad label */
+
+	/*
+	 * If we must truncate, preferentially truncate the longer name. This
+	 * logic could be expressed without a loop, but it's simple and obvious as
+	 * a loop.
+	 */
+	while (name1chars + name2chars > availchars)
+	{
+		if (name1chars > name2chars)
+			name1chars--;
+		else
+			name2chars--;
+	}
+
+	name1chars = pg_encoding_mbcliplen(encoding, name1, name1chars, name1chars);
+	if (name2)
+		name2chars = pg_encoding_mbcliplen(encoding, name2, name2chars, name2chars);
+
+	/* Now construct the string using the chosen lengths */
+	name = palloc(name1chars + name2chars + overhead + 1);
+	memcpy(name, name1, name1chars);
+	ndx = name1chars;
+	if (name2)
+	{
+		name[ndx++] = '_';
+		memcpy(name + ndx, name2, name2chars);
+		ndx += name2chars;
+	}
+	if (label)
+	{
+		name[ndx++] = '_';
+		strcpy(name + ndx, label);
+	}
+	else
+		name[ndx] = '\0';
+
+	return name;
+}
diff --git a/src/common/wchar.c b/src/common/wchar.c
index e7b6595b042..ef683e789d7 100644
--- a/src/common/wchar.c
+++ b/src/common/wchar.c
@@ -2244,3 +2244,56 @@ pg_encoding_max_length(int encoding)
 		pg_wchar_table[encoding].maxmblen :
 		pg_wchar_table[PG_SQL_ASCII].maxmblen;
 }
+
+/*
+ * cliplen for any single-byte encoding.
+ *
+ * Note: a copy of this function exists in mbutils.c, used by
+ * pg_mbcharcliplen().  They are kept separate because pg_mbcharcliplen
+ * is backend-only while pg_encoding_mbcliplen is in src/common.
+ */
+static int
+cliplen(const char *str, int len, int limit)
+{
+	int			l = 0;
+
+	len = Min(len, limit);
+	while (l < len && str[l])
+		l++;
+	return l;
+}
+
+/*
+ * pg_encoding_mbcliplen -- return the byte length of a multibyte string
+ * (not necessarily NULL terminated) that is no longer than limit bytes.
+ * This function does not break multibyte character boundaries.
+ *
+ * The string must be valid in the specified encoding.
+ */
+int
+pg_encoding_mbcliplen(int encoding, const char *mbstr,
+					  int len, int limit)
+{
+	mblen_converter mblen_fn;
+	int			clen = 0;
+	int			l;
+
+	/* optimization for single byte encoding */
+	if (pg_encoding_max_length(encoding) == 1)
+		return cliplen(mbstr, len, limit);
+
+	mblen_fn = pg_wchar_table[encoding].mblen;
+
+	while (len > 0 && *mbstr)
+	{
+		l = (*mblen_fn) ((const unsigned char *) mbstr);
+		if ((clen + l) > limit)
+			break;
+		clen += l;
+		if (clen == limit)
+			break;
+		len -= l;
+		mbstr += l;
+	}
+	return clen;
+}
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 8f4a2d9bbc1..554c01504ec 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -38,8 +38,6 @@ extern ObjectAddress DefineIndex(ParseState *pstate,
 								 bool skip_build,
 								 bool quiet);
 extern void ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel);
-extern char *makeObjectName(const char *name1, const char *name2,
-							const char *label);
 extern char *ChooseRelationName(const char *name1, const char *name2,
 								const char *label, Oid namespaceid,
 								bool isconstraint);
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 2a7c31ea74e..9d3963599fc 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -23,6 +23,10 @@ typedef struct PromptInterruptContext
 	bool		canceled;		/* indicates whether cancellation occurred */
 } PromptInterruptContext;
 
+/* functions in src/common/objectname.c */
+extern char *makeObjectName(const char *name1, const char *name2,
+							const char *label, int encoding);
+
 /* functions in src/common/string.c */
 extern bool pg_str_endswith(const char *str, const char *end);
 extern int	strtoint(const char *pg_restrict str, char **pg_restrict endptr,
-- 
2.52.0



^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2026-03-19 09:12 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-19 05:40 pg_dump: fix NOT NULL constraint name comparison using makeObjectName JoongHyuk Shin <[email protected]>
2026-03-19 09:12 ` JoongHyuk Shin <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox