On Tue, Sep 2, 2014 at 7:51 PM, Peter Geoghegan <pg@heroku.com> wrote:
> On Tue, Sep 2, 2014 at 12:22 PM, Robert Haas <robertmhaas@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.
Well, the additional code needed in ApplySortComparator would be about
two lines long. Maybe that's going to turn out to be too expensive to
do in all cases, so that we'll end up with ApplySortComparator and
ApplyAbbreviatedSortComparator, but even if we do that seems less
heavyweight than spawning a whole separate object for the tiebreak
case.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company