Re: [HACKERS] A design for amcheck heapam verification

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] A design for amcheck heapam verification
Date: 2018-03-31 02:37:38
Message-ID: CAH2-Wz=fVewc5QPwQQyMr1RzKt-WvtEZKW0RZXM5CC_SkVoZGQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 30, 2018 at 6:55 PM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> On Fri, Mar 30, 2018 at 6:20 PM, Andres Freund <andres(at)anarazel(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2018-03-31 05:38:37 Re: [HACKERS][PATCH] adding simple sock check for windows
Previous Message Andres Freund 2018-03-31 02:04:19 Re: [HACKERS] A design for amcheck heapam verification