On Thu, Oct 24, 2024 at 8:40 PM jian he <jian.universal...@gmail.com> wrote:
>
> On Wed, Oct 23, 2024 at 10:28 PM Junwang Zhao <zhjw...@gmail.com> wrote:
> > PFA v7 with multi-array support.
> >
>
> if (ARR_NDIM(array) == 1)
> {
> }
> else
> {
> }
> can be simplified.
> i think beginning part of array_sort can be like the following:
> (newline emitted)
>
> ---------------------------------------------------------------------
>     if (ARR_NDIM(array) < 1)
>         PG_RETURN_ARRAYTYPE_P(array);
>     if (dirstr != NULL)
>     {
>         if (!parse_sort_order(text_to_cstring(dirstr), &sort_asc, 
> &nulls_first))
>             ereport(ERROR,
>                     (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>                      errmsg("second parameter must be a valid sort
> direction")));
>     }
>     elmtyp = ARR_ELEMTYPE(array);
>     if (ARR_NDIM(array) > 1)
>         elmtyp = get_array_type(elmtyp);

I'm wondering should we cache the type entry for both element type and
the corresponding array type, for example if we have a table:

create table t(b int[]);
insert into t values ('{1,3}'),('{{2,3}}'),('{{1,2},{0,2}}');

with 1-d array and m-d array interleaved, then the following query will
call lookup_type_cache multiple times.

select array_sort((t.b)) from t;

>     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);
>         if ((sort_asc && !OidIsValid(typentry->lt_opr)) ||
>             (!sort_asc && !OidIsValid(typentry->gt_opr)))
>             ereport(ERROR,
>                     (errcode(ERRCODE_UNDEFINED_FUNCTION),
>                     errmsg("could not identify an ordering operator
> for type %s",
>                             format_type_be(elmtyp))));
>         fcinfo->flinfo->fn_extra = (void *) typentry;
>     }
> ---------------------------------------------------------------------
> /*
>  * array_sort
>  *
>  * Sorts the array in either ascending or descending order.
>  * The array must be empty or one-dimensional.
>  */
> comments need to be updated.

will fix it in the next version of patch.

>
>
> typedef enum
>     PARSE_SORT_ORDER_DONE
> } ParseSortOrderState;
>
> last one, should have comma, like
> "PARSE_SORT_ORDER_DONE, "

will fix it.


-- 
Regards
Junwang Zhao


Reply via email to