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

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.
>>
>> To wit:
>> divide=# create type the_row as (mfg text, id text);
>> CREATE TYPE
>> divide=# create table the_table (widget the_row);
>>
>>
>> CREATE TABLE
>>
>>
>> divide=# insert into the_table values(row('foo', 'bar')::the_row);
>>
>>
>> INSERT 0 1
>>
>>
>> divide=# insert into the_table values('(bar,baz)'::the_row);
>>
>>
>> INSERT 0 1
>> divide=# select * from the_table;
>>   widget
>> -----------
>>  (foo,bar)
>>  (bar,baz)
>> (2 rows)
>>
>> divide=# select * from the_table where widget in
>> ('(foo,bar)'::the_row);
>>   widget
>> -----------
>>  (foo,bar)
>> (1 row)
>>
>> divide=# select * from the_table where widget in
>> (row('foo','bar')::the_row);
>> ERROR:  arguments of row IN must all be row expressions
>> LINE 1: select * from the_table where widget in (row('foo','bar')::t...
>
> The similar query for equal ('=') operator works fine.
> select * from the_table where widget = (row('foo','bar')::the_row);
>
> The reason for above is that in function transformAExprOp(..), it uses make_row_comparison_op() to operate on
expressionsonly if both left and right are row expressions, else it will use make_op() to operate on expressions. Refer
codebelow in function transformAExprOp() 
> else if (lexpr && IsA(lexpr, RowExpr) &&
>                          rexpr && IsA(rexpr, RowExpr))
>         {
>                     ....
>                 result = make_row_comparison_op(pstate,
>                                                                                 a->name,
>                                                                                 ((RowExpr *) lexpr)->args,
>                                                                                 ((RowExpr *) rexpr)->args,
>                                                                                 a->location);
>         }
>         else
>         {
>                 ....
>                 result = (Node *) make_op(pstate,
>                                                                   a->name,
>                                                                   lexpr,
>                                                                   rexpr,
>                                                                   a->location);
>         }
>
> However for IN clause, if any one expr (left or right) is RowExpr, then it will try to use make_row_comparison_op,
whichresult in error. 
> Refer below code of function transformAExprIn():
> 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)));
>                         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,
>                                                                    copyObject(lexpr),
>                                                                    rexpr,
>                                                                    a->location);
>
> Changing the functionality of transformAExprIn() similar to transformAExprOp() will fix this issue, but not sure if
thereis any other side effect of same. 

Thanks for the analysis! This problem seems to have been introduced in
3d376fce8dd4 [1] (almost eight years ago! I guess not many people use
row types...).

I'm pretty sure the original intent was to afford some extra checks so
that conditions such as "ROW(1, 2) IN ((ROW(3, 4), ROW(5, 6, 7))"
would get rejected at parsing time (CCing the original author; please
confirm).

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,


[1] http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=3d376fce8dd45d43fb6dbeb5a08c08400a589ff8
--
Rafał Rzepecki