Hi,
On 2015-08-06 12:43:06 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> > On 2015-08-06 12:29:15 -0300, Alvaro Herrera wrote:
>
> > > Ah, but that's because you cheated and didn't remove the include from
> > > namespace.h ...
> >
> > Well, it's not included from frontend code, so I didn't see the need?
> > Going through all the backend code and replacing lock.h by lockdefs.h
> > and some other includes doesn't seem particularly beneficial to me.
> >
> > FWIW, removing it from namespace.h is relatively easy. It starts to get
> > a lot more noisy when you want to touch heapam.h.
>
> Ah, heapam.h is the granddaddy of all header messes, isn't it. (Actually
> it's execnodes.h or something like that.)
> > > > diff --git a/src/include/storage/lockdefs.h b/src/include/storage/lockdefs.h
> > > > new file mode 100644
> > > > index 0000000..bfbcdba
> > > > --- /dev/null
> > > > +++ b/src/include/storage/lockdefs.h
> > > > @@ -0,0 +1,56 @@
> > > > +/*-------------------------------------------------------------------------
> > > > + *
> > > > + * lockdefs.h
> > > > + * Frontend exposed parts of postgres' low level lock mechanism
> > > > + *
> > > > + * The split between lockdefs.h and lock.h is not very principled.
> > >
> > > No kidding!
> >
> > Do you have a good suggestion about the split? I wanted to expose the
> > minimal amount necessary, and those were the ones.
>
> Nope, nothing useful, sorry.
To pick up on the general discussion and on the points you made here:
I actually think the split came out to work far better than I'd
anticipated. Having a slimmed-down version of lock.h for code that just
needs to declare/pass lockmode parameters seems to improve our layering
considerably. I got round to Alvaro's and Roberts position that
removing lock.h from namespace.h and heapam.h would be a really nice
improvemen on that front.
Attached is a WIP patch to that end. After the further changes only few
headers still have to include lock.h and they're pretty firmly in the
backend-only territory. It also allows to remove the uglyness of passing
around LOCKMODE as an int in parserOpenTable().
Imo lockdefs.h should be updated to describe that it contains the part
of the lock infrastructure needed by indirect users of lock.h
infrastructure, and that that currently unfortunately may include some
frontend programs.
I *also* think that removing lwlock.h from lock.h would be a good
idea. In my opinion it's a bad idea to pointlessly expose so much
low-level things to the wider world, even if it's only needed by
relatively low level things. Given that dependent macros are in
lwlock.h, it seems pretty sane to move LockHash* there too.
We could additionally move all but LOCKMETHODID, LockTagType,
LockAcquire*(), LockRelease() and probably one or two more to
lock_internals.h (although I'd maybe rather name it lock_details?). I
think it'd be an improvement, although possibly not a tremendous one
given how many places it's likely going to be included from.
Greetings,
Andres Freund
