>On Thu, Feb 11, 2021, at 16:50, Mark Rofail wrote:
>> Here comes a first review of the anyarray_anyelement_operators-v1.patch.
>Great, thanks! I’ll start applying your comments today and release a new patch.
Here comes some more feedback:
I was surprised to see <<@ and @>> don't complain when trying to compare
incompatible types:
regression=# select '1'::text <<@ ARRAY[1::integer,2::integer];
?column?
----------
f
(1 row)
I would expect the same result as if using the <@ and @> operators,
and wrapping the value in an array:
regression=# select ARRAY['1'::text] <@ ARRAY[1::integer,2::integer];
ERROR: operator does not exist: text[] <@ integer[]
LINE 1: select ARRAY['1'::text] <@ ARRAY[1::integer,2::integer];
^
HINT: No operator matches the given name and argument types. You might need to
add explicit type casts.
The error above for the existing <@ operator is expected,
and I think the <<@ should give a similar error.
Even worse, when using integer on the left side and text in the array,
the <<@ operator causes a seg fault:
regression=# select 1::integer <<@ ARRAY['1'::text,'2'::text];
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
2021-02-11 22:18:53.083 CET [91680] LOG: server process (PID 1666) was
terminated by signal 11: Segmentation fault: 11
2021-02-11 22:18:53.083 CET [91680] DETAIL: Failed process was running: select
1::integer <<@ ARRAY['1'::text,'2'::text];
2021-02-11 22:18:53.083 CET [91680] LOG: terminating any other active server
processes
2021-02-11 22:18:53.084 CET [1735] FATAL: the database system is in recovery
mode
2021-02-11 22:18:53.084 CET [91680] LOG: all server processes terminated;
reinitializing
2021-02-11 22:18:53.092 CET [1736] LOG: database system was interrupted; last
known up at 2021-02-11 22:14:41 CET
2021-02-11 22:18:53.194 CET [1736] LOG: database system was not properly shut
down; automatic recovery in progress
2021-02-11 22:18:53.197 CET [1736] LOG: redo starts at 0/2BCA5520
2021-02-11 22:18:53.197 CET [1736] LOG: invalid record length at 0/2BCA5558:
wanted 24, got 0
2021-02-11 22:18:53.197 CET [1736] LOG: redo done at 0/2BCA5520 system usage:
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s
2021-02-11 22:18:53.207 CET [91680] LOG: database system is ready to accept
connections
Maybe it's the combination of "anyarray" and "anycompatiblenonarray" that is
the problem here?
Some more comments on the code:
array_contains_elem(AnyArrayType *array, Datum elem,
+ /*
+ * Apply the comparison operator to each pair of array elements.
+ */
This comment has been copy/pasted from array_contain_compare().
Maybe the wording should clarify there is only one array in this function,
the word "pair" seems to imply working with two arrays.
+ for (i = 0; i < nelems; i++)
+ {
+ Datum elt1;
The name `elt1` originates from the array_contain_compare() function.
But since this function, array_contains_elem(), doesn't have a `elt2`,
it would be better to use `elt` as a name here. The same goes for `it1`.
/Joel
