On Fri, Sep 5, 2014 at 10:45 PM, Peter Geoghegan <pg@heroku.com> wrote:
> While I gave serious consideration to your idea of having a dedicated
> abbreviation comparator, and not duplicating sortsupport state when
> abbreviated keys are used (going so far as to almost fully implement
> the idea), I ultimately decided that my vote says we don't do that. It
> seemed to me that there were negligible benefits for increased
> complexity. In particular, I didn't want to burden tuplesort with
> having to worry about whether or not abbreviation was aborted during
> tuple copying, or was not used by the opclass in the first place -
> implementing your scheme makes that distinction relevant. It's very
> convenient to have comparetup_heap() "compare the leading sort key"
> (that specifically looks at SortTuple.datum1 pairs) indifferently,
> using the same comparator for "abbreviated" and "not abbreviated"
> cases indifferently. comparetup_heap() does not seem like a great
> place to burden with caring about each combination any more than
> strictly necessary.
>
> I like that I don't have to care about every combination, and can
> treat abbreviation abortion as the special case with the extra step,
> in line with how I think of the optimization conceptually. Does that
> make sense?
No. comparetup_heap() is hip-deep in this optimization as it stands,
and what I proposed - if done correctly - isn't going to make that
significantly worse. In fact, it really ought to make things better:
you should be able to set things up so that ssup->comparator is always
the test that should be applied first, regardless of whether we're
aborted or not-aborted or not doing this in the first place; and then
ssup->tiebreak_comparator, if not NULL, can be applied after that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company