Re: Use of "long" in incremental sort code

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: "Tang, Haiying" <tanghy(dot)fnst(at)cn(dot)fujitsu(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, James Coleman <jtc331(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Use of "long" in incremental sort code
Date: 2020-11-03 21:42:46
Message-ID: 20201103214246.qaznz2ac4x73lpee@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 03, 2020 at 03:53:53AM +0100, Tomas Vondra wrote:
>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.
>

I've pushed this part. Thanks for the patch, Haiying Tang.

>
>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 ...
>

IMHO this should simply switch the current int64 variable to long, as it
was before. Not sure about about the hashagg uint64 variable.

regards

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2020-11-03 21:48:24 Re: Collation versioning
Previous Message Tomas Vondra 2020-11-03 21:39:42 Re: enable_incremental_sort changes query behavior