Re: multiset patch review

On Sat, Feb 12, 2011 at 05:01, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> Input arrays are always flattened into one-dimensional arrays.
> That just strikes me as completely broken when it comes to PG Arrays.

Contains operators (<@, &&, @>) ignore multi-dimensions.
Array slice operator ([lo:hi]) always reset the index offsets.

> What does the spec say about this, if anything?  Is that required by
> spec, or is the spec not relevant to this because MULTISETs are only one
> dimensional..?

Yes. The SQL standard has only one-dimensional ARRAYs and MULTISETs ,
though it supports "collections of collections", that we don't have.

> I would think that we would have a way to define what dimension or piece
> of the array that would be sorted or returned as a set, etc.

It would be consistent if we return (ARRAY[][])[] => ARRAY[],
but we throw errors actually.

> In my view, we should be throwing an error if we get called on a
> multi-dim array and we can't perform the operation on that in an
> obviously correct way, not forcing the input to match something we can
> make work.

Agreed, I'll do so. We could also keep lower-bounds of arrays,
at least on sort.

> I'm not thrilled with called ArrayGetNItems array_cardinality().  Why
> not just provide a function with a name like "array_getnitems()"
> instead?

We must use the name, because it is in the SQL standard.
FUNCTION cardinality(collection) => number
We would have overloaded two versions for ARRAYs andMULTISETs.

> trim_array() suffers from two problems: lack of comments, and no spell
> checking done on those that are there.

What comments do you want?

> Should get_type_cache() really live in array_userfuncs.c ?

Do you find codes using the same operation in other files?

> There's more, primairly lack of comments and what I consider poor
> function naming ("sort_or_unique()" ?  really?),

Could you suggest better names?

--
Itagaki Takahiro