On Tue, Jan 20, 2015 at 6:39 PM, Peter Geoghegan <p...@heroku.com> wrote:
> On Tue, Jan 20, 2015 at 6:34 PM, Robert Haas <robertmh...@gmail.com> wrote:
>> That might be OK. Probably needs a bit of performance testing to see
>> how it looks.
>
> Well, we're still only doing it when we do our final merge. So that's
> "only" doubling the number of conversions required, which if we're
> blocked on I/O might not matter at all.
You'll probably prefer the attached. This patch works by disabling
abbreviation, but only after writing out runs, with the final merge
left to go. That way, it doesn't matter when abbreviated keys are not
read back from disk (or regenerated).
I believe this bug was missed because it only occurs when there are
multiple runs, and not in the common case where there is one big
initial run that is found already sorted when we reach mergeruns().
--
Peter Geoghegan
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 6d3aa88..b6e302f 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -2172,6 +2172,22 @@ mergeruns(Tuplesortstate *state)
return;
}
+ if (state->sortKeys->abbrev_converter)
+ {
+ /*
+ * If there are multiple runs to be merged, when we go to read back
+ * tuples from disk, abbreviated keys will not have been stored, and we
+ * don't care to regenerate them. Disable abbreviation from this point
+ * on.
+ */
+ state->sortKeys->abbrev_converter = NULL;
+ state->sortKeys->comparator = state->sortKeys->abbrev_full_comparator;
+
+ /* Not strictly necessary, but be tidy */
+ state->sortKeys->abbrev_abort = NULL;
+ state->sortKeys->abbrev_full_comparator = NULL;
+ }
+
/* End of step D2: rewind all output tapes to prepare for merging */
for (tapenum = 0; tapenum < state->tapeRange; tapenum++)
LogicalTapeRewind(state->tapeset, tapenum, false);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
