Re: [HACKERS] path toward faster partition pruning

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: jesper(dot)pedersen(at)redhat(dot)com, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Amit Langote <amitlangote09(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Beena Emerson <memissemerson(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] path toward faster partition pruning
Date: 2018-04-06 12:24:34
Message-ID: 20180406122434.frhvnobms5asau6y@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2018-04-06 12:29:58 Re: Problem while setting the fpw with SIGHUP
Previous Message Andrew Dunstan 2018-04-06 12:00:36 Re: [HACKERS] logical decoding of two-phase transactions