Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit8e2e0f7

Browse files
committed
Fix failure to validate the result of select_common_type().
Although select_common_type() has a failure-return convention, anapparent successful return just provides a type OID that *might* workas a common supertype; we've not validated that the required castsactually 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'tactually work. However, a few callers didn't read the (nonexistent)fine print, and thought that if they got back a nonzero OID then thecoercions were sure to work.This affects in particular the recently-added "anycompatible"polymorphic types; we might think that a function/operator usingsuch types matches cases it really doesn't. A likely end resultof that is unexpected "ambiguous operator" errors, as for examplein bug #17387 from James Inform. Another, much older, case is thatthe parser might try to transform an "x IN (list)" construct toa ScalarArrayOpExpr even when the list elements don't actually havea common supertype.It doesn't seem desirable to add more checking to select_common_typeitself, as that'd just slow down the mainstream use-cases. Instead,write a separate function verify_common_type that performs themissing checks, and add a call to that where necessary. Likewise addverify_common_type_from_oids to go with select_common_type_from_oids.Back-patch to v13 where the "anycompatible" types came in. (Thesymptom complained of in bug #17387 doesn't appear till v14, butthat's just because we didn't get around to converting || to useanycompatible till then.) In principle the "x IN (list)" fix couldgo back all the way, but I'm not currently convinced that it makesmuch difference in real-world cases, so I won't bother for now.Discussion:https://postgr.es/m/17387-5dfe54b988444963@postgresql.org
1 parent5ecd018 commit8e2e0f7

File tree

7 files changed

+131
-1
lines changed

7 files changed

+131
-1
lines changed

‎src/backend/parser/parse_coerce.c

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1298,6 +1298,10 @@ parser_coercion_errposition(ParseState *pstate,
12981298
* rather than throwing an error on failure.
12991299
* 'which_expr': if not NULL, receives a pointer to the particular input
13001300
* expression from which the result type was taken.
1301+
*
1302+
* Caution: "failure" just means that there were inputs of different type
1303+
* categories. It is not guaranteed that all the inputs are coercible to the
1304+
* selected type; caller must check that (see verify_common_type).
13011305
*/
13021306
Oid
13031307
select_common_type(ParseState*pstate,List*exprs,constchar*context,
@@ -1426,6 +1430,10 @@ select_common_type(ParseState *pstate, List *exprs, const char *context,
14261430
* earlier entries in the array have some preference over later ones.
14271431
* On failure, return InvalidOid if noerror is true, else throw an error.
14281432
*
1433+
* Caution: "failure" just means that there were inputs of different type
1434+
* categories. It is not guaranteed that all the inputs are coercible to the
1435+
* selected type; caller must check that (see verify_common_type_from_oids).
1436+
*
14291437
* Note: neither caller will pass any UNKNOWNOID entries, so the tests
14301438
* for that in this function are dead code. However, they don't cost much,
14311439
* and it seems better to keep this logic as close to select_common_type()
@@ -1548,6 +1556,48 @@ coerce_to_common_type(ParseState *pstate, Node *node,
15481556
returnnode;
15491557
}
15501558

1559+
/*
1560+
* verify_common_type()
1561+
*Verify that all input types can be coerced to a proposed common type.
1562+
*Return true if so, false if not all coercions are possible.
1563+
*
1564+
* Most callers of select_common_type() don't need to do this explicitly
1565+
* because the checks will happen while trying to convert input expressions
1566+
* to the right type, e.g. in coerce_to_common_type(). However, if a separate
1567+
* check step is needed to validate the applicability of the common type, call
1568+
* this.
1569+
*/
1570+
bool
1571+
verify_common_type(Oidcommon_type,List*exprs)
1572+
{
1573+
ListCell*lc;
1574+
1575+
foreach(lc,exprs)
1576+
{
1577+
Node*nexpr= (Node*)lfirst(lc);
1578+
Oidntype=exprType(nexpr);
1579+
1580+
if (!can_coerce_type(1,&ntype,&common_type,COERCION_IMPLICIT))
1581+
return false;
1582+
}
1583+
return true;
1584+
}
1585+
1586+
/*
1587+
* verify_common_type_from_oids()
1588+
*As above, but work from an array of type OIDs.
1589+
*/
1590+
staticbool
1591+
verify_common_type_from_oids(Oidcommon_type,intnargs,constOid*typeids)
1592+
{
1593+
for (inti=0;i<nargs;i++)
1594+
{
1595+
if (!can_coerce_type(1,&typeids[i],&common_type,COERCION_IMPLICIT))
1596+
return false;
1597+
}
1598+
return true;
1599+
}
1600+
15511601
/*
15521602
* select_common_typmod()
15531603
*Determine the common typmod of a list of input expressions.
@@ -1917,7 +1967,13 @@ check_generic_type_consistency(const Oid *actual_arg_types,
19171967
true);
19181968

19191969
if (!OidIsValid(anycompatible_typeid))
1920-
return false;/* there's no common supertype */
1970+
return false;/* there's definitely no common supertype */
1971+
1972+
/* We have to verify that the selected type actually works */
1973+
if (!verify_common_type_from_oids(anycompatible_typeid,
1974+
n_anycompatible_args,
1975+
anycompatible_actual_types))
1976+
return false;
19211977

19221978
if (have_anycompatible_nonarray)
19231979
{
@@ -2494,6 +2550,14 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
24942550
anycompatible_actual_types,
24952551
false);
24962552

2553+
/* We have to verify that the selected type actually works */
2554+
if (!verify_common_type_from_oids(anycompatible_typeid,
2555+
n_anycompatible_args,
2556+
anycompatible_actual_types))
2557+
ereport(ERROR,
2558+
(errcode(ERRCODE_DATATYPE_MISMATCH),
2559+
errmsg("arguments of anycompatible family cannot be cast to a common type")));
2560+
24972561
if (have_anycompatible_array)
24982562
{
24992563
anycompatible_array_typeid=get_array_type(anycompatible_typeid);

‎src/backend/parser/parse_expr.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,11 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11211121
allexprs=list_concat(list_make1(lexpr),rnonvars);
11221122
scalar_type=select_common_type(pstate,allexprs,NULL,NULL);
11231123

1124+
/* We have to verify that the selected type actually works */
1125+
if (OidIsValid(scalar_type)&&
1126+
!verify_common_type(scalar_type,allexprs))
1127+
scalar_type=InvalidOid;
1128+
11241129
/*
11251130
* Do we have an array type to use? Aside from the case where there
11261131
* isn't one, we don't risk using ScalarArrayOpExpr when the common

‎src/include/parser/parse_coerce.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ extern Oidselect_common_type(ParseState *pstate, List *exprs,
7070
externNode*coerce_to_common_type(ParseState*pstate,Node*node,
7171
OidtargetTypeId,
7272
constchar*context);
73+
externboolverify_common_type(Oidcommon_type,List*exprs);
7374

7475
externint32select_common_typmod(ParseState*pstate,List*exprs,Oidcommon_type);
7576

‎src/test/regress/expected/arrays.out

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,18 @@ SELECT ARRAY[1.1] || ARRAY[2,3,4];
747747
{1.1,2,3,4}
748748
(1 row)
749749

750+
SELECT array_agg(x) || array_agg(x) FROM (VALUES (ROW(1,2)), (ROW(3,4))) v(x);
751+
?column?
752+
-----------------------------------
753+
{"(1,2)","(3,4)","(1,2)","(3,4)"}
754+
(1 row)
755+
756+
SELECT ROW(1,2) || array_agg(x) FROM (VALUES (ROW(3,4)), (ROW(5,6))) v(x);
757+
?column?
758+
---------------------------
759+
{"(1,2)","(3,4)","(5,6)"}
760+
(1 row)
761+
750762
SELECT * FROM array_op_test WHERE i @> '{32}' ORDER BY seqno;
751763
seqno | i | t
752764
-------+---------------------------------+------------------------------------------------------------------------------------------------------------------------------------

‎src/test/regress/expected/expressions.out

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,36 @@ explain (verbose, costs off) select * from bpchar_view
232232

233233
rollback;
234234
--
235+
-- Ordinarily, IN/NOT IN can be converted to a ScalarArrayOpExpr
236+
-- with a suitably-chosen array type.
237+
--
238+
explain (verbose, costs off)
239+
select random() IN (1, 4, 8.0);
240+
QUERY PLAN
241+
------------------------------------------------------------
242+
Result
243+
Output: (random() = ANY ('{1,4,8}'::double precision[]))
244+
(2 rows)
245+
246+
explain (verbose, costs off)
247+
select random()::int IN (1, 4, 8.0);
248+
QUERY PLAN
249+
---------------------------------------------------------------------------
250+
Result
251+
Output: (((random())::integer)::numeric = ANY ('{1,4,8.0}'::numeric[]))
252+
(2 rows)
253+
254+
-- However, if there's not a common supertype for the IN elements,
255+
-- we should instead try to produce "x = v1 OR x = v2 OR ...".
256+
-- In most cases that'll fail for lack of all the requisite = operators,
257+
-- but it can succeed sometimes. So this should complain about lack of
258+
-- an = operator, not about cast failure.
259+
select '(0,0)'::point in ('(0,0,0,0)'::box, point(0,0));
260+
ERROR: operator does not exist: point = box
261+
LINE 1: select '(0,0)'::point in ('(0,0,0,0)'::box, point(0,0));
262+
^
263+
HINT: No operator matches the given name and argument types. You might need to add explicit type casts.
264+
--
235265
-- Tests for ScalarArrayOpExpr with a hashfn
236266
--
237267
-- create a stable function so that the tests below are not

‎src/test/regress/sql/arrays.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,8 @@ SELECT ARRAY[[1,2],[3,4]] || ARRAY[5,6] AS "{{1,2},{3,4},{5,6}}";
317317
SELECT ARRAY[0,0]|| ARRAY[1,1]|| ARRAY[2,2]AS"{0,0,1,1,2,2}";
318318
SELECT0|| ARRAY[1,2]||3AS"{0,1,2,3}";
319319
SELECT ARRAY[1.1]|| ARRAY[2,3,4];
320+
SELECT array_agg(x)|| array_agg(x)FROM (VALUES (ROW(1,2)), (ROW(3,4))) v(x);
321+
SELECT ROW(1,2)|| array_agg(x)FROM (VALUES (ROW(3,4)), (ROW(5,6))) v(x);
320322

321323
SELECT*FROM array_op_testWHERE i @>'{32}'ORDER BY seqno;
322324
SELECT*FROM array_op_testWHERE i &&'{32}'ORDER BY seqno;

‎src/test/regress/sql/expressions.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,22 @@ explain (verbose, costs off) select * from bpchar_view
103103
rollback;
104104

105105

106+
--
107+
-- Ordinarily, IN/NOT IN can be converted to a ScalarArrayOpExpr
108+
-- with a suitably-chosen array type.
109+
--
110+
explain (verbose, costs off)
111+
select random()IN (1,4,8.0);
112+
explain (verbose, costs off)
113+
select random()::intIN (1,4,8.0);
114+
-- However, if there's not a common supertype for the IN elements,
115+
-- we should instead try to produce "x = v1 OR x = v2 OR ...".
116+
-- In most cases that'll fail for lack of all the requisite = operators,
117+
-- but it can succeed sometimes. So this should complain about lack of
118+
-- an = operator, not about cast failure.
119+
select'(0,0)'::pointin ('(0,0,0,0)'::box,point(0,0));
120+
121+
106122
--
107123
-- Tests for ScalarArrayOpExpr with a hashfn
108124
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp