Re: Expand applicability of aggregate's sortop optimization

From: Andrei Lepikhov <lepihov(at)gmail(dot)com>
To: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, David Rowley <dgrowleyml(at)gmail(dot)com>, ilmari(at)ilmari(dot)org
Subject: Re: Expand applicability of aggregate's sortop optimization
Date: 2024-07-19 03:47:02
Message-ID: 40e00c3e-eab9-448c-92a5-65f295885067@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 18/7/2024 19:49, Matthias van de Meent wrote:
> On Wed, 17 Jul 2024 at 16:09, Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
>>
>> On 17/7/2024 16:33, Matthias van de Meent wrote:
>>> On Wed, 17 Jul 2024 at 05:29, Andrei Lepikhov <lepihov(at)gmail(dot)com> wrote:
> Because the @<@ and @>@ operators are not registered as commutative,
> it couldn't apply the optimization in your patch, while the btree
> operator check does allow it to apply the optimization.
Ok, I got it.
And next issue: I think it would be better to save cycles than to free
some piece of memory, so why not to break the foreach cycle if you
already matched the opfamily?
Also, in the patch attached I added your smoothed test to the aggregates.sql
>
> Aside: Arguably, checking for commutator operators would not be
> incorrect when looking at it from "applied operators" point of view,
> but if that commutative operator isn't registered as opposite ordering
> of the same btree opclass, then we'd probably break some assumptions
> of some aggregate's sortop - it could be registered with another
> opclass, and that could cause us to select a different btree opclass
> (thus: ordering) than is indicated to be required by the aggregate;
> the thing we're trying to protect against hereYes, I also think if someone doesn't register < as a commutator to >, it
may mean they do it intentionally: may be it is a bit different
sortings? - this subject is too far from my experience and I can agree
with your approach.

--
regards, Andrei Lepikhov

Attachment Content-Type Size
rewritten-patch-v4.diff text/plain 9.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2024-07-19 04:00:01 Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop()
Previous Message Michael Paquier 2024-07-19 03:17:35 Re: REINDEX not updating partition progress