Amit Langote wrote:
> Hi.
>
> On 2018/04/06 7:35, Alvaro Herrera wrote:
> > I seems pretty clear that putting get_matching_partitions() in
> > catalog/partition.c is totally the wrong thing; it belongs wholly in
> > partprune. I think the reason you put it there is that it requires
> > access to a lot of internals that are static in partition.c. In the
> > attached not yet cleaned version of the patch, I have moved a whole lot
> > of what you added to partition.c to partprune.c; and for the functions
> > and struct declarations that were required to make it work, I created
> > catalog/partition_internal.h.
>
> Yes, I really wanted for most of the new code that this patch adds to land
> in the planner, especially after Robert's comments here:
>
> https://www.postgresql.org/message-id/CA%2BTgmoabi-29Vs8H0xkjtYB%3DcU%2BGVCrNwPz7okpa3KsoLmdEUQ%40mail.gmail.com
>
> It would've been nice if we'd gotten the "reorganizing partitioning code"
> thread resolved sooner.
Grumble.
I don't actually like very much the idea of putting all this code in
optimizer/util. This morning it occurred to me that we should create a new
src/backend/partitioning/ (and a src/include/partitioning/ to go with
it) and drop a bunch of files there. Even your proposed new partcache.c
will seem misplaced *anywhere*, since it contains support code to be
used by both planner and executor; in src/{backend,include}/partitioning
it will be able to serve both without it being a modularity wart.
BTW including partition_internal.h in partition.h would defeat the point
of having partition_internal.h in the first place -- at that point you'd
be better off just putting it all in partition.h and save the hassle of
a separate file. But given the liberty with which catalog/partition.h
has been included everywhere else, IMO that would be pretty disastrous.
I propose to work on reorganizing this code after the commitfest is
over, as part of release stabilization. I'd rather not have us
supporting a messy system for only five years, if we restructure during
pg12 (which would mean a lot of backpatching pain and pg11-specific
bugs); or worse, forever, if we keep the current proposed layout.
One thing I don't want to do is create a new file that we'll later have
to rename or move, so choosing the best locations is a necessity.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
