Re: Internal error codes triggered by regression tests and user queries, take 2

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alexander Lakhin <exclusion(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Justin Pryzby <pryzby(at)telsasoft(dot)com>
Subject: Re: Internal error codes triggered by regression tests and user queries, take 2
Date: 2026-08-31 06:15:33
Message-ID: apUcBVyazHEW5pcg@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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] https://www.postgresql.org/message-id/flat/ba76c6bc-f03f-4285-bf16-47759cfcab9e%40eisentraut.org
> [2] https://www.postgresql.org/message-id/Zic_GNgos5sMxKoa%40paquier.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

In response to

Browse pgsql-hackers by date

  From Date Subject
Previous Message Michael Paquier 2026-08-31 05:37:46 Re: WAL_LOG CREATE DATABASE strategy broken for non-standard page layouts