Hi, On 2019-01-28 15:50:22 -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > On 2019-01-28 15:17:19 -0500, Tom Lane wrote: > >> Since I was intentionally trying to limit what optimizer.h pulls in, > >> and in particular not let it include relation.h, I needed an opaque > >> typedef for PlannerInfo. On the other hand relation.h also needs to > >> typedef that. I fixed that with a method that we've not used in our > >> code AFAIK, but is really common in system headers: there's a #define > >> symbol to remember whether we've created the typedef, and including > >> both headers in either order will work fine. > > > Ugh, isn't it nicer to just use the underlying struct type instead of > > that? > > No, because that'd mean that anyplace relying on optimizer.h would also > have to write "struct PlannerInfo" rather than "PlannerInfo"; the > effects wouldn't be limited to the header itself.
Why? It can be called with the typedef'd version, or not. Unless it calling code has on-stack pointers to it, it ought to never deal with PlannerInfo vs struct PlannerInfo. In a lot of cases the code calling the function will either get the PlannerInfo from somewhere (in which case that'll either have the typedef'd version or not). > > Or alternatively we could expand our compiler demands to require > > that redundant typedefs are allowed - I'm not sure there's any animal > > left that doesn't support that (rather than triggering an error it via > > an intentionally set flag). > > I'd be happy with that if it actually works, but I strongly suspect > that it does not. Or can you cite chapter and verse where C99 > requires it to work? My own compiler complains about "redefinition > of typedef 'foo'". It's not required by C99, it however is required by C11. But a lot of compilers have allowed it as an extension for a long time (like before C99), unless suppressed by some option. I think that's partially because C++ has allowed it for longer. I don't know how many of the BF compilers could be made to accept that - I'd be very suprised if yours couldn't. > >> I would have exposed estimate_rel_size, which is needed by > >> access/hash/hash.c, except that it requires Relation and > >> BlockNumber typedefs. The incremental value from keeping > >> hash.c from using plancat.h probably isn't worth widening > >> optimizer.h's #include footprint further. Also, I wonder > >> whether that whole area needs a rethink for pluggable storage. > > > What kind of rejiggering were you thinking of re pluggable storage? > > I wasn't; I was just thinking that I didn't want to advertise it > as a stable globally-accessible API if it's likely to get whacked > around soon. Ah. So far the signature didn't need to change, and I'm not aware of pending issues requiring it. Greetings, Andres Freund
