Re: [HACKERS] path toward faster partition pruning

Thank you and sorry for the confused comments.

At Mon, 13 Nov 2017 18:46:28 +0900, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote in <8460a6c3-68c5-b78a-7d18-d253180f2188(at)lab(dot)ntt(dot)co(dot)jp>
> Horiguchi-san,
>
> Thanks for taking a look. Replying to all your emails here.

> > In 0003,
> >
> > +match_clauses_to_partkey(RelOptInfo *rel,
> > ...
> > + if (rinfo->pseudoconstant &&
> > + (IsA(clause, Const) &&
> > + ((((Const *) clause)->constisnull) ||
> > + !DatumGetBool(((Const *) clause)->constvalue))))
> > + {
> > + *constfalse = true;
> > + continue;
> >
> > If we call this function in both conjunction and disjunction
> > context (the latter is only in recursive case). constfalse ==
> > true means no need of any clauses for the former case.
> >
> > Since (I think) just a list of RestrictInfo is expected to be
> > treated as a conjunction (it's quite doubious, though..),
>
> I think it makes sense to consider a list of RestrictInfo's, such as
> baserestrictinfo, that is passed as input to match_clauses_to_partkey(),
> to be mutually conjunctive for our purpose here.

You're right and I know it. I'm ok to leave it since I recalled
that clause_selectivity always has a similar code.

> On 2017/11/10 14:44, Kyotaro HORIGUCHI wrote:
> > At Fri, 10 Nov 2017 14:38:11 +0900, Kyotaro HORIGUCHI wrote:
> >
> > This is working fine. Sorry for the bogus comment.
>
> I'd almost started looking around if something might be wrong after all. :)

Very sorry for the wrong comment:(

> On 2017/11/10 16:07, Kyotaro HORIGUCHI wrote:
> > At Fri, 10 Nov 2017 14:44:55 +0900, Kyotaro HORIGUCHI wrote:
> >
> >>> Those two conditions are not orthogonal. Maybe something like
> >>> following seems more understantable.
> >>>
> >>>> if (!constfalse)
> >>>> {
> >>>> /* No constraints on the keys, so, return *all* partitions. */
> >>>> if (nkeys == 0)
> >>>> return bms_add_range(result, 0, partdesc->nparts - 1);
> >>>>
> >>>> result = get_partitions_for_keys(relation, &keys);
> >>>> }
> >
> > So, the condition (!constfalse && nkeys == 0) cannot return
> > there. I'm badly confused by the variable name.
>
> Do you mean by 'constfalse'?

Perhaps. The name "constfalse" is suggesting (for me) that the
cluses evaluate to false constantly. But acutally it means just
the not-in-the-return clauses are results in false. Anyway I'll
take a look on v12 and will comment at the time.

> > I couldn't find another reasonable structure using the current
> > classify_p_b_keys(), but could you add a comment like the
> > following as an example?
>
> OK, will add comments explaining what's going on.
>
>
> Will post the updated patches after also taking care of David's and Amul's
> review comments upthread.
>
> Thanks,
> Amit

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center