Re: ERROR: failed to find conversion function from unknown to text

Hi jian,

jian he <jian(dot)universality(at)gmail(dot)com> wrote:
> I choose to disallow UNKNOWN target types in find_coercion_pathway.

I reviewed v2 on master at 9e17d25e79d. It applies cleanly, builds
without warnings, and make check passes. The new test in misc fails
without the parse_coerce.c change (I applied only the test files to
master), so it does exercise the fix.

It does what Tom asked for: statements that reached the internal
"failed to find conversion function" elog now get a user-level error,
42846 instead of XX000. One problem, though. Besides the PL/pgSQL
assignment you mention, v2 rejects statements that work today, and
some of them can be stored. On master all of these succeed:

create table t (a text);
create view v1 as select cast(a as unknown)::text as c from t;
create view v2 as select cast(a as unknown) is null as c from t;
create index i on t ((cast(a as unknown)::text));
create table t_chk (a text check (cast(a as unknown) is not null));
create table t_def (a text default cast('x'::text as unknown)::text);
create function f() returns text
begin atomic select cast('x'::text as unknown)::text; end;

A pg_dump of that database restored onto v2 fails for all six objects
with "cannot cast type text to unknown". Since the CHECK and the
DEFAULT are part of CREATE TABLE, t_chk and t_def are missing after the
restore, and so is their data. pg_upgrade from such a cluster to v2
fails as well:

pg_restore: error: could not execute query: ERROR: cannot cast
type text to unknown

Casting to unknown on purpose is unlikely, but when it is there the
cost is a failed upgrade.

The same elog was item 6 in Alexander's "Internal error codes triggered
by regression tests and user queries, take 2" [1], and it was left
aside because it was only reachable through unknownin(), an internal
input function. This thread shows it is also reachable with plain SQL:
a CAST to unknown, which is listed among the pseudo-types in the docs,
and from there through CREATE VIEW, CREATE TABLE AS and PREPARE.

So there is a narrower alternative to v2. Literals are handled at the
top of coerce_type(), so the only way to reach the "caller blew it"
elog from SQL is with an unknown input that is not a literal. Raising
ERRCODE_CANNOT_COERCE for that case only keeps the elog for real caller
bugs, and it can reuse "cannot cast type %s to %s", which parse_coerce.c
already uses four times and is translated. I tried it (attached as
.txt so the CF bot does not pick it up): the twelve statements in my
tests that give XX000 on master all get 42846, the statements that work
today keep working, the dump above restores with no errors, pg_upgrade
succeeds with the data in place, and make check passes.

The behavioral difference is that the cast itself stays legal and the
error is raised where the value is used:

master: failed to find conversion function from unknown to text
v2: cannot cast type text to unknown
narrower: cannot cast type unknown to text

If rejecting the cast outright is preferred, as v2 does, it would need
an answer for expressions that are already stored. I don't have a
strong opinion on which way is better, but these numbers seemed worth
having before choosing.

Two small things in v2: the new test lands at the end of misc.sql under
the "-- rewrite rules" heading, which it is unrelated to, and the
"commitfest entry:" line in the commit message is empty.

The SQL I used for the comparison is attached as well.

[1] https://www.postgresql.org/message-id/apUcBVyazHEW5pcg%40paquier.xyz

Regards,
Manu