Skip to content

Commit f80872b

Browse files
tglsfdctglsfdc
committed
Remove unnecessary restrictions about RowExprs in transformAExprIn().
When the existing code here was written, it made sense to special-case RowExprs because that was the only way that we could handle row comparisons at all. Now that we have record_eq() and arrays of composites, the generic logic for "scalar" types will in fact work on RowExprs too, so there's no reason to throw error for combinations of RowExprs and other ways of forming composite values, nor to ignore the possibility of using a ScalarArrayOpExpr. But keep using the old logic when comparing two RowExprs, for consistency with the main transformAExprOp() logic. (This allows some cases with not-quite-identical rowtypes to succeed, so we might get push-back if we removed it.) Per bug #8198 from Rafal Rzepecki. Back-patch to all supported branches, since this works fine as far back as 8.4. Rafal Rzepecki and Tom Lane
1 parent 1476a94 commit f80872b

File tree

3 files changed

+78
-24
lines changed

3 files changed

+78
-24
lines changed

‎src/backend/parser/parse_expr.c

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
873873
else if (lexpr && IsA(lexpr, RowExpr) &&
874874
rexpr && IsA(rexpr, RowExpr))
875875
{
876-
/* "row op row" */
876+
/* ROW() op ROW() is handled specially */
877877
lexpr = transformExpr(pstate, lexpr);
878878
rexpr = transformExpr(pstate, rexpr);
879879
Assert(IsA(lexpr, RowExpr));
@@ -978,7 +978,7 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a)
978978
if (lexpr && IsA(lexpr, RowExpr) &&
979979
rexpr && IsA(rexpr, RowExpr))
980980
{
981-
/* "row op row" */
981+
/* ROW() op ROW() is handled specially */
982982
return make_row_distinct_op(pstate, a->name,
983983
(RowExpr *) lexpr,
984984
(RowExpr *) rexpr,
@@ -1068,7 +1068,6 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
10681068
List *rvars;
10691069
List *rnonvars;
10701070
bool useOr;
1071-
bool haveRowExpr;
10721071
ListCell *l;
10731072

10741073
/*
@@ -1081,24 +1080,21 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
10811080

10821081
/*
10831082
* We try to generate a ScalarArrayOpExpr from IN/NOT IN, but this is only
1084-
* possible if the inputs are all scalars (no RowExprs) and there is a
1085-
* suitable array type available. If not, we fall back to a boolean
1086-
* condition tree with multiple copies of the lefthand expression. Also,
1087-
* any IN-list items that contain Vars are handled as separate boolean
1088-
* conditions, because that gives the planner more scope for optimization
1089-
* on such clauses.
1083+
* possible if there is a suitable array type available. If not, we fall
1084+
* back to a boolean condition tree with multiple copies of the lefthand
1085+
* expression. Also, any IN-list items that contain Vars are handled as
1086+
* separate boolean conditions, because that gives the planner more scope
1087+
* for optimization on such clauses.
10901088
*
1091-
* First step: transform all the inputs, and detect whether any are
1092-
* RowExprs or contain Vars.
1089+
* First step: transform all the inputs, and detect whether any contain
1090+
* Vars.
10931091
*/
10941092
lexpr = transformExpr(pstate, a->lexpr);
1095-
haveRowExpr = (lexpr && IsA(lexpr, RowExpr));
10961093
rexprs = rvars = rnonvars = NIL;
10971094
foreach(l, (List *) a->rexpr)
10981095
{
10991096
Node *rexpr = transformExpr(pstate, lfirst(l));
11001097

1101-
haveRowExpr |= (rexpr && IsA(rexpr, RowExpr));
11021098
rexprs = lappend(rexprs, rexpr);
11031099
if (contain_vars_of_level(rexpr, 0))
11041100
rvars = lappend(rvars, rexpr);
@@ -1108,9 +1104,9 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11081104

11091105
/*
11101106
* ScalarArrayOpExpr is only going to be useful if there's more than one
1111-
* non-Var righthand item. Also, it won't work for RowExprs.
1107+
* non-Var righthand item.
11121108
*/
1113-
if (!haveRowExpr && list_length(rnonvars) > 1)
1109+
if (list_length(rnonvars) > 1)
11141110
{
11151111
List *allexprs;
11161112
Oid scalar_type;
@@ -1126,8 +1122,13 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11261122
allexprs = list_concat(list_make1(lexpr), rnonvars);
11271123
scalar_type = select_common_type(pstate, allexprs, NULL, NULL);
11281124

1129-
/* Do we have an array type to use? */
1130-
if (OidIsValid(scalar_type))
1125+
/*
1126+
* Do we have an array type to use? Aside from the case where there
1127+
* isn't one, we don't risk using ScalarArrayOpExpr when the common
1128+
* type is RECORD, because the RowExpr comparison logic below can cope
1129+
* with some cases of non-identical row types.
1130+
*/
1131+
if (OidIsValid(scalar_type) && scalar_type != RECORDOID)
11311132
array_type = get_array_type(scalar_type);
11321133
else
11331134
array_type = InvalidOid;
@@ -1177,26 +1178,25 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11771178
Node *rexpr = (Node *) lfirst(l);
11781179
Node *cmp;
11791180

1180-
if (haveRowExpr)
1181+
if (IsA(lexpr, RowExpr) &&
1182+
IsA(rexpr, RowExpr))
11811183
{
1182-
if (!IsA(lexpr, RowExpr) ||
1183-
!IsA(rexpr, RowExpr))
1184-
ereport(ERROR,
1185-
(errcode(ERRCODE_SYNTAX_ERROR),
1186-
errmsg("arguments of row IN must all be row expressions"),
1187-
parser_errposition(pstate, a->location)));
1184+
/* ROW() op ROW() is handled specially */
11881185
cmp = make_row_comparison_op(pstate,
11891186
a->name,
11901187
(List *) copyObject(((RowExpr *) lexpr)->args),
11911188
((RowExpr *) rexpr)->args,
11921189
a->location);
11931190
}
11941191
else
1192+
{
1193+
/* Ordinary scalar operator */
11951194
cmp = (Node *) make_op(pstate,
11961195
a->name,
11971196
copyObject(lexpr),
11981197
rexpr,
11991198
a->location);
1199+
}
12001200

12011201
cmp = coerce_to_boolean(pstate, cmp, "IN");
12021202
if (result == NULL)

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,25 @@ ERROR: could not determine interpretation of row comparison operator ~~
205205
LINE 1: select ROW('ABC','DEF') ~~ ROW('DEF','ABC') as fail;
206206
^
207207
HINT: Row comparison operators must be associated with btree operator families.
208+
-- Comparisons of ROW() expressions can cope with some type mismatches
209+
select ROW(1,2) = ROW(1,2::int8);
210+
?column?
211+
----------
212+
t
213+
(1 row)
214+
215+
select ROW(1,2) in (ROW(3,4), ROW(1,2));
216+
?column?
217+
----------
218+
t
219+
(1 row)
220+
221+
select ROW(1,2) in (ROW(3,4), ROW(1,2::int8));
222+
?column?
223+
----------
224+
t
225+
(1 row)
226+
208227
-- Check row comparison with a subselect
209228
select unique1, unique2 from tenk1
210229
where (unique1, unique2) < any (select ten, ten from tenk1 where hundred < 3)
@@ -249,6 +268,26 @@ order by thousand, tenthous;
249268
999 | 9999
250269
(25 rows)
251270

271+
-- Check row comparisons with IN
272+
select * from int8_tbl i8 where i8 in (row(123,456)); -- fail, type mismatch
273+
ERROR: cannot compare dissimilar column types bigint and integer at record column 1
274+
explain (costs off)
275+
select * from int8_tbl i8
276+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
277+
QUERY PLAN
278+
-------------------------------------------------------------------------------------------------------------
279+
Seq Scan on int8_tbl i8
280+
Filter: (i8.* = ANY (ARRAY[ROW(123::bigint, 456::bigint)::int8_tbl, '(4567890123456789,123)'::int8_tbl]))
281+
(2 rows)
282+
283+
select * from int8_tbl i8
284+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
285+
q1 | q2
286+
------------------+-----
287+
123 | 456
288+
4567890123456789 | 123
289+
(2 rows)
290+
252291
-- Check some corner cases involving empty rowtypes
253292
select ROW();
254293
row

‎src/test/regress/sql/rowtypes.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ select ROW('ABC','DEF') ~<=~ ROW('DEF','ABC') as true;
9595
select ROW('ABC','DEF') ~>=~ ROW('DEF','ABC') as false;
9696
select ROW('ABC','DEF') ~~ ROW('DEF','ABC') as fail;
9797

98+
-- Comparisons of ROW() expressions can cope with some type mismatches
99+
select ROW(1,2) = ROW(1,2::int8);
100+
select ROW(1,2) in (ROW(3,4), ROW(1,2));
101+
select ROW(1,2) in (ROW(3,4), ROW(1,2::int8));
102+
98103
-- Check row comparison with a subselect
99104
select unique1, unique2 from tenk1
100105
where (unique1, unique2) < any (select ten, ten from tenk1 where hundred < 3)
@@ -106,6 +111,16 @@ select thousand, tenthous from tenk1
106111
where (thousand, tenthous) >= (997, 5000)
107112
order by thousand, tenthous;
108113

114+
-- Check row comparisons with IN
115+
select * from int8_tbl i8 where i8 in (row(123,456)); -- fail, type mismatch
116+
117+
explain (costs off)
118+
select * from int8_tbl i8
119+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
120+
121+
select * from int8_tbl i8
122+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
123+
109124
-- Check some corner cases involving empty rowtypes
110125
select ROW();
111126
select ROW() IS NULL;

0 commit comments

Comments
 (0)