On 6 February 2018 at 16:11, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Thanks for the explanation. I'm fine with it. Well may I make
> some comments on the patch?
Sure, always welcome.
>
> - It seems to me that the if (!should_warp_around) block and else
> block are better be transposed so that make the bail out code
> closer to the exit condition check as the previous code
> was. (In other was the second block should be if
> (should_wrap...), not if (!should_warp..).
Yeah, I think it looks equally good that way, and like you said, the
current code does it that way. So in the attached patch, I have
swapped the two conditions.
>
> - The following comment needs a "the" before the "first", maybe.
> + /* Loop back to first partial plan. */
Now since we are not changing this line in the patch, I have avoided
that. But the committer can very well make this change, all though I
am not sure myself about the grammar.
--
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company