Re: Re: Reusing abbreviated keys during second pass of ordered [set] aggregates

On Wed, Dec 9, 2015 at 2:15 PM, Peter Geoghegan <pg@heroku.com> wrote:
> I think that you're missing that patch 0001 formally forbids
> abbreviated keys that are pass-by-value, by revising the contract
> (this is proposed for backpatch to 9.5 -- only comments are changed).
> This is already something that is all but forbidden, although the
> datum case does tacitly acknowledge the possibility by not allowing
> abbreviation to work with the pass-by-value-and-yet-abbreviated case.
>
> I think that this revision is also useful for putting abbreviated keys
> in indexes, something that may happen yet.

I'm also depending on this for the "quicksort for every sort run" patch, BTW:

+           /*
+            * Kludge:  Trigger abbreviated tie-breaker if in-memory tuples
+            * use abbreviation (writing tuples to tape never preserves
+            * abbreviated keys).  Do this by assigning in-memory
+            * abbreviated tuple to tape tuple directly.
+            *
+            * It doesn't seem worth generating a new abbreviated key for
+            * the tape tuple, and this approach is simpler than
+            * "unabbreviating" the memtuple tuple from a "common" routine
+            * like this.
+            */
+           if (state->sortKeys != NULL &&
state->sortKeys->abbrev_converter != NULL)
+               stup->datum1 = state->memtuples[state->current].datum1;

I could, as an alternative approach, revise tuplesort so that
self-comparison works (something we currently assert against [1]),
something that would probably *also* require and update to the
sortsupport.h contract, but this seemed simpler and more general.

In general, I think that there are plenty of reasons to forbid
pass-by-reference abbreviated keys (where the abbreviated comparator
itself is a pointer, something much more complicated than an integer
3-way comparison or similar).

[1] Commit c5a03256c
-- 
Peter Geoghegan