Re: Use of "long" in incremental sort code

Hi,

I took another look at this, and 99% of the patch (the fixes to sort
debug messages) seems fine to me. Attached is the part I plan to get
committed, including commit message etc.

The one change I decided to remove is this change in tuplesort_free:

- long spaceUsed;
+ int64 spaceUsed;

The reason why I think this variable should be 'long' is that we're
using it for this:

spaceUsed = LogicalTapeSetBlocks(state->tapeset);

and LogicalTapeSetBlocks is defined like this:

extern long LogicalTapeSetBlocks(LogicalTapeSet *lts);

FWIW the "long" is not introduced by incremental sort - it used to be in
tuplesort_end, the incremental sort patch just moved it to a different
function. It's a bit confusing that tuplesort_updatemax has this:

int64 spaceUsed;

But I'd argue this is actually wrong, and should be "long" instead. (And
this actually comes from the incremental sort patch, by me.)

FWIW while looking at what the other places calling LogicalTapeSetBlocks
do, and I noticed this:

uint64 disk_used = LogicalTapeSetBlocks(...);

in the disk-based hashagg patch. So that's a third data type ...

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services