> > 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.
