Internal error codes triggered by regression tests and user queries, take 2
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253558psql -h localhost -U postgresBuilt from patchset v11 (message #11), October 22, 2026 at 11:36 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253558_11 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253558_11 && git checkout t253558_11Patchset v11 (message #11) is on t253558_11
Hello Michael,
Please take a look at the follow-up to [0]/messages/by-id/aozYob22-UJ8CWzk@paquier.xyz. I've prepared a patch to
assign error codes for errors reached by the following queries:
1)
create table notnull_tbl_fail (a serial constraint foo not null constraint bar not null); -- from constraints.sql
ERROR: XX000: conflicting not-null constraint names "foo" and "bar"
LOCATION: transformColumnDefinition, parse_utilcmd.c:800
ERRCODE_INVALID_TABLE_DEFINITION
2)
SELECT satisfies_hash_partition(0, 4, 0, NULL); -- from hash_part.sql
ERROR: XX000: could not open relation with OID 0
LOCATION: relation_open, relation.c:62
ERRCODE_INVALID_PARAMETER_VALUE (also modified try_relation_open() for consistency)
3)
SELECT COUNT(*) = 0 AS ok FROM pg_shmem_allocations_numa; -- from numa.sql
ERROR: XX000: libnuma initialization failed or NUMA is not supported on this platform
LOCATION: pg_get_shmem_allocations_numa, shmem.c:1122
ERRCODE_FEATURE_NOT_SUPPORTED
4)
create table idxpart (a int) partition by range (a);
create table idxpart0 (like idxpart);
alter table idxpart0 add unique (a);
alter table idxpart attach partition idxpart0 default;
alter table only idxpart add primary key (a); -- from indexing.sql
ERROR: XX000: column "a" of table "idxpart0" is not marked NOT NULL
LOCATION: ATPrepAddPrimaryKey, tablecmds.c:9695
ERRCODE_INVALID_TABLE_DEFINITION
5)
CREATE FUNCTION test_pglz_compress(bytea)
RETURNS bytea
AS '.../src/test/regress/regress.so' LANGUAGE C STRICT;
CREATE FUNCTION test_pglz_decompress(bytea, int4, bool)
RETURNS bytea
AS '.../src/test/regress/regress.so' LANGUAGE C STRICT;
SELECT test_pglz_decompress(test_pglz_compress(
decode(repeat('abcd', 100), 'escape')), 500, true); -- compression_pglz.sql:
ERROR: XX000: pglz_decompress failed
LOCATION: test_pglz_decompress, regress.c:1497
ERRCODE_DATA_CORRUPTED (like for "compressed pglz data is corrupt")
With these changes plus
v1-0001-Report-specific-SQLSTATEs-for-stats-restore-error.patch from [1]
applied, `make check` passes without XX000 errors for me. Tested with:
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -498,6 +498,7 @@ errfinish(const char *filename, int lineno, const char *funcname)elevel = edata->elevel;
+Assert(!(elevel == ERROR && edata->sqlerrcode == ERRCODE_INTERNAL_ERROR));
/*
* Do processing in ErrorContext, which we hope has enough reserved space
* to report an error.
There are also other internal errors produced during `make check-world`,
but I think they should be considered separately, as most of them are too
generic or really internal, like "cache lookup failed for relation xxx"
triggered by intra-grant-inplace.
Besides the above, I've included in the patch assigning error codes to
errors reported by Justin Pryzby at [2]/messages/by-id/20230213135053.GZ1653@telsasoft.com, except for the dubious
amvalidate() and pg_read_file with parse_weight (both are already fixed):
6)
select unknownin('');
ERROR: XX000: failed to find conversion function from unknown to text
LOCATION: coerce_type, parse_coerce.c:544
ERRCODE_CANNOT_COERCE (used by other functions in parse_coerce.c)
7)
SELECT pg_catalog.interval( '12 seconds'::interval ,3);
ERROR: XX000: unrecognized interval typmod: 3
LOCATION: AdjustIntervalForTypmod, timestamp.c:1492
ERRCODE_INVALID_PARAMETER_VALUE (like below in the same function)
8)
SELECT pg_describe_object(1,0,1);
ERROR: XX000: unsupported object class: 1
LOCATION: getObjectDescription, objectaddress.c:4317
ERRCODE_WRONG_OBJECT_TYPE
9)
SELECT acldefault('a',0);
ERROR: XX000: unrecognized object type abbreviation: a
LOCATION: acldefault_sql, acl.c:999
ERRCODE_WRONG_OBJECT_TYPE
10)
select float8_regr_intercept(ARRAY[1]/messages/by-id/CAHGQGwHZLiLa9iM7NAiugp1B7CumN94=YBeho9t=qKJMnTGwMQ@mail.gmail.com);
ERROR: XX000: float8_regr_intercept: expected 8-element float8 array
LOCATION: check_float8_array, float.c:2985
ERRCODE_INVALID_PARAMETER_VALUE
A couple of cases I reported before:
11)
do $$ #print_strict_params XXX $$;
ERROR: XX000: unrecognized print_strict_params option xxx
CONTEXT: compilation of PL/pgSQL function "inline_code_block" near line 1
LOCATION: plpgsql_yyparse, pl_gram.y:396
ERRCODE_SYNTAX_ERROR
12)
select pg_catalog.range_in('', 23, 0);
ERROR: XX000: type 23 is not a range type
LOCATION: get_range_io_data, rangetypes.c:339
ERRCODE_DATATYPE_MISMATCH (like for "data type %s is not an array type")
[0]: /messages/by-id/aozYob22-UJ8CWzk@paquier.xyz
[1]: /messages/by-id/CAHGQGwHZLiLa9iM7NAiugp1B7CumN94=YBeho9t=qKJMnTGwMQ@mail.gmail.com
[2]: /messages/by-id/20230213135053.GZ1653@telsasoft.com
Best regards,
Alexander
Alexander Lakhin <exclusion@gmail.com> writes:
Please take a look at the follow-up to [0]. I've prepared a patch to
assign error codes for errors reached by the following queries:
I'm not on board with these proposed changes to [try_]relation_open.
They're basically band-aids rather than fixes of the root problems.
That is, if someone passes OID 0 to relation_open, that IS an internal
bug; labeling it otherwise is a lie and does nothing to fix the real
problem at the caller level.
A lot of these other proposed changes have the same whiff of blaming
the messenger rather than looking for the root cause.
regards, tom lane
Hi,
On 2026-08-26 10:26:58 -0400, Tom Lane wrote:
Alexander Lakhin <exclusion@gmail.com> writes:
Please take a look at the follow-up to [0]. I've prepared a patch to
assign error codes for errors reached by the following queries:I'm not on board with these proposed changes to [try_]relation_open.
They're basically band-aids rather than fixes of the root problems.
That is, if someone passes OID 0 to relation_open, that IS an internal
bug; labeling it otherwise is a lie and does nothing to fix the real
problem at the caller level.A lot of these other proposed changes have the same whiff of blaming
the messenger rather than looking for the root cause.
+1.
This is triply true for stuff like test_pglz_decompress() - this isn't
something that should be translated or that we care about having a proper
error code assigned.
Most of these would make it *harder* to find unexpected scenarios, i.e. the
patch would make the situation strictly worse. It's useful to be able to
search production logs for internal errors. As-is the patch would break that.
Looking through them, the ones that look relatively clearly to be worth
turning into ereports seem to be:
- pg_get_shmem_allocations_numa() - there's really no reason for this to be an
elog(), it's obviously expected to be reached on a bunch of platforms / configurations
- transformColumnDefinition() - that's clearly reachable with plain DDL,
without representing a bug in an upper layer.
I'm a bit on the fence about the one get_range_io_data(). We could catch that
on a higher layer, but
a) that'd be duplicated code,
b) other errors, like not having send/receive are already handled in
get_range_io_data, with ereports()
c) for e.g. composites, we handle the same problem centrally, rather than have
checks in record_in() etc (the check is in lookup_rowtype_tupdesc_internal).
Looking at this reminds me of something orthogonal: Isn't it pretty weird that
we have a per-type cache, with lots of information about the types, but don't
have the in/out/send/recv cached in there, but do cache EQ/LT/.. etc?
Greetings,
Andres Freund
Hello Andres and Tom,
Thank you for your feedback!
26.08.2026 18:17, Andres Freund wrote:
On 2026-08-26 10:26:58 -0400, Tom Lane wrote:
I'm not on board with these proposed changes to [try_]relation_open.
They're basically band-aids rather than fixes of the root problems.
That is, if someone passes OID 0 to relation_open, that IS an internal
bug; labeling it otherwise is a lie and does nothing to fix the real
problem at the caller level.
I agree. The fact that the internal error is reached from a legal SQL-level
function doesn't make all other ways to reach it expected/normal. This is
one issue with this proposal.
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 remember previous moves in this direction: [1]/messages/by-id/ba76c6bc-f03f-4285-bf16-47759cfcab9e@eisentraut.org, [2]/messages/by-id/Zic_GNgos5sMxKoa@paquier.xyz, so I started from
errors produced just by `make check`. I understand that such cases can be
discussed and processed one-by-one (e.g., bug #19659, #19630, #19631,
#19632), but given the number of "elog(ERROR)" in the tree and the number
of known ways to reach them (dozens, I guess), maybe it would be more
efficient to agree on the definition of internal errors and then
categorize all the cases at hand (I thought of creating a wiki page for
that).
A lot of these other proposed changes have the same whiff of blaming
the messenger rather than looking for the root cause.+1.
This is triply true for stuff like test_pglz_decompress() - this isn't
something that should be translated or that we care about having a proper
error code assigned.
Yes, and this consideration (we don't care about error code) is closer to
the definition 1, I guess. I proposed the change to align that error with
the similar "compressed pglz data is corrupt", as it doesn't make the
error less scary if we get back to the "search production logs for
critical errors" use case (on the other hand, the error is produced by a
test-only function...).
Most of these would make it *harder* to find unexpected scenarios, i.e. the
patch would make the situation strictly worse. It's useful to be able to
search production logs for internal errors. As-is the patch would break that.
Yes, it would hardly land as-is, but I hope it can help determine ways to
approach these errors in general.
Looking through them, the ones that look relatively clearly to be worth
turning into ereports seem to be:- pg_get_shmem_allocations_numa() - there's really no reason for this to be an
elog(), it's obviously expected to be reached on a bunch of platforms / configurations
- transformColumnDefinition() - that's clearly reachable with plain DDL,
without representing a bug in an upper layer.I'm a bit on the fence about the one get_range_io_data(). We could catch that
on a higher layer, but
a) that'd be duplicated code,
b) other errors, like not having send/receive are already handled in
get_range_io_data, with ereports()
c) for e.g. composites, we handle the same problem centrally, rather than have
checks in record_in() etc (the check is in lookup_rowtype_tupdesc_internal).
I would elaborate on these when the consensus on the definition is reached.
[1]: /messages/by-id/ba76c6bc-f03f-4285-bf16-47759cfcab9e@eisentraut.org
[2]: /messages/by-id/Zic_GNgos5sMxKoa@paquier.xyz
Best regards,
Alexander
On Fri, Aug 28, 2026 at 03:00:00PM +0300, Alexander Lakhin wrote:
I agree. The fact that the internal error is reached from a legal SQL-level
function doesn't make all other ways to reach it expected/normal. This is
one issue with this proposal.
Not all of them are. See below for details.
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.
- ATPrepAddPrimaryKey(). This one is a bug to me, reachable with SQL.
- transformColumnDefinition(). Also a bug to me.
- pg_get_shmem_allocations_numa. Bug.
- acldefault() is documented. Seems worth fixing for correctness,
documented function.
- AdjustIntervalForTypmod(). Documented feature.
- The pl_gram.y one looks like a defect to me by not assigning an
error code.
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.
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.
I would elaborate on these when the consensus on the definition is reached.
[1] /messages/by-id/ba76c6bc-f03f-4285-bf16-47759cfcab9e@eisentraut.org
[2] /messages/by-id/Zic_GNgos5sMxKoa@paquier.xyz
Among the 13 patterns you have sent, 7 of them make sense to me as
being reachable through documented patterns, 6 are not. As far as I
can see, Andres seems to me roughly among the same line, except for
get_range_io_data() that I disagree with as not reachable by normal
(aka documented) means based on the information provided.
Among the 7 patterns worth fixing, I'd suggest either improvements to
the error messages, or even better: check if we could reuse any of the
existing error messages already translated.
--
Michael
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
Alexander Lakhin <exclusion@gmail.com> writes:
Thank you for having a look! Please find v2 patch with 7 approved changes
attached.
Thanks for working on this cleanup, but I have a few quibbles:
@@ -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)));
}I think we mostly use ERRCODE_WRONG_OBJECT_TYPE for cases where a SQL
object exists but is the wrong kind for the command (eg, a table for
a command that operates on indexes). Here that's not the case;
there cannot be any relevant object. The only access path that
I know of is something like
regression=# select pg_describe_object(34,56,0);
ERROR: unsupported object class: 34
so I'd be kind of inclined to use ERRCODE_INVALID_PARAMETER_VALUE.
@@ -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));This does not seem like a "syntax error". Perhaps
ERRCODE_INVALID_OBJECT_DEFINITION would serve?
@@ -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)));
}As above, I think ERRCODE_INVALID_PARAMETER_VALUE is better suited.
Also, if we're going to take this seriously as a reachable error,
we need to protect against producing invalidly-encoded output
in case objtypec has the high bit set (compare ce6bf3cd1).
@@ -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)));Hmm, is this actually a user-reachable case? The place that I think
is reachable is over here:
regression=# select format_type('interval'::regtype, 52);
ERROR: invalid INTERVAL typmod: 0x34
regression=# \errverbose
ERROR: XX000: invalid INTERVAL typmod: 0x34
LOCATION: intervaltypmodout, timestamp.c:1189
We'd be well advised to make those two messages more consistent,
whether they're worth translating or not. Also I guess a sweep
for other "unreachable" errors in typmodout functions might be
useful.
@@ -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
{There's an argument for ERRCODE_INVALID_PARAMETER_VALUE here too,
perhaps. But I see that all the other uses of this errmsg string
use ERRCODE_SYNTAX_ERROR, so maybe it's fine as you have it.
regards, tom lane
Hello Tom,
06.09.2026 22:07, Tom Lane wrote:
Thanks for working on this cleanup, but I have a few quibbles:
@@ -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))); }I think we mostly use ERRCODE_WRONG_OBJECT_TYPE for cases where a SQL
object exists but is the wrong kind for the command (eg, a table for
a command that operates on indexes). Here that's not the case;
there cannot be any relevant object. The only access path that
I know of is something likeregression=# select pg_describe_object(34,56,0);
ERROR: unsupported object class: 34so I'd be kind of inclined to use ERRCODE_INVALID_PARAMETER_VALUE.
Thank you for the clarification! Will change this way.
@@ -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));This does not seem like a "syntax error". Perhaps
ERRCODE_INVALID_OBJECT_DEFINITION would serve?
Yeah, I considered this, but the same errmsg in
AddRelationNotNullConstraints() uses ERRCODE_SYNTAX_ERROR. I found it
more consistent to emit the same code. Maybe it would make do the opposite
-- change the errcode in AddRelationNotNullConstraints() (there are two
instances there, though)... Moreover, ERRCODE_SYNTAX_ERROR is chosen for
many other similar messages in transformColumnDefinition(), so I'm not
sure if it makes sense to change all of those or just report "syntax error"
for consistency while keeping the patch focused.
@@ -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))); }As above, I think ERRCODE_INVALID_PARAMETER_VALUE is better suited.
Also, if we're going to take this seriously as a reachable error,
we need to protect against producing invalidly-encoded output
in case objtypec has the high bit set (compare ce6bf3cd1).
Thank you for the suggestion! Will change the code and add the protection.
@@ -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)));Hmm, is this actually a user-reachable case? The place that I think
is reachable is over here:regression=# select format_type('interval'::regtype, 52);
ERROR: invalid INTERVAL typmod: 0x34
regression=# \errverbose
ERROR: XX000: invalid INTERVAL typmod: 0x34
LOCATION: intervaltypmodout, timestamp.c:1189
Yeah, I have this one in my collection too, but started from internal
errors reported previously -- that one at [1]/messages/by-id/20230213135053.GZ1653@telsasoft.com, it is reachable with,
e.g.:
SELECT pg_catalog.interval('1 second', 0);
ERROR: XX000: unrecognized interval typmod: 0
LOCATION: AdjustIntervalForTypmod, timestamp.c:1492
We'd be well advised to make those two messages more consistent,
whether they're worth translating or not. Also I guess a sweep
for other "unreachable" errors in typmodout functions might be
useful.
Will check them and include into this patch.
@@ -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 {There's an argument for ERRCODE_INVALID_PARAMETER_VALUE here too,
perhaps. But I see that all the other uses of this errmsg string
use ERRCODE_SYNTAX_ERROR, so maybe it's fine as you have it.
Yes, I'm not aware of cases where one errmsg is issued with different
codes (probably there may be such cases with very generic messages), but
I'd stick to reusing existing code if it's not too wrong.
[1]: /messages/by-id/20230213135053.GZ1653@telsasoft.com
Best regards,
Alexander
Hello Tom,
07.09.2026 00:00, Alexander Lakhin wrote:
06.09.2026 22:07, Tom Lane wrote:
@@ -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));This does not seem like a "syntax error". Perhaps
ERRCODE_INVALID_OBJECT_DEFINITION would serve?Yeah, I considered this, but the same errmsg in
AddRelationNotNullConstraints() uses ERRCODE_SYNTAX_ERROR. I found it
more consistent to emit the same code. Maybe it would make do the opposite
-- change the errcode in AddRelationNotNullConstraints() (there are two
instances there, though)... Moreover, ERRCODE_SYNTAX_ERROR is chosen for
many other similar messages in transformColumnDefinition(), so I'm not
sure if it makes sense to change all of those or just report "syntax error"
for consistency while keeping the patch focused.
I've changed that occurrence to ERRCODE_INVALID_OBJECT_DEFINITION, but as
I mentioned above, transformColumnDefinition() has many other SYNTAX_ERRORs
(originating from a56ff9a0b), besides AddRelationNotNullConstraints().
Please find attached a patch to correct these too, for consistency, if you
find this appropriate. There are other similar errors in parse_utilcmd.c,
but I'd stop here unless we want to start a new campaign not to define,
but correct error codes according to some convention.
We'd be well advised to make those two messages more consistent,
whether they're worth translating or not. Also I guess a sweep
for other "unreachable" errors in typmodout functions might be
useful
Changed three occurrences to use the same errmsg and errcode.
Please look at the attached v3-Define-...patch, where I think I addressed
all of your comments.
Thank you for your time!
Best regards,
Alexander
Attachments:
t253558_9v3-Define-missing-errcodes.patchtext/x-patch; charset=UTF-8; name=v3-Define-missing-errcodes.patchDownload+33-9
v3-Correct-errcodes-in-transformColumnDefinition-AddRelationNotNullConstraints.patchtext/x-patch; charset=UTF-8; name=v3-Correct-errcodes-in-transformColumnDefinition-AddRelationNotNullConstraints.patchDownload+14-14
On Sat, Sep 12, 2026 at 02:00:00PM +0300, Alexander Lakhin wrote:
Please look at the attached v3-Define-...patch, where I think I addressed
all of your comments.
The ACL bits were slightly puzzling because it's the first time I see
this pattern in the code (following Tom's comment upthread also in
tsvector), and I indeed get much better error messages:
=# SELECT acldefault(E'\x80'::"char", 0);
ERROR: 22021: invalid byte sequence for encoding "UTF8": 0x80
=# SELECT acldefault(chr(1)::"char", 10);
ERROR: 22023: unrecognized object type abbreviation: "\001"
The second query just showed nothing originally.
For the typmods, %d is a better choice to me as the input is an
integer.
As a whole, that looks OK, so applied on HEAD. I did not look at
0002.
--
Michael
Hello Michael and Tom,
22.09.2026 13:49, Michael Paquier wrote:
As a whole, that looks OK, so applied on HEAD. I did not look at
0002.
Let me share the following set of other internal errors:
1)
CREATE TABLE t AS SELECT ''::text a;
CREATE INDEX brinidx ON t USING brin
(a text_bloom_ops(n_distinct_per_range = -1.0));
ERROR: XX000: the bloom filter is too large (44629 > 8144)
LOCATION: bloom_init, brin_bloom.c:
-> ERRCODE_PROGRAM_LIMIT_EXCEEDED
2)
SELECT pg_identify_object_as_address('1'::regclass, 1, 1);
ERROR: XX000: unsupported object class: 1
LOCATION: getObjectTypeDescription, objectaddress.c:4731
-> ERRCODE_INVALID_PARAMETER_VALUE
3)
SELECT pg_get_acl('1'::regclass, 1, 0);
ERROR: XX000: unrecognized class ID: 1
LOCATION: get_object_property_data, objectaddress.c:2821
-> ERRCODE_INVALID_PARAMETER_VALUE
4)
SELECT jsonb_to_tsvector('{}', '[{}]'::jsonb);
ERROR: XX000: unexpected end of flag array
LOCATION: parse_jsonb_index_flags, jsonfuncs.c:5614
-> ERRCODE_INVALID_PARAMETER_VALUE
5)
CREATE PROCEDURE p(INOUT a integer) LANGUAGE sql
AS 'SELECT 1 WHERE false;';
CALL p(1);
ERROR: XX000: procedure returned null record
LOCATION: ExecuteCallStmt, functioncmds.c:2351
-> ERRCODE_INVALID_OBJECT_DEFINITION
6)
CREATE EXTENSION dblink;
CREATE FUNCTION connection_parameters() RETURNS text LANGUAGE SQL AS
$f$ SELECT $$dbname='$$||current_database()||$$' port=$$||current_setting('port'); $f$;
SELECT dblink_connect('myconn',connection_parameters());
SELECT * FROM dblink_send_query('myconn', 'SELECT 1');
SELECT * FROM dblink('myconn', 'SELECT 2') AS t(i int);
ERROR: XX000: could not send query: another command is already in progress
LOCATION: storeQueryResult, dblink.c:1103
-> ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE
Please look at the attached patch which defines these error codes.
Best regards,
Alexander