Re: pg_dump: fix NOT NULL constraint name comparison using makeObjectName

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 <sjh910805(at)gmail(dot)com> 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.
>