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