On 2019-Jan-13, Andres Freund wrote:
> On 2019-01-13 23:54:58 -0300, Alvaro Herrera wrote:
> > On 2019-Jan-13, Andres Freund wrote:
> >
> > > Alvaro, you'd introduced the split of HeapScanDesc and HeapScanDescData
> > > being in different files (in a3540b0f657c6352) - what do you think about
> > > this change? I didn't revert that split, but I think we ought to
> > > consider just relying on a forward declared struct in heapam.h instead,
> > > this split is pretty confusing and seems to lead to more interdependence
> > > in a lot of cases.
> >
> > I wasn't terribly happy with that split, so I'm not opposed to doing
> > things differently. For your consideration, I've had this patch lying
> > around for a few years, which (IIRC) reduces the exposure of heapam.h by
> > splitting relscan.h in two. This applies on top of dd778e9d8884 (and as
> > I recall it worked well there).
>
> You forgot to attach that patch... :).
Oops :-( Here it is anyway. Notmuch reminded me that I had posted this
before, to a pretty cold reception:
https://postgr.es/m/20130917020228.GB7139@eldon.alvh.no-ip.org
Needless to say, I disagree with the general sentiment in that thread
that header refactoring is pointless and unwelcome.
> I'm not sure I see a need to split relscan - note my patch makes it so
> that it's not included by heapam.h anymore, and doing for the same for
> genam.h would be fairly straightforward. The most interesting bit there
> would be whether we'd add the includes necessary for Snapshot (imo no),
> Relation (?), ScanKey (imo no), or whether to add the necessary includes
> directly.
Ah, you managed to get heapam.h and genam.h out of execnodes.h, which I
think was my main motivation ... that seems good enough to me. I agree
that splitting relscan.h may not be necessary after these changes.
As for struct Relation, note that for that one you only need relcache.h
which should be lean enough, so it doesn't sound too bad to include that
one wherever.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
