Re: btree_gin and btree_gist for enums

On 11/04/2016 04:14 PM, Emre Hasegeli wrote:
>>> Here is a patch to add enum support to btree_gin and btree_gist. I didn't
>>> include distance operations, as I didn't think it terribly important, and
>>> there isn't a simple way to compute it sanely and efficiently, so no KNN
>>> support.
> I don't think we can implement a meaningful distance operator for enums.

Probably.

>
>> This time including the data file for the gist regression tests.
> It doesn't apply to HEAD anymore. I have tested it on b12fd41.

I'll fix the bitrot..

>
> The GiST part of it doesn't really work. The current patch compares
> oids. We need to change it to compare enumsortorder. I could make it
> fail easily:

ouch. Ok,I'll work on this.

>> +ALTER OPERATOR FAMILY gist_enum_ops USING gist ADD
> Why don't we just create it with those changes?

I'll take a look.

>
>> + * Use a similar trick to that used for numeric for enums, since we don't
> Do you mean "similar trick that is used" or "trick similar to numeric"?

This is perfectly legal English. It means "... a similar trick to the
one used for numeric ... ". I'll change it to that if you think it's
clearer.

>
>> + * actually know the leftmost value of any enum without knowing the concrete
>> + * type, so we use a dummy leftmost value of InvalidOid.
>> + return DatumGetBool(
>> + DirectFunctionCall2(enum_gt, ObjectIdGetDatum(*((const Oid *) a)), ObjectIdGetDatum(*((const Oid *) b)))
>> + );
> Wouldn't it be nice to refactor enum_cmp_internal() to accept typcache
> and use it there like the rangetypes?
>

Not sure. Will look.

Thanks for the review.

cheers

andrew