Re: [PATCH] ltree hash functions

From: Tommy Pavlicek <tommypav122(at)gmail(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] ltree hash functions
Date: 2023-06-19 09:18:14
Message-ID: CAEhP-W_qmjy7ZwkhCyhbs-fCx_mPoeeK977wSN_fqsMAVJ7Qqg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
> FWIW the CREATE OPERATOR documentation only talks about hash joins for

HASHES, maybe it should be updated to also mention hash aggregates?

I think I might have been a bit unclear here, the hash aggregate does work
without altering the operator so it's just the join that's blocked. Sorry
about the confusion.

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?

I mostly want to add hash indexes. Beyond selecting specific values, you
can use them to get ancestors (trim the path and do an exact select) and
descendents (using a functional index calculating the parent path for each
row). For example, I've found it can be faster to calculate the path of
every ancestor and use select ltree path = ANY([ancestor paths]) compared
to using a gist index. It's not ideal, but unfortunately I've found that
with enough rows, gist indexes get very large and slow. Btree indexes are
better, but for ltree they can still be up to around 10x bigger than a hash
index. I've also seen ltree hash indexes outperform btree indexes in very
large tables, but I suspect in most cases they'll be similar.

Tommy, are you interested in extending ALTER OPERATOR to allow this,
> which would also allow fixing the ltree operator?

Yes, I can do that. I took a look over the code and email thread and it
seems like it should be relatively straight forward. I'll put a patch
together for that and then update this patch to alter the operator.

On Sat, Jun 17, 2023 at 9:57 PM Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
wrote:

>
>
> On 6/17/23 20:19, Tom Lane wrote:
> > Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> writes:
> >> 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?
> >
> > This code was added by commit 321eed5f0. The thread leading up to
> > that commit is here:
> >
> > https://www.postgresql.org/message-id/flat/3348985.V7xMLFDaJO%40dinodell
> >
> > There are some nontrivial concerns in there about breaking the
> > semantics of existing exclusion constraints, for instance. I think
> > we mostly rejected the concern about invalidation of cached plans
> > as already-covered, but that wasn't the only problem.
> >
> > However, I think we could largely ignore the issues if we restricted
> > ALTER OPERATOR to only add commutator, negator, hashes, or merges
> > properties to operators that lacked them before --- which'd be the
> > primary if not only use-case anyway. That direction can't break
> > anything.
> >
>
> Sound reasonable.
>
> Tommy, are you interested in extending ALTER OPERATOR to allow this,
> which would also allow fixing the ltree operator?
>
>
> regards
>
> --
> Tomas Vondra
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2023-06-19 09:21:53 Re: Do we want a hashset type?
Previous Message Michael Banck 2023-06-19 09:01:12 Re: deb’s pg_upgradecluster(1) vs streaming replication