On Tue, Jan 20, 2015 at 5:42 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Jan 20, 2015 at 8:39 PM, Peter Geoghegan <pg@heroku.com> wrote:
>> On Tue, Jan 20, 2015 at 5:32 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> I was assuming we were going to fix this by undoing the abbreviation
>>> (as in the abort case) when we spill to disk, and not bothering with
>>> it thereafter.
>>
>> The spill-to-disk case is at least as compelling at the internal sort
>> case. The overhead of comparisons is much higher for tapesort.
>
> First, we need to unbreak this. Then, we can look at optimizing it.
> The latter task will require performance testing.
I don't see that any alternative isn't a performance trade-off. My
patch accomplishes unbreaking this. I agree that it needs still needs
review from that perspective, but it doesn't seem any worse than any
other alternative. Would you prefer it if the spill-to-disk case
aborted in the style of low entropy keys? That doesn't seem
significantly safer than this, and it certainly not acceptable from a
performance perspective.
--
Peter Geoghegan