Mop-up for the bootstrap data conversion patch

Started by Tom Laneabout 8 years ago3 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

Those of you who've been paying attention to the bootstrap data conversion
thread will know that one of the key ideas is to put everything in the
catalog headers that's of direct use to client-side code into separate
"pg_foo_d.h" headers. This allows clients to include pg_foo_d.h to get
OID macros or whatever else they need, and not have to worry about whether
the main pg_foo.h header contains backend-only declarations.

Attached is a patch that changes all our frontend code to actually do
things that way. I propose to push this after the bootstrap conversion
proper.

In the wake of these changes, it'd be possible to reverse the changes
we've made to move extern declarations for backend/catalog/pg_foo.c
functions into separate pg_foo_fn.h headers, which we've done whenever
(a) such externs required backend-only typedefs and (b) we really needed
pg_foo.h to be includable by frontend code. So it's time to make a
decision whether we want to do that, or leave well enough alone.

I've always felt that the pg_foo_fn.h business was a kluge, and would
be happy to get rid of it. But one could also argue that it would be
a good design, if we adopted it uniformly instead of haphazardly.
But that'd require more code churn, and there's no longer a lot to be
gained thereby.

Another approach worth discussing is "let's do that, but wait a release
or two to give third-party clients time to adjust to using pg_foo_d.h".
I'm not excited by that though. I think third parties will just wait to
change till they're forced to, so this wouldn't really reduce the pain
only delay it. Plus I don't know that we'd ever get around to it;
our track record for doing cleanup things later isn't great.

So I feel that we should either strike while the iron is hot, and
re-merge the pg_foo_fn.h headers now, or decide that we're never
going to do that and we'll just let them be as-is.

Thoughts?

regards, tom lane

PS: forgot to mention: this patch also gets rid of ecpglib/pg_type.h
entirely, as it's now just a confusingly-named wrapper for pg_type_d.h.

Attachments:

bootstrap-client-fixes-1.patchtext/x-diff; charset=us-ascii; name=bootstrap-client-fixes-1.patchDownload+41-57
#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Tom Lane (#1)
Re: Mop-up for the bootstrap data conversion patch

Tom Lane wrote:

I've always felt that the pg_foo_fn.h business was a kluge, and would
be happy to get rid of it. But one could also argue that it would be
a good design, if we adopted it uniformly instead of haphazardly.
But that'd require more code churn, and there's no longer a lot to be
gained thereby.

I vote for removing pg_foo_fn.h.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#3Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#2)
Re: Mop-up for the bootstrap data conversion patch

On Sat, Apr 07, 2018 at 09:25:44AM -0300, Alvaro Herrera wrote:

Tom Lane wrote:

I've always felt that the pg_foo_fn.h business was a kluge, and would
be happy to get rid of it. But one could also argue that it would be
a good design, if we adopted it uniformly instead of haphazardly.
But that'd require more code churn, and there's no longer a lot to be
gained thereby.

I vote for removing pg_foo_fn.h.

+1 on that.
--
Michael