Re: ScalarArrayOpExpr and multi-dimensional arrays

[Amit Langote]

On 2017/12/08 23:34, Tom Lane wrote:
> Amit Langote <langote_amit...@lab.ntt.co.jp> writes:
>> I wonder if ScalarArrayOpExpr is not really meant for multi-dimensional
>> arrays appearing on the right hand side?  Because:
>> # select array[1] = any (array[array[1], array[2]]);
> 
>> ERROR:  operator does not exist: integer[] = integer
> 
> You are falling into the misimpression that a 2-D array is an array of
> 1-D arrays.  It is not, even if the syntax makes it look like that.
> 
> ScalarArrayOpExpr just iterates over the array elements without regard
> to dimensionality; so the LHS must be of the element type.

Yeah, I can now see that.

Although, I wonder if there is any room for improvement here.  Instead of
waiting for make_scalar_array_op() to emit the error as it does today,
would it be better if we error'd out earlier saying "ERROR: ANY/ALL
leftarg must be scalar, not array"?  Attached a patch for that, if it's
worth going for at all.

Thanks,
Amit

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 29f9da796f..c179d297af 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -958,6 +958,12 @@ transformAExprOpAny(ParseState *pstate, A_Expr *a)
                                                                 a->location);
 
        lexpr = transformExprRecurse(pstate, lexpr);
+       if (type_is_array(exprType(lexpr)))
+               ereport(ERROR,
+                               (errcode(ERRCODE_DATATYPE_MISMATCH),
+                                errmsg("ANY/ALL leftarg must be be scalar, not 
array"),
+                                parser_errposition(pstate, 
exprLocation(lexpr))));
+
        rexpr = transformExprRecurse(pstate, rexpr);
 
        return (Node *) make_scalar_array_op(pstate,
@@ -981,6 +987,12 @@ transformAExprOpAll(ParseState *pstate, A_Expr *a)
                                                                 a->location);
 
        lexpr = transformExprRecurse(pstate, lexpr);
+       if (type_is_array(exprType(lexpr)))
+               ereport(ERROR,
+                               (errcode(ERRCODE_DATATYPE_MISMATCH),
+                                errmsg("ANY/ALL leftarg must be scalar, not 
array"),
+                                parser_errposition(pstate, 
exprLocation(lexpr))));
+
        rexpr = transformExprRecurse(pstate, rexpr);
 
        return (Node *) make_scalar_array_op(pstate,
diff --git a/src/test/regress/expected/arrays.out 
b/src/test/regress/expected/arrays.out
index c730563f03..c127ffaac0 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -1295,6 +1295,13 @@ SELECT -1 != ALL(ARRAY(SELECT NULLIF(g.i, 900) FROM 
generate_series(1,1000) g(i)
  
 (1 row)
 
+-- leftarg must always be scalar
+select '{1}'::int[] = any ('{{1}, {2}}');
+ERROR:  ANY/ALL leftarg must be be scalar, not array
+LINE 1: select '{1}'::int[] = any ('{{1}, {2}}');
+               ^
+create table arr_tbl_check (a int[] check (a = any ('{{1}, {2}}')));
+ERROR:  ANY/ALL leftarg must be be scalar, not array
 -- test indexes on arrays
 create temp table arr_tbl (f1 int[] unique);
 insert into arr_tbl values ('{1,2,3}');
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 25dd4e2c6d..d5ced0f695 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -374,6 +374,9 @@ select 33 = all ('{1,null,3}');
 select 33 = all ('{33,null,33}');
 -- nulls later in the bitmap
 SELECT -1 != ALL(ARRAY(SELECT NULLIF(g.i, 900) FROM generate_series(1,1000) 
g(i)));
+-- leftarg must always be scalar
+select '{1}'::int[] = any ('{{1}, {2}}');
+create table arr_tbl_check (a int[] check (a = any ('{{1}, {2}}')));
 
 -- test indexes on arrays
 create temp table arr_tbl (f1 int[] unique);