Re: [HACKERS] GSoC 2017: Foreign Key Arrays

Hey Joel,

Thanks again for your thorough review!

I was surprised to see <<@ and @>> don't complain when trying to compare
> incompatible types:
> Maybe it's the combination of "anyarray" and "anycompatiblenonarray" that
> is the problem here?

Indeed you are right, to support the correct behaviour we have to use
@>>(anycompatiblearray,
anycompatiblenonarry) and this throws a sanity error in opr_santiy since
the left operand doesn't equal the gin opclass which is anyarray. I am
thinking to solve this we need to add a new opclass under gin "
compatible_array_ops" beside the already existing "array_ops", what do you
think?
@Alvaro your input here would be valuable.

I included the @>>(anycompatiblearray, anycompatiblenonarry) for now as a
fix to the segmentation fault and incompatible data types in v2, the error
messages should be listed correctly as follows:
```sql

select '1'::text <<@ ARRAY[1::integer,2::integer];
ERROR: operator does not exist: text <<@ integer[]
LINE 1: select '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.

select 1::integer <<@ ARRAY['1'::text,'2'::text];
ERROR: operator does not exist: integer <<@ text[]
LINE 1: select 1::integer <<@ ARRAY['1'::text,'2'::text];
HINT: No operator matches the given name and argument types. You might
need to add explicit type casts.

```

doc/src/sgml/func.sgml
> + Does the array contain specified element ?
> * Maybe remove the extra blank space before question mark?

Addressed in v2.

doc/src/sgml/indices.sgml
> -&lt;@ &nbsp; @&gt; &nbsp; = &nbsp; &amp;&amp;
> +&lt;@ @&gt; &nbsp; &lt;&lt;@ @&gt;&gt; &nbsp; = &nbsp; &amp;&amp;
> * To me it looks like the pattern is to insert &nbsp; between each operator

Addressed in v2.

> src/backend/access/gin/ginarrayproc.c
> /* Make copy of array input to ensure it doesn't disappear while
> in use */
> - ArrayType *array = PG_GETARG_ARRAYTYPE_P_COPY(0);
> + ArrayType *array;
> I think the comment above should be changed/moved

Addressed in v2.

> src/backend/utils/adt/arrayfuncs.c
> + /*
> + * We assume that the comparison operator is strict, so a
> NULL can't
> + * match anything. XXX this diverges from the "NULL=NULL"
> behavior of
> + * array_eq, should we act like that?
> + */
> It seems unnecessary to have this open question.

Addressed in v2.

> 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.

Addressed in v2.

+ 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`.

Addressed in v2.

Changelog:
- anyarray_anyelement_operators-v2.patch (compatible with current master
2021-02-12, commit 993bdb9f935a751935a03c80d30857150ba2b645):
* all issues discussed above