Re: Wired if-statement in gen_partprune_steps_internal

On Wed, Apr 7, 2021 at 8:44 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> On Wed, 7 Apr 2021 at 21:53, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> > If canonicalize_qual() had been unable to rewrite that WHERE clause
> > then I could see that we might want to combine steps from other
> > recursive quals. I'm thinking right now that I'm glad
> > canonicalize_qual() does that hard work for us. (I think partprune.c
> > could handle the original WHERE clause as-is in this example
> > anyway...)
>
> I made a pass over the v2 patch and since it's been a long time since
> I'd looked at partprune.c I ended doing further rewriting of the
> comments you'd changed.
>
> There's only one small code change as I didn't like the following:
>
> - return result;
> + /* A single step or no pruning possible with the provided clauses. */
> + return steps ? linitial(steps) : NULL;
>
> I ended up breaking that out into an if condition.
>
> All the other changes are around the comments.
>
> Can you look over this and let me know if you're happy with the changes?

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.

--
Amit Langote
EDB: http://www.enterprisedb.com