Thread

More weird stuff in polymorphic type resolution

Tom Lane <tgl@sss.pgh.pa.us> — 2020-03-15T23:17:51Z

While poking at Pavel's "anycompatible" patch, I found a couple
more pre-existing issues having to do with special cases for
actual input type "anyarray".  Ordinarily that would be impossible
since we should have resolved "anyarray" to some specific array
type earlier; but you can make it happen by applying a function
to one of the "anyarray" columns of pg_statistic or pg_stats.

* I can provoke an assertion failure thus:

regression=# create function fooid(f1 anyarray) returns anyarray
as 'select $1' language sql;
CREATE FUNCTION
regression=# select fooid(stavalues1) from pg_statistic;
server closed the connection unexpectedly

The server log shows

TRAP: BadArgument("!IsPolymorphicType(rettype)", File: "functions.c", Line: 1606)
postgres: postgres regression [local] SELECT(ExceptionalCondition+0x55)[0x8e85c5]
postgres: postgres regression [local] SELECT(check_sql_fn_retval+0x79b)[0x6664db]

The reason this happens is that the parser intentionally allows an
actual argument type of anyarray to match a declared argument type
of anyarray, whereupon the "resolved" function output type is also
anyarray.  We have some regression test cases that depend on that
behavior, so we can't just take it out.  However, check_sql_fn_retval
is assuming too much.  In a non-assert build, what happens is

ERROR:  42P13: return type anyarray is not supported for SQL functions
CONTEXT:  SQL function "fooid" during inlining
LOCATION:  check_sql_fn_retval, functions.c:1888

because the code after the assert properly rejects pseudotypes.
That seems fine, so I think it's sufficient to take out that assertion.
(Note: all the PLs throw errors successfully, so it's just SQL-language
functions with this issue.)

* There's some inconsistency between these cases:

regression=# create function foo1(f1 anyarray, f2 anyelement) returns bool
language sql as 'select $1[1] = f2';
CREATE FUNCTION
regression=# select foo1(stavalues1, 46) from pg_statistic;
ERROR:  function foo1(anyarray, integer) does not exist
LINE 1: select foo1(stavalues1, 46) from pg_statistic;
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

regression=# create function foo2(f1 anyarray, f2 anyrange) returns bool
language sql as 'select $1[1] = lower(f2)';
CREATE FUNCTION
regression=# select foo2(stavalues1, int8range(42,46)) from pg_statistic;
ERROR:  argument declared anyrange is not consistent with argument declared anyelement
DETAIL:  int8range versus anyelement

The reason for the inconsistency is that parse_coerce.c has special cases
to forbid the combination of (a) matching an actual argument type of
anyarray to a declared anyarray while (b) also having an anyelement
argument.  That's to prevent the risk that the actual array element type
doesn't agree with the other argument.  But there's no similar restriction
for the combination of anyarray and anyrange arguments, which seems like
a clear oversight in the anyrange logic.  On reflection, in fact, we
should not allow matching an actual-argument anyarray if there are *any*
other pseudotype arguments, including another anyarray, because we can't
guarantee that the two anyarrays will contain matching argument types.

A rule like that would also justify not worrying about matching
anyarray to anycompatiblearray (as Pavel's patch already doesn't,
though not for any documented reason).  There is little point in using
anycompatiblearray unless there's at least one other polymorphic argument
to match it against, so if we're going to reject anyarray input in such
cases, there's no situation where it's useful to allow that.

Having said that, I'm not sure whether to prefer the first error, which
happens because check_generic_type_consistency decides the function
doesn't match at all, or the second, where check_generic_type_consistency
accepts the match and then enforce_generic_type_consistency spits up.
The second error isn't all that much better ... but maybe we could accept
the match and then make enforce_generic_type_consistency issue a more
on-point error?  That would carry some risk of creating ambiguous-function
errors where there were none before, but I doubt it's a big problem.
If that change makes us unable to pick a function, the same failure
would occur for a similar call involving a regular array column.

In any case I'm inclined not to back-patch a fix for the second issue,
since all it's going to do is exchange one error for another.  Not sure
about the assertion removal --- that wouldn't affect production builds,
but people doing development/testing on back branches might appreciate it.

Thoughts?

			regards, tom lane