On Tue, Sep 2, 2014 at 12:22 PM, Robert Haas <robertmh...@gmail.com> wrote: > Maybe we should get rid of the tiebreak case altogether: the second > SortSupport object is just containing all the same values as the first > one, with only the comparator being different. Can't we just have > both the abbreviated-comparator and authoritative-comparator as > members of the SortSupport, and call whichever one is appropriate, > instead of copying the whole SortSupport object? That would have the > nice property of avoiding the need for special handling in > reversedirection_heap().
I thought about that. I think that there are other disadvantages to explicitly having a second comparator, associated with a the same sort support state as the authoritative comparator: ApplySortComparator() expects to compare using ssup->comparator(). You'd have to duplicate that for your alternative/abbreviated comparator. It might be to our advantage to use the same ApplySortComparator() inline comparator muliple times in routines like comparetup_heap(), if not for clarity then for performance (granted, that isn't something I have any evidence for, but I wouldn't be surprised if it was noticeable). It might also be to our advantage to have a separate work space. By having a second comparator, you're making the leading key/abbreviated comparison special, rather than the (hopefully) less common tie-breaker case. I find it more logical to structure the code such that the leading/abbreviated comparison is the "regular state", with a tie-breaker on the "irregular"/authoritative state accessed through indirection from the leading key state, reflecting our preference for having most comparisons resolved using cheap abbreviated comparisons. It's not as if I feel that strongly about it, though. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
