pgsql: Fix failure to validate the result of select_common_type(). - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Fix failure to validate the result of select_common_type().
Date
Msg-id E1nDqnH-0003Uv-VL@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Fix failure to validate the result of select_common_type().

Although select_common_type() has a failure-return convention, an
apparent successful return just provides a type OID that *might* work
as a common supertype; we've not validated that the required casts
actually exist.  In the mainstream use-cases that doesn't matter,
because we'll proceed to invoke coerce_to_common_type() on each input,
which will fail appropriately if the proposed common type doesn't
actually work.  However, a few callers didn't read the (nonexistent)
fine print, and thought that if they got back a nonzero OID then the
coercions were sure to work.

This affects in particular the recently-added "anycompatible"
polymorphic types; we might think that a function/operator using
such types matches cases it really doesn't.  A likely end result
of that is unexpected "ambiguous operator" errors, as for example
in bug #17387 from James Inform.  Another, much older, case is that
the parser might try to transform an "x IN (list)" construct to
a ScalarArrayOpExpr even when the list elements don't actually have
a common supertype.

It doesn't seem desirable to add more checking to select_common_type
itself, as that'd just slow down the mainstream use-cases.  Instead,
write a separate function verify_common_type that performs the
missing checks, and add a call to that where necessary.  Likewise add
verify_common_type_from_oids to go with select_common_type_from_oids.

Back-patch to v13 where the "anycompatible" types came in.  (The
symptom complained of in bug #17387 doesn't appear till v14, but
that's just because we didn't get around to converting || to use
anycompatible till then.)  In principle the "x IN (list)" fix could
go back all the way, but I'm not currently convinced that it makes
much difference in real-world cases, so I won't bother for now.

Discussion: https://postgr.es/m/17387-5dfe54b988444963@postgresql.org

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/8e2e0f75869e47b3a429b1251b00b3d6d5861028

Modified Files
--------------
src/backend/parser/parse_coerce.c         | 66 ++++++++++++++++++++++++++++++-
src/backend/parser/parse_expr.c           |  5 +++
src/include/parser/parse_coerce.h         |  1 +
src/test/regress/expected/arrays.out      | 12 ++++++
src/test/regress/expected/expressions.out | 30 ++++++++++++++
src/test/regress/sql/arrays.sql           |  2 +
src/test/regress/sql/expressions.sql      | 16 ++++++++
7 files changed, 131 insertions(+), 1 deletion(-)


pgsql-committers by date:

Previous
From: Michael Paquier
Date:
Subject: pgsql: Fix comments about bgworker registration before MaxBackends init
Next
From: Alvaro Herrera
Date:
Subject: pgsql: Remove xloginsert.h from xlog.h