Re: Macro nesting hell - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Macro nesting hell
Date
Msg-id 20150812234355.GB701@awork2.anarazel.de
Whole thread Raw
In response to Re: Macro nesting hell  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Macro nesting hell  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 2015-08-12 10:18:12 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2015-07-01 12:55:48 -0300, Alvaro Herrera wrote:
> >> Tom Lane wrote:
> >>> I'm thinking we really ought to mount a campaign to replace some of these
> >>> macros with inlined-if-possible functions.
>
> >> My guess is that changing a very small amount of them will do a large
> >> enough portion of the job.
>
> > I think it'd be good to convert the macros in bufpage.h and bufmgr.h
> > that either currently have multiple evaluation hazards, or have a
> > AssertMacro() in them. The former for obvious reasons, the latter
> > because that immediately makes them rather large (on and it implies
> > multiple evaluation hazards anyway).
>
> > That'd mean inlining PageSetPageSizeAndVersion(), PageGetSpecialSize(),
> > PageGetSpecialPointer(), PageGetItem(), PageGetMaxOffsetNumber(),
> > PageIsPrunable(), PageSetPrunable(), PageClearPrunable(),
> > BufferIsValid(), BufferGetBlock(), BufferGetPageSize().
>
> Sounds reasonable to me.  If you do this, I'll see whether pademelon
> can be adjusted to build using the minimum macro expansion buffer
> size specified by the C standard.

Here's the patch attached.

There's two more macros on the list that I had missed:
PageXLogRecPtrSet(), PageXLogRecPtrGet(). Unfortunately *Set() requires
to pas a pointer to the PageXLogRecPtr - there's only two callers tho.
We could instead just leave these, or add an indirecting macro. Seems
fine to me though.

With it applied pg compiles without the warning I saw before:
/home/andres/src/postgresql/src/backend/access/brin/brin_pageops.c:778:5: warning: string literal of length 7732
exceedsmaximum length 4095 
      that ISO C99 compilers are required to support [-Woverlength-strings]
                                Assert(BRIN_IS_REGULAR_PAGE(BufferGetPage(oldbuf)));
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We could obviously be more aggressive and convert all the applicable
defines, but they are already readable and have no multiple eval
hazards, so I'm not inclined to that.

Andres

Attachment

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: [COMMITTERS] pgsql: Close some holes in BRIN page assignment
Next
From: "David G. Johnston"
Date:
Subject: Re: count_nulls(VARIADIC "any")