Re: BUG #8198: ROW() literals not supported in an IN clause

On Wed, Jun 5, 2013 at 7:58 AM, Amit Kapila <amit(dot)kapila(at)huawei(dot)com> wrote:
> On Wednesday, June 05, 2013 5:34 AM Rafał Rzepecki wrote:
>> On Tue, Jun 4, 2013 at 12:35 PM, Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
>> wrote:
>> > On Saturday, June 01, 2013 9:37 PM
>> >
>> >> Row type literals constructed with ROW() cause an error when used in
>> >> an IN clause (string literals casted appropriately are allowed).
>> This
>> >> is especially problematic since many client libraries use these
>> >> literals to pass values of row-type arguments, hence making it
>> >> impossible to use them in IN-clause queries.
>> >>
>>
>> If I'm right, the proper fix would be (patch 0001; caution, not
>> tested):
>>
>> --- a/src/backend/parser/parse_expr.c
>> +++ b/src/backend/parser/parse_expr.c
>> @@ -1203,10 +1203,9 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
>> Node *rexpr = (Node *) lfirst(l);
>> Node *cmp;
>>
>> - if (haveRowExpr)
>> + if (haveRowExpr && IsA(lexpr, RowExpr))
>> {
>> - if (!IsA(lexpr, RowExpr) ||
>> - !IsA(rexpr, RowExpr))
>> + if (!IsA(rexpr, RowExpr))
>> ereport(ERROR,
>>
>> (errcode(ERRCODE_SYNTAX_ERROR),
>> errmsg("arguments of row IN must all
>> be row expressions"),
>>
>>
>> Since the restriction seems a rather arbitrary (at least I fail to see
>> any reason for it), it can be removed altogether (patch 0002, not
>> tested as well):
>>
>> --- a/src/backend/parser/parse_expr.c
>> +++ b/src/backend/parser/parse_expr.c
>> @@ -1203,20 +1203,12 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
>> Node *rexpr = (Node *) lfirst(l);
>> Node *cmp;
>>
>> - if (haveRowExpr)
>> - {
>> - if (!IsA(lexpr, RowExpr) ||
>> - !IsA(rexpr, RowExpr))
>> - ereport(ERROR,
>> -
>> (errcode(ERRCODE_SYNTAX_ERROR),
>> - errmsg("arguments of row IN must
>> all be row expressions"),
>> -
>> parser_errposition(pstate, a->location)));
>> + if (IsA(lexpr, RowExpr) && IsA(rexpr, RowExpr))
>> cmp = make_row_comparison_op(pstate,
>>
>> a->name,
>> (List *)
>> copyObject(((RowExpr *) lexpr)->args),
>>
>> ((RowExpr *) rexpr)->args,
>>
>> a->location);
>> - }
>> else
>> cmp = (Node *) make_op(pstate,
>> a-
>> >name,
>>
>
> I had tried, both your patches have passed all regression tests (tested on Windows). I feel fixing it in a way similar to your Patch-1 would be
> more appropriate as with Patch-1 it can generate meaningful error message for some cases like below:
>
> postgres=# select * from the_table where ROW('abc','def') in (row('foo','bar')::the_row,12);
> ERROR: arguments of row IN must all be row expressions
> LINE 1: select * from the_table where ROW('abc','def') in (row('foo'...

Perhaps you're right, rare cases when you want to do something like
'ROW('abc','def') in (row('foo','bar')::the_row, a_column)' are, I
suppose, so exotic that working around this restriction probably won't
be much of a hassle.
--
Rafał Rzepecki