On Tue, Nov 25, 2014 at 10:38 AM, Peter Geoghegan <pg@heroku.com> wrote:
>> - Also, I don't think making abbrev_state an enumerated value with two
>> values is really doing anything for us; we could just use a Boolean.
>> I'm wondering if we should actually go a bit further and remove this
>> from the SortSupport object and instead add an additional Boolean flag
>> to PrepareSortSupportFrom(OrderingOp|IndexRel) that gets passed all
>> the way down to the opclass's sortsupport function. It seems like
>> that might be more clear. Once the opclass function has done its
>> thing, the other three new nembers are enough to know whether we're
>> using the optimization or not (and can be fiddled if we want to make a
>> later decision to call the whole thing off).
>
> I'm not sure about that. I'd prefer to have tuplesort (and one or two
> other sites) set the "abbreviation is possible in principle" flag.
As for the related question of whether or not there should just be a
bool in place of an abbreviation state enum: I thought that we might
add some more flags to that enum (you'll recall that there actually
was another flag in earlier revisions, relating to optimistic
tie-breaks with memcmp() that the master branch now always does
anyway). But come to think of it, I think it's very unlikely that that
flag will ever be extended to represent some now unforeseen state
regarding abbreviation. It's either going to be "abbreviation
applicable for this sort and this attribute" or "not applicable". So,
yes, let's make it a boolean instead.
As I think I mentioned at one point, I imagine that if and when we do
abbreviation with the numeric opclass, it won't ever abort - its
encoding strategy will adapt. That ought to be possible for certain
other datatypes, of which numeric is the best example. Although, I
think it's only going to be a handful of the most important datatypes
that get abbreviation support.
--
Peter Geoghegan