From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Subject: Re: Internal error codes triggered by regression tests and user queries, take 2
Date: Sat, 5 Sep 2026 11:00:00 +0300
Message-ID: <91a1b0ab-2c6c-4471-b7a4-ee8ee8aed64e@gmail.com> (raw)
In-Reply-To: <apUcBVyazHEW5pcg@paquier.xyz>
References: <94ac928b-8370-4db5-ada5-579a9ebc089d@gmail.com>
<697477.1787754418@sss.pgh.pa.us>
<mosliefjrclrikqpsasjy5ibzowba6vwk4ygimouikiht2i5pb@5g562jqflxsz>
<73eb724a-c1a3-456c-be12-0375d4556b67@gmail.com>
<apUcBVyazHEW5pcg@paquier.xyz>
Hello Michael,
31.08.2026 09:15, Michael Paquier wrote:
> Not all of them are. See below for details.
Thank you for having a look! Please find v2 patch with 7 approved changes
attached.
> Looking (finally) at the list you have sent in your patch, I can get
> on board for:
> - getObjectDescription(), can be queried with a function officially
> documented.
I kept the message as-is, as it's used in 3 other places, with elog(ERROR), but still...
> - ATPrepAddPrimaryKey(). This one is a bug to me, reachable with SQL.
already translatable
> - transformColumnDefinition(). Also a bug to me.
already translatable, thanks to AddRelationNotNullConstraints(), I
borrowed ERRCODE_SYNTAX_ERROR from it
> - pg_get_shmem_allocations_numa. Bug.
a unique message, looks good enough to be user-visible
> - acldefault() is documented. Seems worth fixing for correctness,
> documented function.
the message "unrecognized object type abbreviation: %c" is unique, a
closest one is "unrecognized object type \"%s\"", but I don't find it
suitable here
> - AdjustIntervalForTypmod(). Documented feature.
the message is unique, looks good too
> - The pl_gram.y one looks like a defect to me by not assigning an
> error code.
reused the closest translatable message "unrecognized %s option \"%s\""
> But not for these:
> - relation_open(), incorrect internal state from the caller.
> - try_relation_open(), incorrect internal state from the caller.
> coerce_type(), also an internal state. unknownin() is also an
> - "internal" input, non-documented SQL function.
> - check_float8_array(). Your case involves float8_regr_intercept(),
> which is a final aggregate function. It is not documented, and should
> never really be called directly.
> - get_range_io_data(). Cannot get excited about this one, either, as
> your case involves the direct call of an input function. Can this
> happen with more patterns?
> - test_pglz_decompress() is test code, no need to worry about it.
I agree, let's leave them aside until other (documented) ways to reach
them found.
>> The other one is the unclear definition of internal/XX000 errors in
>> principle. As far as I can see, it varies from "all the errors that have
>> no specific code assigned" to "critical errors, similar to failed asserts"
>> (and probably a substitute for asserts in production). And as Andres (and
>> I too) find it useful to search for XX000 in production logs, it means
>> they are interpreted per the second definition. In this case, it makes
>> sense to define missing codes for cases which are not worth paying
>> attention to in production, e.g. "#print_strict_params XXX". Moreover, if
>> the second definition (XX000 error ~ failed assert) is going to be
>> accepted, it seems sensible to have no such errors produced during
>> regression tests (like no assertion failures).
> I think that I would draw a line depending on the documentation. If
> a function is documented as usable, or callable through a view, it
> sounds fair to me to assume that a user gets an error code that fits
> with the incorrect behavior provided as input argument. Input
> function for types, test code, direct function calls to embedded
> facilities like aggregates, or anything like that, not documented, is
> not worth bothering. The fact that none of them is documented is an
> argument good enough for me to state that we don't really expect users
> to use these this way.
>
> The important piece to keep in mind is that non-internal ereport()
> calls mean translation. There is no point in providing translation
> for states that we expect users to never see in the field, and the
> docs define what we make user-visible.
Thank you for sharing your point of view! I'll review my collection in
light of it.
Best regards,
Alexander
Attachments:
[text/x-patch] v2-Define-missing-errcodes.patch (4.3K, ../91a1b0ab-2c6c-4471-b7a4-ee8ee8aed64e@gmail.com/3-v2-Define-missing-errcodes.patch)
download | inline diff:diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.cindex 703754a8123..cf6f4d866e4 100644--- a/src/backend/catalog/objectaddress.c+++ b/src/backend/catalog/objectaddress.c@@ -4314,7 +4314,9 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
}
default:
- elog(ERROR, "unsupported object class: %u", object->classId);+ ereport(ERROR,+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),+ errmsg("unsupported object class: %u", object->classId)));
}
/* an empty buffer is equivalent to no object found */
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex fd144d783d9..a3865c02e20 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -9631,6 +9631,7 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd,
tup = findNotNullConstraint(childrelid, strVal(column));
if (!tup)
ereport(ERROR,
+ errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("column \"%s\" of table \"%s\" is not marked NOT NULL",
strVal(column), get_rel_name(childrelid)));
/* verify it's good enough */
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.cindex d83616a8507..360bcbb2cbd 100644--- a/src/backend/parser/parse_utilcmd.c+++ b/src/backend/parser/parse_utilcmd.c@@ -794,8 +794,10 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
if (constraint->conname &&
notnull_constraint->conname &&
strcmp(notnull_constraint->conname, constraint->conname) != 0)
- elog(ERROR, "conflicting not-null constraint names \"%s\" and \"%s\"",- notnull_constraint->conname, constraint->conname);+ ereport(ERROR,+ errcode(ERRCODE_SYNTAX_ERROR),+ errmsg("conflicting not-null constraint names \"%s\" and \"%s\"",+ notnull_constraint->conname, constraint->conname));
if (notnull_constraint->is_no_inherit != constraint->is_no_inherit)
ereport(ERROR,
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.cindex f971ee24192..f78c34daee1 100644--- a/src/backend/storage/ipc/shmem.c+++ b/src/backend/storage/ipc/shmem.c@@ -1210,7 +1210,9 @@ pg_get_shmem_allocations_numa(PG_FUNCTION_ARGS)
Size *nodes;
if (pg_numa_init() == -1)
- elog(ERROR, "libnuma initialization failed or NUMA is not supported on this platform");+ ereport(ERROR,+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),+ errmsg("libnuma initialization failed or NUMA is not supported on this platform")));
InitMaterializedSRF(fcinfo, 0);
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.cindex 25cd5d0296b..2385c1ebbe9 100644--- a/src/backend/utils/adt/acl.c+++ b/src/backend/utils/adt/acl.c@@ -996,7 +996,9 @@ acldefault_sql(PG_FUNCTION_ARGS)
objtype = OBJECT_TYPE;
break;
default:
- elog(ERROR, "unrecognized object type abbreviation: %c", objtypec);+ ereport(ERROR,+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),+ errmsg("unrecognized object type abbreviation: %c", objtypec)));
}
PG_RETURN_ACL_P(acldefault(objtype, owner));
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.cindex 9c17ba2f905..ab86a03280e 100644--- a/src/backend/utils/adt/timestamp.c+++ b/src/backend/utils/adt/timestamp.c@@ -1489,7 +1489,9 @@ AdjustIntervalForTypmod(Interval *interval, int32 typmod,
/* fractional-second rounding will be dealt with below */
}
else
- elog(ERROR, "unrecognized interval typmod: %d", typmod);+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),+ errmsg("unrecognized interval typmod: %d", typmod)));
/* Need to adjust sub-second precision? */
if (precision != INTERVAL_FULL_PRECISION)
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.yindex 5e14a2d7302..37785f58cc2 100644--- a/src/pl/plpgsql/src/pl_gram.y+++ b/src/pl/plpgsql/src/pl_gram.y@@ -393,7 +393,9 @@ comp_option : '#' K_OPTION K_DUMP
else if (strcmp($3, "off") == 0)
plpgsql_curr_compile->print_strict_params = false;
else
- elog(ERROR, "unrecognized print_strict_params option %s", $3);+ ereport(ERROR,+ (errcode(ERRCODE_SYNTAX_ERROR),+ errmsg("unrecognized %s option \"%s\"", "print_strict_params", $3)));
}
| '#' K_VARIABLE_CONFLICT K_ERROR
{
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: exclusion@gmail.com, michael@paquier.xyz, andres@anarazel.de, tgl@sss.pgh.pa.us, pryzby@telsasoft.com
Subject: Re: Internal error codes triggered by regression tests and user queries, take 2
In-Reply-To: <91a1b0ab-2c6c-4471-b7a4-ee8ee8aed64e@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox