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

Commit5ad7056

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 parente90f258 commit5ad7056

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
@@ -1295,6 +1295,10 @@ parser_coercion_errposition(ParseState *pstate,
12951295
* rather than throwing an error on failure.
12961296
* 'which_expr': if not NULL, receives a pointer to the particular input
12971297
* expression from which the result type was taken.
1298+
*
1299+
* Caution: "failure" just means that there were inputs of different type
1300+
* categories. It is not guaranteed that all the inputs are coercible to the
1301+
* selected type; caller must check that (see verify_common_type).
12981302
*/
12991303
Oid
13001304
select_common_type(ParseState*pstate,List*exprs,constchar*context,
@@ -1423,6 +1427,10 @@ select_common_type(ParseState *pstate, List *exprs, const char *context,
14231427
* earlier entries in the array have some preference over later ones.
14241428
* On failure, return InvalidOid if noerror is true, else throw an error.
14251429
*
1430+
* Caution: "failure" just means that there were inputs of different type
1431+
* categories. It is not guaranteed that all the inputs are coercible to the
1432+
* selected type; caller must check that (see verify_common_type_from_oids).
1433+
*
14261434
* Note: neither caller will pass any UNKNOWNOID entries, so the tests
14271435
* for that in this function are dead code. However, they don't cost much,
14281436
* and it seems better to keep this logic as close to select_common_type()
@@ -1545,6 +1553,48 @@ coerce_to_common_type(ParseState *pstate, Node *node,
15451553
returnnode;
15461554
}
15471555

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

17931843
if (!OidIsValid(anycompatible_typeid))
1794-
return false;/* there's no common supertype */
1844+
return false;/* there's definitely no common supertype */
1845+
1846+
/* We have to verify that the selected type actually works */
1847+
if (!verify_common_type_from_oids(anycompatible_typeid,
1848+
n_anycompatible_args,
1849+
anycompatible_actual_types))
1850+
return false;
17951851

17961852
if (have_anycompatible_nonarray)
17971853
{
@@ -2222,6 +2278,14 @@ enforce_generic_type_consistency(const Oid *actual_arg_types,
22222278
anycompatible_actual_types,
22232279
false);
22242280

2281+
/* We have to verify that the selected type actually works */
2282+
if (!verify_common_type_from_oids(anycompatible_typeid,
2283+
n_anycompatible_args,
2284+
anycompatible_actual_types))
2285+
ereport(ERROR,
2286+
(errcode(ERRCODE_DATATYPE_MISMATCH),
2287+
errmsg("arguments of anycompatible family cannot be cast to a common type")));
2288+
22252289
if (have_anycompatible_array)
22262290
{
22272291
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
@@ -1283,6 +1283,11 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
12831283
allexprs=list_concat(list_make1(lexpr),rnonvars);
12841284
scalar_type=select_common_type(pstate,allexprs,NULL,NULL);
12851285

1286+
/* We have to verify that the selected type actually works */
1287+
if (OidIsValid(scalar_type)&&
1288+
!verify_common_type(scalar_type,allexprs))
1289+
scalar_type=InvalidOid;
1290+
12861291
/*
12871292
* Do we have an array type to use? Aside from the case where there
12881293
* 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
externboolcheck_generic_type_consistency(constOid*actual_arg_types,
7576
constOid*declared_arg_types,

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,18 @@ SELECT 0 || ARRAY[1,2] || 3 AS "{0,1,2,3}";
726726
{0,1,2,3}
727727
(1 row)
728728

729+
SELECT array_agg(x) || array_agg(x) FROM (VALUES (ROW(1,2)), (ROW(3,4))) v(x);
730+
?column?
731+
-----------------------------------
732+
{"(1,2)","(3,4)","(1,2)","(3,4)"}
733+
(1 row)
734+
735+
SELECT ROW(1,2) || array_agg(x) FROM (VALUES (ROW(3,4)), (ROW(5,6))) v(x);
736+
?column?
737+
---------------------------
738+
{"(1,2)","(3,4)","(5,6)"}
739+
(1 row)
740+
729741
SELECT * FROM array_op_test WHERE i @> '{32}' ORDER BY seqno;
730742
seqno | i | t
731743
-------+---------------------------------+------------------------------------------------------------------------------------------------------------------------------------

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,33 @@ explain (verbose, costs off) select * from bpchar_view
231231
(3 rows)
232232

233233
rollback;
234+
--
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.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,8 @@ SELECT ARRAY[[['hello','world']]] || ARRAY[[['happy','birthday']]] AS "ARRAY";
311311
SELECT ARRAY[[1,2],[3,4]]|| ARRAY[5,6]AS"{{1,2},{3,4},{5,6}}";
312312
SELECT ARRAY[0,0]|| ARRAY[1,1]|| ARRAY[2,2]AS"{0,0,1,1,2,2}";
313313
SELECT0|| ARRAY[1,2]||3AS"{0,1,2,3}";
314+
SELECT array_agg(x)|| array_agg(x)FROM (VALUES (ROW(1,2)), (ROW(3,4))) v(x);
315+
SELECT ROW(1,2)|| array_agg(x)FROM (VALUES (ROW(3,4)), (ROW(5,6))) v(x);
314316

315317
SELECT*FROM array_op_testWHERE i @>'{32}'ORDER BY seqno;
316318
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
@@ -101,3 +101,19 @@ explain (verbose, costs off) select * from bpchar_view
101101
where f1::bpchar='foo';
102102

103103
rollback;
104+
105+
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));

0 commit comments

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