On Thu, Sep 11, 2014 at 8:34 PM, Peter Geoghegan <p...@heroku.com> wrote: > On Tue, Sep 9, 2014 at 2:25 PM, Robert Haas <robertmh...@gmail.com> wrote: >>> 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. > > I'm not following here. Isn't that at least equally true of what I've > proposed? Sure, I'm checking "if (!state->abbrevAborted)" first, but > that's irrelevant to the non-abbreviated case. It has nothing to > abort. Also, AFAICT we cannot abort and still call ssup->comparator() > indifferently, since sorttuple.datum1 fields are perhaps abbreviated > keys in half of all cases (i.e. pre-abort tuples), and uninitialized > garbage the other half of the time (i.e. post-abort tuples).
You can if you engineer ssup->comparator() to contain the right pointer at the right time. Also, shouldn't you go back and fix up those abbreviated keys to point to datum1 again if you abort? > Where is the heap_getattr() stuff supposed to happen for the first > attribute to get an authoritative comparison in the event of aborting > (if we're calling ssup->comparator() on datum1 indifferently)? We > decide that we're going to use abbreviated keys within datum1 fields > up-front. When we abort, we cannot use datum1 fields at all (which > doesn't appear to matter for text -- the datum1 optimization has > historically only benefited pass-by-value types). You always pass datum1 to a function. The code doesn't need to care about whether that function is expecting abbreviated or non-abbreviated datums unless that function returns equality. Then it needs to think about calling a backup comparator if there is one. > Is doing all this worth the small saving in memory? Personally, I > don't think that it is, but I defer to you. I don't care about the memory; I care about the code complexity and the ease of understanding that code. I am confident that it can be done better than the patch does it today. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
