Thank you and sorry for the confused comments.
At Mon, 13 Nov 2017 18:46:28 +0900, Amit Langote
<langote_amit...@lab.ntt.co.jp> wrote in
<8460a6c3-68c5-b78a-7d18-d253180f2...@lab.ntt.co.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
