8000 Remove unnecessary restrictions about RowExprs in transformAExprIn(). · kosalalakshitha/postgres@5cd77ba · GitHub
[go: up one dir, main page]
Skip to content

Commit 5cd77ba

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 5f4a311 commit 5cd77ba

File tree

3 files changed

+93
-24
lines changed

3 files changed

+93
-24
lines changed

‎src/backend/parser/parse_expr.c

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ transformAExprOp(ParseState *pstate, A_Expr *a)
879879
else if (lexpr && IsA(lexpr, RowExpr) &&
880880
rexpr && IsA(rexpr, RowExpr))
881881
{
882-
/* "row op row" */
882+
/* ROW() op ROW() is handled specially */
883883
lexpr = transformExpr(pstate, lexpr);
884884
rexpr = transformExpr(pstate, rexpr);
885885
Assert(IsA(lexpr, RowExpr));
@@ -984,7 +984,7 @@ transformAExprDistinct(ParseState *pstate, A_Expr *a)
984984
if (lexpr && IsA(lexpr, RowExpr) &&
985985
rexpr && IsA(rexpr, RowExpr))
986986
{
987-
/* "row op row" */
987+
/* ROW() op ROW() is handled specially */
988988
return make_row_distinct_op(pstate, a->name,
989989
(RowExpr *) lexpr,
990990
(RowExpr *) rexpr,
@@ -1083,7 +1083,6 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
10831083
List *rvars;
10841084
List *rnonvars;
10851085
bool useOr;
1086-
bool haveRowExpr;
10871086
ListCell *l;
10881087

10891088
/*
@@ -1096,24 +1095,21 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
10961095

10971096
/*
10981097
* We try to generate a ScalarArrayOpExpr from IN/NOT IN, but this is only
1099-
* possible if the inputs are all scalars (no RowExprs) and there is a
1100-
* suitable array type available. If not, we fall back to a boolean
1101-
* condition tree with multiple copies of the lefthand expression. Also,
1102-
* any IN-list items that contain Vars are handled as separate boolean
1103-
* conditions, because that gives the planner more scope for optimization
1104-
* on such clauses.
1098+
* possible if there is a suitable array type available. If not, we fall
1099+
* back to a boolean condition tree with multiple copies of the lefthand
1100+
* expression. Also, any IN-list items that contain Vars are handled as
1101+
* separate boolean conditions, because that gives the planner more scope
1102+
* for optimization on such clauses.
11051103
*
1106-
* First step: transform all the inputs, and detect whether any are
1107-
* RowExprs or contain Vars.
1104+
* First step: transform all the inputs, and detect whether any contain
1105+
* Vars.
11081106
*/
11091107
lexpr = transformExpr(pstate, a->lexpr);
1110-
haveRowExpr = (lexpr && IsA(lexpr, RowExpr));
11111108
rexprs = rvars = rnonvars = NIL;
11121109
foreach(l, (List *) a->rexpr)
11131110
{
11141111
Node *rexpr = transformExpr(pstate, lfirst(l));
11151112

1116-
haveRowExpr |= (rexpr && IsA(rexpr, RowExpr));
11171113
rexprs = lappend(rexprs, rexpr);
11181114
if (contain_vars_of_level(rexpr, 0))
11191115
rvars = lappend(rvars, rexpr);
@@ -1123,9 +1119,9 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11231119

11241120
/*
11251121
* ScalarArrayOpExpr is only going to be useful if there's more than one
1126-
* non-Var righthand item. Also, it won't work for RowExprs.
1122+
* non-Var righthand item.
11271123
*/
1128-
if (!haveRowExpr && list_length(rnonvars) > 1)
1124+
if (list_length(rnonvars) > 1)
11291125
{
11301126
List *allexprs;
11311127
Oid scalar_type;
@@ -1141,8 +1137,13 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11411137
allexprs = list_concat(list_make1(lexpr), rnonvars);
11421138
scalar_type = select_common_type(pstate, allexprs, NULL, NULL);
11431139

1144-
/* Do we have an array type to use? */
1145-
if (OidIsValid(scalar_type))
1140+
/*
1141+
* Do we have an array type to use? Aside from the case where there
1142+
* isn't one, we don't risk using ScalarArrayOpExpr when the common
1143+
* type is RECORD, because the RowExpr comparison logic below can cope
1144+
* with some cases of non-identical row types.
1145+
*/
1146+
if (OidIsValid(scalar_type) && scalar_type != RECORDOID)
11461147
array_type = get_array_type(scalar_type);
11471148
else
11481149
array_type = InvalidOid;
@@ -1193,26 +1194,25 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
11931194
Node *rexpr = (Node *) lfirst(l);
11941195
Node *cmp;
11951196

1196-
if (haveRowExpr)
1197+
if (IsA(lexpr, RowExpr) &&
1198+
IsA(rexpr, RowExpr))
11971199
{
1198-
if (!IsA(lexpr, RowExpr) ||
1199-
!IsA(rexpr, RowExpr))
1200-
ereport(ERROR,
1201-
(errcode(ERRCODE_SYNTAX_ERROR),
1202-
errmsg("arguments of row IN must all be row expressions"),
1203-
parser_errposition(pstate, a->location)));
1200+
/* ROW() op ROW() is handled specially */
12041201
cmp = make_row_comparison_op(pstate,
12051202
a->name,
12061203
(List *) copyObject(((RowExpr *) lexpr)->args),
12071204
((RowExpr *) rexpr)->args,
12081205
a->location);
12091206
}
12101207
else
1208+
{
1209+
/* Ordinary scalar operator */
12111210
cmp = (Node *) make_op(pstate,
12121211
a->name,
12131212
copyObject(lexpr),
12141213
rexpr,
12151214
a->location);
1215+
}
12161216

12171217
cmp = coerce_to_boolean(pstate, cmp, "IN");
12181218
if (result == NULL)

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

Lines changed: 49 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)
@@ -217,6 +236,16 @@ order by 1;
217236
(2 rows)
218237

219238
-- Also check row comparison with an indexable condition
239+
explain (costs off)
240+
select thousand, tenthous from tenk1
241+
where (thousand, tenthous) >= (997, 5000)
242+
order by thousand, tenthous;
243+
QUERY PLAN
244+
-----------------------------------------------------------
245+
Index Only Scan using tenk1_thous_tenthous on tenk1
246+
Index Cond: (ROW(thousand, tenthous) >= ROW(997, 5000))
247+
(2 rows)
248+
220249
select thousand, tenthous from tenk1
221250
where (thousand, tenthous) >= (997, 5000)
222251
order by thousand, tenthous;
@@ -249,6 +278,26 @@ order by thousand, tenthous;
249278
999 | 9999
250279
(25 rows)
251280

281+
-- Check row comparisons with IN
282+
select * from int8_tbl i8 where i8 in (row(123,456)); -- fail, type mismatch
283+
ERROR: cannot compare dissimilar column types bigint and integer at record column 1
284+
explain (costs off)
285+
select * from int8_tbl i8
286+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
287+
QUERY PLAN
288+
-------------------------------------------------------------------------------------------------------------
289+
Seq Scan on int8_tbl i8
290+
Filter: (i8.* = ANY (ARRAY[ROW(123::bigint, 456::bigint)::int8_tbl, '(4567890123456789,123)'::int8_tbl]))
291+
(2 rows)
292+
293+
select * from int8_tbl i8
294+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
295+
q1 | q2
296+
------------------+-----
297+
123 | 456
298+
4567890123456789 | 123
299+
(2 rows)
300+
252301
-- Check some corner cases involving empty rowtypes
253302
select ROW();
254303
row

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,17 +95,37 @@ 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)
101106
and unique1 <= 20
102107
order by 1;
103108

104109
-- Also check row comparison with an indexable condition
110+
explain (costs off)
111+
select thousand, tenthous from tenk1
112+
where (thousand, tenthous) >= (997, 5000)
113+
order by thousand, tenthous;
114+
105115
select thousand, tenthous from tenk1
106116
where (thousand, tenthous) >= (997, 5000)
107117
order by thousand, tenthous;
108118

119+
-- Check row comparisons with IN
120+
select * from int8_tbl i8 where i8 in (row(123,456)); -- fail, type mismatch
121+
122+
explain (costs off)
123+
select * from int8_tbl i8
124+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
125+
126+
select * from int8_tbl i8
127+
where i8 in (row(123,456)::int8_tbl, '(4567890123456789,123)');
128+
109129
-- Check some corner cases involving empty rowtypes
110130
select ROW();
111131
select ROW() IS NULL;

0 commit comments

Comments
 (0)
0