BUG #8198: ROW() literals not supported in an IN clause
The following bug has been logged on the website:
Bug reference: 8198
Logged by: Rafal Rzepecki
Email address: divided.mind@gmail.com
PostgreSQL version: 9.2.4
Operating system: Ubuntu 13.04
Description:
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...
^
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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 expressions only if both left and right are row expressions, else it will use make_op() to operate on expressions. Refer code below 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, which result 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 there is any other side effect of same.
With Regards,
Amit Kapila.
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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 expressions only if both left and right are row expressions, else it will use make_op() to operate on expressions. Refer code below 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, which result 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 there is any other side effect of same.
Thanks for the analysis! This problem seems to have been introduced in
3d376fce8dd4 [1]http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=3d376fce8dd45d43fb6dbeb5a08c08400a589ff8 -- Rafał Rzepecki (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
--
Rafał Rzepecki
Attachments:
0001-Only-use-make_row_comparison_op-in-transformAExprIn-.patchapplication/octet-stream; name=0001-Only-use-make_row_comparison_op-in-transformAExprIn-.patchDownload+2-4
0002-Allow-mixing-RowExprs-and-other-expressions-in-the-R.patchapplication/octet-stream; name=0002-Allow-mixing-RowExprs-and-other-expressions-in-the-R.patchDownload+1-10
Import Notes
Reply to msg id not found: 51adc2f4.65cc440a.3921.ffffdd76SMTPIN_ADDED_BROKEN@mx.google.com
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 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'...
With Regards,
Amit Kapila.
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
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 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
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
Import Notes
Reply to msg id not found: 51aed411.08e8440a.5ca2.455eSMTPIN_ADDED_BROKEN@mx.google.com
=?UTF-8?Q?Rafa=C5=82_Rzepecki?= <divided.mind@gmail.com> writes:
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).
No; the reason it was like that was that when that code was written,
make_row_comparison_op was the only way to compare two row values at
all. We didn't have record_eq and friends; nor did we have arrays
of composites.
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):
This is reasonable as far as it goes, but I think it doesn't go far
enough --- there's really no reason anymore to reject RowExprs as
components of ScalarArrayOpExpr either. I've extended this patch
some and committed it. Thanks for the report!
regards, tom lane
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs