diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 9ff0eff..ee13fc0 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -363,10 +363,6 @@ initialize_aggregates(AggState *aggstate, * We use a plain Datum sorter when there's a single input column; * otherwise sort the full tuple. (See comments for * process_ordered_aggregate_single.) - * - * In the future, we should consider forcing the - * tuplesort_begin_heap() case when the abbreviated key - * optimization can thereby be used, even when numInputs is 1. */ peraggstate->sortstate = (peraggstate->numInputs == 1) ? diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c index f9a5f7f..869a83b 100644 --- a/src/backend/utils/adt/orderedsetaggs.c +++ b/src/backend/utils/adt/orderedsetaggs.c @@ -266,13 +266,7 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples) osastate->qstate = qstate; osastate->gcontext = gcontext; - /* - * Initialize tuplesort object. - * - * In the future, we should consider forcing the tuplesort_begin_heap() - * case when the abbreviated key optimization can thereby be used, even - * when !use_tuples. - */ + /* Initialize tuplesort object */ if (use_tuples) osastate->sortstate = tuplesort_begin_heap(qstate->tupdesc, qstate->numSortCols, diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index 8ad5635..92f0355 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -336,9 +336,9 @@ struct Tuplesortstate bool markpos_eof; /* saved "eof_reached" */ /* - * The sortKeys variable is used by every case other than the datum and - * hash index cases; it is set by tuplesort_begin_xxx. tupDesc is only - * used by the MinimalTuple and CLUSTER routines, though. + * The sortKeys variable is used by every case other than the hash index + * case; it is set by tuplesort_begin_xxx. tupDesc is only used by the + * MinimalTuple and CLUSTER routines, though. */ TupleDesc tupDesc; SortSupport sortKeys; /* array of length nKeys */ @@ -901,29 +901,41 @@ 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; + /* lookup necessary attributes of the datum type */ + get_typlenbyval(datumType, &typlen, &typbyval); + state->datumTypeLen = typlen; + state->datumTypeByVal = typbyval; + /* Prepare SortSupport data */ - state->onlyKey = (SortSupport) palloc0(sizeof(SortSupportData)); + state->sortKeys = (SortSupport) palloc0(sizeof(SortSupportData)); + + state->sortKeys->ssup_cxt = CurrentMemoryContext; + state->sortKeys->ssup_collation = sortCollation; + state->sortKeys->ssup_nulls_first = nullsFirstFlag; - 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. + * For the datum case, abbreviation is used only for pass-by-reference + * types (in principle abbreviation can be applied for pass-by-value types + * elsewhere). It's convenient to assume that in the pass-by-value datum + * case, ssup.tuple is always NULL. There is no known example of + * pass-by-value types that offer abbreviation, so this seems fine. */ - state->onlyKey->abbreviate = false; + state->sortKeys->abbreviate = !typbyval; - PrepareSortSupportFromOrderingOp(sortOperator, state->onlyKey); + PrepareSortSupportFromOrderingOp(sortOperator, state->sortKeys); - /* lookup necessary attributes of the datum type */ - get_typlenbyval(datumType, &typlen, &typbyval); - state->datumTypeLen = typlen; - state->datumTypeByVal = typbyval; + /* + * 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); @@ -1307,8 +1319,16 @@ tuplesort_putdatum(Tuplesortstate *state, Datum val, bool isNull) SortTuple stup; /* - * If it's a pass-by-reference value, copy it into memory we control, and - * decrease availMem. Then call the common code. + * Pass-by-value types or null values are just stored directly in + * stup.datum1 (and stup.tuple is not used and set to NULL). + * + * Non-null pass-by-reference values need to be copied into memory we + * control, and possibly abbreviated. We decrease availMem as appropriate, + * either way. The copied value is reliably pointed to by stup.tuple and + * is treated as the canonical copy (e.g. it is returned by + * tuplesort_getdatum, or when writing to tape); stup.datum1 stores + * abbreviated values when abbreviation is in play. Otherwise, it's + * identical to stup.tuple. */ if (isNull || state->datumTypeByVal) { @@ -1318,10 +1338,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); @@ -1886,10 +1939,12 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward, } else { + /* use stup.tuple; stup.datum1 may store an abbreviated key */ 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; } @@ -3715,9 +3770,23 @@ 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); + int32 compare; + + compare = ApplySortComparator(a->datum1, a->isnull1, + b->datum1, b->isnull1, + state->sortKeys); + if (compare != 0) + return compare; + + /* When using abbreviation, "tuple" stores original/authoritative value */ + if (state->sortKeys->abbrev_converter) + return ApplySortAbbrevFullComparator(PointerGetDatum(a->tuple), + a->isnull1, + PointerGetDatum(b->tuple), + b->isnull1, + state->sortKeys); + + return 0; } static void @@ -3746,8 +3815,9 @@ 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); }