Re: Raising our compiler requirements for 9.6

From: Andres Freund <andres(at)anarazel(dot)de>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Raising our compiler requirements for 9.6
Date: 2015-08-12 07:53:20
Message-ID: 20150812075320.GD28835@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2015-08-08 02:30:44 -0400, Noah Misch wrote:
> On Sat, Aug 08, 2015 at 02:30:47AM +0200, Andres Freund wrote:
> > On 2015-08-07 20:16:20 -0400, Noah Misch wrote:
> > > I agree that lock.h offers little to frontend code. Headers that the
> > > lockdefs.h patch made usable in the frontend, particularly genam.h and hash.h,
> > > are no better.
> >
> > It's not that simple. Those two, and tuptoaster.h, are actually somewhat
> > validly included by frontend code via the rmgr descriptor routines.
>
> genam.h is a dependency of the non-frontend-relevant content of some
> frontend-relevant headers, _exactly_ as lock.h has been. I count zero things
> in genam.h that a frontend program could harness. The frontend includes
> hash.h for two hashdesc.c prototypes, less than the material you moved out of
> lock.h for frontend benefit. Yes, it is that simple.

The point is that it's included from various headers and that there's
really no need to include lock.h from a header that just wants to
declare a LOCKMODE as it's parameter. There's no other contents in
lock.h afaics that fit the billet similarly. To me it's a rather
worthwhile goald to want *am.h not to pull in details of the locking
implementations, but they're going to have to define a LOCKMODE
parameter and the values passed in. We're not entirely there, but it's
not much further work.

We could split some stuff in a more 'locking internals' oriented headers
(PROC_QUEUE, LockMethodData and dependants, LOCKTAG and depenedants),
but afaics it'd not be a proper lock_internal.h header either, because
there's code like index.c that currently does SET_LOCKTAG_RELATION
itself.

> > > The lock.h/lockdefs.h unprincipled split would do more harm
> > > than letting frontends continue to pull in lock.h.
> >
> > Why?
>
> Your header comment for lockdefs.h sums up the harm nicely. Additionally, the
> term "defs" does nothing to explain the split. "lock2.h" would be no less
> evocative.

Oh, come on. It's not the only header that's split that way (see
xlogdefs.h). Splitting away some key definitions so not all the
internals are dragged in when needing just the simplest definitions is
not an absurd concept.

> > Consider what happens when lock.h/c gets more complicated and
> > e.g. grows some atomics. None of the frontend code should see that, and
> > it's not much effort to keep it that way. Allowing client code to see
> > LOCKMODE isn't something that's going to cause any harm.
>
> Readying the headers for that day brings some value, but you added a worse
> mess to achieve it. The overall achievement has negative value.

I honestly can't follow. Why is it so absurd to avoid including lock.h
from more code, including FRONTEND one? Or is it just the lockdefs.h
split along the 'as-currently-required' line that you object to?

Greetings,

Andres Freund

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2015-08-12 08:02:57 Re: patch : Allow toast tables to be moved to a different tablespace
Previous Message Simon Riggs 2015-08-12 07:39:26 Re: Test code is worth the space