On Fri, Mar 30, 2018 at 6:55 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> On Fri, Mar 30, 2018 at 6:20 PM, Andres Freund <andres@anarazel.de> wrote:
>>> What would the upcasting you have in mind look like?
>>
>> Just use UINT64CONST()? Let's try not to introduce further code that'll
>> need to get painfully fixed.
>
> What I have right now is based on imitating the style that Tom uses.
> I'm pretty sure that I did something like that in the patch I posted
> that became 9563d5b5, which Tom editorialized to be in
> "maintenance_work_mem * 1024L" style. That was only about 2 years ago.
>
> I'll go ahead and use UINT64CONST(), as requested, but I wish that the
> messaging on the right approach to such a standard question of style
> was not contradictory.
BTW, it's not obvious that using UINT64CONST() is going to become the
standard in the future. You may recall that commit 79e0f87a156 fixed a
bug that came from using Size instead of long in tuplesort.c;
tuplesort expects a signed type, since availMem must occasionally go
negative. Noah was not aware of using long for work_mem calculations,
imagining quite reasonable (but incorrectly) that that would break on
Windows, in the process missing this subtle negative availMem
requirement.
The 79e0f87a156 fix changed tuplesort to use int64 (even though it
could have been changed tuplesort back to using long without
consequence instead), which I thought might spread further and
eventually become a coding standard to follow. The point of things
like coding standards around expanding work_mem KB arguments to bytes,
or the MaxAllocSize thing, is that they cover a wide variety of cases
quite well, without much danger. Now, as it happens the Bloom filter
doesn't need to think about something like a negative availMem, so I
could use uint64 (or UINT64CONST()) for the size of the allocation.
But let's not pretend that that doesn't have its own problems. Am I
expected to learn everyone's individual preferences and prejudices
here?
--
Peter Geoghegan
