Re: Use of "long" in incremental sort code
От | Tomas Vondra |
---|---|
Тема | Re: Use of "long" in incremental sort code |
Дата | |
Msg-id | 20201103214246.qaznz2ac4x73lpee@development обсуждение исходный текст |
Ответ на | Re: Use of "long" in incremental sort code (Tomas Vondra <tomas.vondra@2ndquadrant.com>) |
Ответы |
Re: Use of "long" in incremental sort code
Re: Use of "long" in incremental sort code |
Список | 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
В списке pgsql-hackers по дате отправления: