From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: amitlangote09@gmail.com
Cc: alvherre@2ndquadrant.com
Cc: simon@2ndquadrant.com
Cc: yuzukohosoya@gmail.com
Cc: shawn.wang.pg@gmail.com
Cc: shawn.wang@highgo.ca
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Problem with default partition pruning
Date: Fri, 09 Aug 2019 14:44:38 +0900 (Tokyo Standard Time)
Message-ID: <20190809.144438.221700512.horikyota.ntt@gmail.com> (raw)
In-Reply-To: <CA+HiwqGm18B8UQ5Sip_nsNYmDiHtoaVORvCPumo_bbXTXHPRBw@mail.gmail.com>
References: <20190809.120920.64991939.horikyota.ntt@gmail.com>
<CA+HiwqHDaxHyCHNY7d7nzDFBrNAFzUUfcb=db1DeT_-J1GsfGQ@mail.gmail.com>
<CA+HiwqGm18B8UQ5Sip_nsNYmDiHtoaVORvCPumo_bbXTXHPRBw@mail.gmail.com>
At Fri, 9 Aug 2019 14:02:36 +0900, Amit Langote <amitlangote09@gmail.com> wrote in <CA+HiwqGm18B8UQ5Sip_nsNYmDiHtoaVORvCPumo_bbXTXHPRBw@mail.gmail.com>
> On Fri, Aug 9, 2019 at 1:17 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > On Fri, Aug 9, 2019 at 12:09 PM Kyotaro Horiguchi
> > <horikyota.ntt@gmail.com> wrote:
> > > At Thu, 8 Aug 2019 14:50:54 +0900, Amit Langote wrote:
> > > > When working on it, I realized
> > > > that the way RelOptInfo.partition_qual is processed is a bit
> > > > duplicative, so I created a separate patch to make that a bit more
> > > > consistent.
> > >
> > > 0001 seems reasonable. By the way, the patch doesn't touch
> > > get_relation_constraints(), but I suppose it can use the modified
> > > partition constraint qual already stored in rel->partition_qual
> > > in set_relation_partition_info. And we could move constifying to
> > > set_rlation_partition_info?
> >
> > Ah, good advice. This make partition constraint usage within the
> > planner quite a bit more consistent.
>
> Hmm, oops. I think that judgement was a bit too rushed on my part. I
> unintentionally ended up making the partition constraint to *always*
> be fetched, whereas we don't need it in most cases. I've reverted
(v2 has been withdrawn before I see it:p)
Yeah, I agreed. It is needed only by (sub)partition parents.
> that change. RelOptInfo.partition_qual is poorly named in retrospect.
> :( It's not set for all partitions, only those that are partitioned
> themselves.
>
> Attached updated patches.
+++ b/src/backend/optimizer/util/plancat.c
@@ -1267,10 +1267,14 @@ get_relation_constraints(PlannerInfo *root,
*/
if (include_partition && relation->rd_rel->relispartition)
{
...
+ else
{
+ /* Nope, fetch from the relcache. */
I seems to me that include_partition is true both and only for
modern and old-fasheoned partition parents.
set_relation_partition_info() is currently called only for modern
partition parents. If we need that at the place above,
set_relation_partition_info can be called also for old-fashioned
partition parent, and get_relation_constraints may forget the
else case in a broad way.
+ /* Nope, fetch from the relcache. */
+ List *pcqual = RelationGetPartitionQual(relation);
If the comment above is right, This would be duplicate. What we
should do instaed is only eval_const_expression. And we could
move it to set_relation_partition_info completely. Consitify must
be useful in both cases.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: horikyota.ntt@gmail.com, amitlangote09@gmail.com, alvherre@2ndquadrant.com, simon@2ndquadrant.com, yuzukohosoya@gmail.com, shawn.wang.pg@gmail.com, shawn.wang@highgo.ca, pgsql-hackers@lists.postgresql.org
Subject: Re: Problem with default partition pruning
In-Reply-To: <20190809.144438.221700512.horikyota.ntt@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox