On Wed, Oct 2, 2024 at 9:51 AM jian he <jian.universal...@gmail.com> wrote: > > > > > > > + 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. > > > > i think it really should be: > > 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; > if ((sort_asc && !OidIsValid(typentry->lt_opr) || (!sort_as && > OidIsValid(typentry->gt_opr)); > ereport(ERROR,....) > } > > Imagine a type that doesn't have TYPECACHE_LT_OPR or TYPECACHE_GT_OPR > then we cannot do the sort, we should just error out. > > I just tried this colour type [1] with (CREATE TYPE colour (INPUT = > colour_in, OUTPUT = colour_out, LIKE = pg_catalog.int4); > > select array_sort('{#FF0000, #FF0000}'::colour[]); > of course it will segfault with your new Assert. > > > [1] https://github.com/hlinnaka/colour-datatype/blob/master/colour.c
Make sense, PFA v5 with Jian's suggestion. -- Regards Junwang Zhao
v5-0001-general-purpose-array_sort.patch
Description: Binary data