On Thu, Apr 8, 2021 at 5:34 PM David Rowley <dgrowleyml@gmail.com> wrote:
> On Thu, 8 Apr 2021 at 00:49, Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > Thanks David. Actually, I was busy updating the patch to revert to
> > gen_partprune_steps_internal() returning a list and was almost done
> > with it when I saw your message.
> >
> > I read through v3 and can say that it certainly looks better than v2.
> > If you are happy with gen_partprune_steps_internal() no longer
> > returning a list, I would not object if you wanted to go ahead and
> > commit the v3.
> >
> > I've attached the patch I had ended up with and was about to post as
> > v3, just in case you wanted to glance.
>
> Thanks. I've made a pass over that and just fixed up the places that
> were mixing up NIL and NULL.
>
> I applied most of my comments from my last version after adapting them
> to account for the variation in the functions return value. I also did
> a bit more explaining about op steps and combine steps in the header
> comment for gen_partprune_steps_internal.
Thanks for updating the patch.
+ * These partition pruning steps come in 2 forms; operation steps and combine
+ * steps.
Maybe you meant "operator" steps? IIRC, the reason why we named it
PartitionPruneStepOp is that an op step is built to prune based on the
semantics of the operators that were involved in the matched clause.
Although, they're abused for pruning based on nullness clauses too.
Maybe, we should also updated the description of node struct as
follows to consider that last point:
* PartitionPruneStepOp - Information to prune using a set of mutually ANDed
* OpExpr and any IS [ NOT ] NULL clauses
+ * Combine steps (PartitionPruneStepCombine) instruct the partition pruning
+ * code how it should produce a single set of partitions from multiple input
+ * operation steps.
I think the last part should be: ...from multiple operation/operator
and [ other ] combine steps.
If that sounds fine, likewise adjust the following sentences in the
same paragraph.
--
Amit Langote
EDB: http://www.enterprisedb.com