On Wed, Apr 7, 2021 at 8:44 PM David Rowley <dgrowle...@gmail.com> wrote: > On Wed, 7 Apr 2021 at 21:53, David Rowley <dgrowle...@gmail.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
v3_amit.diff
Description: Binary data