Re: multiset patch review

Here is a cleanup version of multiset function patch. But all syntax
for multiset has been removed, so the patch just adds additional array
functions and aggregations.

=== The standard-compatible functions ===
- cardinality(anyarray)
- trim_array(anyarray, integer)
=== Renamed version of MULTISET functions ===
- array_trim(anyarray, integer) <= alias to trim_array()
- array_sort(anyarray)
- array_unique(anyarray)
- array_is_unique(anyarray)
- array_union(anyarray, anyarray)
- array_union_all(anyarray, anyarray) <= alias to array_cat()
- array_intersect(anyarray, anyarray)
- array_intersect_all(anyarray, anyarray)
- array_except(anyarray, anyarray)
- array_except_all(anyarray, anyarray)
- array_fusion(SETOF anyarray)
- array_intersection(SETOF anyarray)

On Sat, Feb 5, 2011 at 02:29, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I am still not in favor of adding this syntax.  I'd be in favor of
> adding array_cardinality(), trim_array(), array_sort(), and
> array_flatten().  [It sure is awkward that trim_array has the word
> array second and all of our other array functions have it first - is
> this required by the spec?]

array_flatten() no longer exists. I added array_trim() as an alias
to trim_array() because it would be a FAQ.

> array_to_set() and array_is_set() seem possibly useful, but I probably
> would have called them array_remove_dups() and array_has_dups().  I
> might be in the minority on that one, though.

I prefer array_unique() and array_is_unique(), but will change
if objections.

> I think array_subset(), array_union(), array_intersect(), and
> array_except() are useful, but I think they should just be regular
> functions, without any special syntax.

I removed array_subset(). It might be re-added with index support in
the future. We need more discussions to have different definition of
contains/subset operators (<@ vs. SUBMULTISET OF-like operator) .

All special syntax are removed. I split those set-operation functions
into "all" and "non-all" versions. So, we will have:
array_union[_all], array_intersect[_all], and array_except[_all].

> fusion() and intersection() also seem useful, but maybe it would be
> more consistent to all them array_fusion() and array_intersection().

Renamed. One issue might be array_intersect (normal function) and
array_intersection (aggregate function) have similar names.

On Sat, Feb 12, 2011 at 22:27, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> > I'm not thrilled with called ArrayGetNItems array_cardinality().
>> We must use the name, because it is in the SQL standard.
> If we use the name, then we have to throw an error when it's not a
> single dimension array, since that's what's in the SQL standard.

Hmmm, in my understanding, we can define cardinality()
as "number of elements", that is reasonable for multi-dimensional arrays.

Other functions except cardinality() and trim_array() raise errors
if multi-dimensional arrays are passed. Also, trim_array(), array_sort(),
and array_unique() now keep lower bounds of the input arrays.

> > Should get_type_cache() really live in array_userfuncs.c ?
I think storing cache information in fcinfo is not so general solution.
So, the subroutine is still kept as a static function. Other comments
and variable names you suggested in the revised patch. Thanks.

--
Itagaki Takahiro