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

On Wed, Jun 5, 2013 at 7:58 AM, Amit Kapila <amit.kapila@huawei.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.kapila@huawei.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
similarto 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