Re: [PATCH] ltree hash functions

Hi,

I've created a CF entry for the patch:

https://commitfest.postgresql.org/43/4375/

I only briefly skimmed the code, so a couple comments.

On 6/17/23 17:45, Tommy Pavlicek wrote:
> Hi All,
>
> I've written a patch to add hash functions for the ltree extension. It
> adds support for hash indexes and hash aggregation. I've reused the
> existing logic that's used to hash arrays and added tests that mirror
> elsewhere (i.e. hstore and hash_func regression tests).
>

Reusing code/logic is the right approach, IMHO.

> The patch doesn't currently support hash joins as the ltree = operator
> was created without support for it. The ALTER OPERATOR command doesn't
> support changing the hash join support, so I'm not sure what the best
> strategy to change it is. Is it ok to update the operator's row in the
> pg_operator system catalog or is there a better way to change this that
> someone could recommend?
>

I guess the "correct" solution would be to extend ALTER OPERATOR. I
wonder why it's not supported - it's clearly an intentional decision
(per comment in AlterOperator). So what might break if this changes for
an existing operator?

FWIW the CREATE OPERATOR documentation only talks about hash joins for
HASHES, maybe it should be updated to also mention hash aggregates?

> Any comments on the overall approach or other feedback would be appreciated.
>

I wonder what's the use case for this. I wonder how often people join on
ltree, for example. Did you just notice ltree can't hash and decided to
fix that, or do you have a practical use case / need for this feature?

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company