>>>>> "Peter" == Peter Geoghegan <p...@heroku.com> writes:
Peter> Basically, the intersection of the datum sort case with
Peter> abbreviated keys seems complicated.
Not to me. To me it seems completely trivial.
Now, I follow this general principle that someone who is not doing the
work should never say "X is easy" to someone who _is_ doing it, unless
they're prepared to at least outline the solution on request or
otherwise contribute. So see the attached patch (which I will concede
could probably do with more comments, it's a quick hack intended for
illustration) and tell me what you think is missing that would make it a
complicated problem.
Peter> I tended to think that the solution was to force a heaptuple
Peter> sort instead (where abbreviation naturally can be used),
This seems completely wrong - why should the caller have to worry about
this implementation detail? The caller shouldn't have to know about what
types or what circumstances might or might not benefit from
abbreviation.
--
Andrew (irc:RhodiumToad)
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index b6e302f..0dbb277 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -901,30 +901,34 @@ tuplesort_begin_datum(Oid datumType, Oid sortOperator, Oid sortCollation,
state->copytup = copytup_datum;
state->writetup = writetup_datum;
state->readtup = readtup_datum;
+ state->abbrevNext = 10;
state->datumType = datumType;
- /* Prepare SortSupport data */
- state->onlyKey = (SortSupport) palloc0(sizeof(SortSupportData));
-
- state->onlyKey->ssup_cxt = CurrentMemoryContext;
- state->onlyKey->ssup_collation = sortCollation;
- state->onlyKey->ssup_nulls_first = nullsFirstFlag;
- /*
- * Conversion to abbreviated representation infeasible in the Datum case.
- * It must be possible to subsequently fetch original datum values within
- * tuplesort_getdatum(), which would require special-case preservation of
- * original values.
- */
- state->onlyKey->abbreviate = false;
-
- PrepareSortSupportFromOrderingOp(sortOperator, state->onlyKey);
-
/* lookup necessary attributes of the datum type */
get_typlenbyval(datumType, &typlen, &typbyval);
state->datumTypeLen = typlen;
state->datumTypeByVal = typbyval;
+ /* Prepare SortSupport data */
+ state->sortKeys = (SortSupport) palloc0(sizeof(SortSupportData));
+
+ state->sortKeys->ssup_cxt = CurrentMemoryContext;
+ state->sortKeys->ssup_collation = sortCollation;
+ state->sortKeys->ssup_nulls_first = nullsFirstFlag;
+ state->sortKeys->abbreviate = !typbyval;
+
+ PrepareSortSupportFromOrderingOp(sortOperator, state->sortKeys);
+
+ /*
+ * The "onlyKey" optimization cannot be used with abbreviated keys, since
+ * tie-breaker comparisons may be required. Typically, the optimization is
+ * only of value to pass-by-value types anyway, whereas abbreviated keys
+ * are typically only of value to pass-by-reference types.
+ */
+ if (!state->sortKeys->abbrev_converter)
+ state->onlyKey = state->sortKeys;
+
MemoryContextSwitchTo(oldcontext);
return state;
@@ -1318,10 +1322,43 @@ tuplesort_putdatum(Tuplesortstate *state, Datum val, bool isNull)
}
else
{
- stup.datum1 = datumCopy(val, false, state->datumTypeLen);
+ Datum original = datumCopy(val, false, state->datumTypeLen);
stup.isnull1 = false;
- stup.tuple = DatumGetPointer(stup.datum1);
+ stup.tuple = DatumGetPointer(original);
USEMEM(state, GetMemoryChunkSpace(stup.tuple));
+
+ if (!state->sortKeys->abbrev_converter)
+ {
+ stup.datum1 = original;
+ }
+ else if (!consider_abort_common(state))
+ {
+ /* Store abbreviated key representation */
+ stup.datum1 = state->sortKeys->abbrev_converter(original,
+ state->sortKeys);
+ }
+ else
+ {
+ /* Abort abbreviation */
+ int i;
+
+ stup.datum1 = original;
+
+ /*
+ * Set state to be consistent with never trying abbreviation.
+ *
+ * Alter datum1 representation in already-copied tuples, so as to
+ * ensure a consistent representation (current tuple was just handled).
+ * Note that we rely on all tuples copied so far actually being
+ * contained within memtuples array.
+ */
+ for (i = 0; i < state->memtupcount; i++)
+ {
+ SortTuple *mtup = &state->memtuples[i];
+
+ mtup->datum1 = PointerGetDatum(mtup->tuple);
+ }
+ }
}
puttuple_common(state, &stup);
@@ -1887,9 +1924,9 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward,
else
{
if (should_free)
- *val = stup.datum1;
+ *val = PointerGetDatum(stup.tuple);
else
- *val = datumCopy(stup.datum1, false, state->datumTypeLen);
+ *val = datumCopy(PointerGetDatum(stup.tuple), false, state->datumTypeLen);
*isNull = false;
}
@@ -3712,9 +3749,22 @@ readtup_index(Tuplesortstate *state, SortTuple *stup,
static int
comparetup_datum(const SortTuple *a, const SortTuple *b, Tuplesortstate *state)
{
- return ApplySortComparator(a->datum1, a->isnull1,
- b->datum1, b->isnull1,
- state->onlyKey);
+ int compare;
+
+ compare = ApplySortComparator(a->datum1, a->isnull1,
+ b->datum1, b->isnull1,
+ state->sortKeys);
+ if (compare != 0)
+ return compare;
+
+ if (state->sortKeys->abbrev_converter)
+ {
+ compare = ApplySortAbbrevFullComparator(PointerGetDatum(a->tuple), a->isnull1,
+ PointerGetDatum(b->tuple), b->isnull1,
+ state->sortKeys);
+ }
+
+ return compare;
}
static void
@@ -3743,8 +3793,8 @@ writetup_datum(Tuplesortstate *state, int tapenum, SortTuple *stup)
}
else
{
- waddr = DatumGetPointer(stup->datum1);
- tuplen = datumGetSize(stup->datum1, false, state->datumTypeLen);
+ waddr = stup->tuple;
+ tuplen = datumGetSize(PointerGetDatum(stup->tuple), false, state->datumTypeLen);
Assert(tuplen != 0);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
