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 02:53:53
Message-ID: 20201103025353.t7rtngqrn5qjhni6@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v2-0001-Use-INT64_FORMAT-to-print-int64-variables-in-sort-de.patch text/plain 6.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-11-03 02:57:43 Re: Fix typo in xlogreader.h and procarray.c
Previous Message Tomas Vondra 2020-11-03 02:37:43 Re: enable_incremental_sort changes query behavior