Hi Jian, On Mon, Sep 30, 2024 at 11:13 PM jian he <jian.universal...@gmail.com> wrote: > > On Mon, Sep 30, 2024 at 1:01 PM Junwang Zhao <zhjw...@gmail.com> wrote: > > > > > I would suggest accepting: > > > asc > > > desc > > > asc nulls first > > > asc nulls last * > > > desc nulls first * > > > desc nulls last > > > > > > As valid inputs for "dir" - and that the starred options are the defaults > > > when null position is omitted. > > > > > > In short, mimic create index. > > > > > > David J. > > > > > > > PFA v3 with David's suggestion addressed. > > > > I think just adding 2 bool arguments (asc/desc, nulls last/not nulls > last) would be easier. Yeah, this would be easier, it's just the intarray module use the direction parameter, I keep it here for the same user experience, I don't insist if some committer thinks 2 bool arguments would be a better option.
> but either way, (i don't have a huge opinion)
> but document the second argument, imagine case
> SELECT array_sort('{a,B}'::text[] , E'aSc NulLs LaST \t\r\n');
> would be tricky?
The case you provide should give the correct results, but
I doubt users will do this.
I'm not good at document wording, so you might give me some help
with the document part.
>
>
> errmsg("multidimensional arrays sorting are not supported")));
> write a sql test to trigger the error message that would be great.
>
> you can add two or one example to collate.icu.utf8.sql to demo that it
> actually works with COLLATE collation_name
> like:
> SELECT array_sort('{a,B}'::text[] COLLATE case_insensitive);
> SELECT array_sort('{a,B}'::text[] COLLATE "C");
>
Fixed.
>
> #define WHITESPACE " \t\n\r"
> you may also check function scanner_isspace
>
Fixed.
>
> + typentry = (TypeCacheEntry *) fcinfo->flinfo->fn_extra;
> + if (typentry == NULL || typentry->type_id != elmtyp)
> + {
> + typentry = lookup_type_cache(elmtyp, sort_asc ? TYPECACHE_LT_OPR :
> TYPECACHE_GT_OPR);
> + fcinfo->flinfo->fn_extra = (void *) typentry;
> + }
> you need to one-time check typentry->lt_opr or typentry->gt_opr exists?
> see CreateStatistics.
> /* Disallow data types without a less-than operator */
> type = lookup_type_cache(attForm->atttypid, TYPECACHE_LT_OPR);
> if (type->lt_opr == InvalidOid)
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("column \"%s\" cannot be used in
> statistics because its type %s has no default btree operator class",
> attname, format_type_be(attForm->atttypid))));
I added an Assert for this part, not sure if that is enough.
--
Regards
Junwang Zhao
v4-0001-general-purpose-array_sort.patch
Description: Binary data
