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