Re: Considering additional sort specialisation functions for PG16

From: John Naylor <john(dot)naylor(at)enterprisedb(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Subject: Re: Considering additional sort specialisation functions for PG16
Date: 2023-01-30 10:32:04
Message-ID: CAFBsxsFdFpzyBekxxkiA4vXnLpw-wcaQXz=EAP4pzkZMo91-MA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:

> On Thu, Jan 26, 2023 at 7:15 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> > I think the slower sorts I found in [2] could also be partially caused
> > by the current sort specialisation comparators re-comparing the
> > leading column during a tie-break. I've not gotten around to disabling
> > the sort specialisations to see if and how much this is a factor for
> > that test.
>
> Right, that's worth addressing independently of the window function
consideration. I'm still swapping this area back in my head, but I believe
one issue is that state->base.onlyKey signals two things: "one sortkey, not
abbreviated". We could add a separate branch for "first key unabbreviated,
nkeys>1" -- I don't think we'd need to specialize, just branch -- and
instead of state->base.comparetup, call a set of analogous functions that
only handle keys 2 and above (comparetup_tail_* ? or possibly just add a
boolean parameter compare_first). That would not pose a huge challenge, I
think, since they're already written like this:
>
> /* Compare the leading sort key */
> compare = ApplySortComparator(...);
> if (compare != 0)
> return compare;
>
> /* Compare additional sort keys */
> ...
>
> The null/non-null separation would eliminate a bunch of branches in
inlined comparators, so we could afford to add another branch for number of
keys.

I gave this a go, and it turns out we don't need any extra branches in the
inlined comparators -- the new fallbacks are naturally written to account
for the "!onlyKey" case. If the first sortkey was abbreviated, call its
full comparator, otherwise skip to the next sortkey (if there isn't one, we
shouldn't have gotten here). The existing comparetup functions try the
simple case and then call the fallback (which could be inlined for them but
I haven't looked).

Tests pass, but I'm not sure yet if we need more tests. I don't have a
purpose-built benchmark at the moment, but I'll see if any of my existing
tests exercise this code path. I can also try the window function case
unless someone beats me to it.

--
John Naylor
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v1-0001-Split-out-fallback-functionality-from-comparetup-.patch text/x-patch 11.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2023-01-30 10:32:52 Re: Deadlock between logrep apply worker and tablesync worker
Previous Message Maxim Orlov 2023-01-30 10:22:31 Re: Add SHELL_EXIT_CODE to psql