Hi Alvaro,
On Mon, Aug 5, 2019 at 12:24 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> On 2019-Aug-04, Alvaro Herrera wrote:
>
> > So this is the best commit messages I could come up with at this stupid
> > hour. I think the wording is pretty poor but at least it seems correct.
> > I'm not sure I'll be able to get this pushed tomorrow, but I'll try.
>
> Pushed. Since this is Sunday before minors, I'll be checking buildfarm
> and will summarily revert if anything goes wrong.
Thanks for the revisions and committing. I can imagine the stress
when writing that commit message, but it seems correct to me. Thanks
to Hosoya-san for spotting the problem and working on the fix.
It had occurred to me when reviewing this patch, prompted by
Horiguchi-san's comment [1], that our design of PruneStepResult might
not be so good. Especially, we don't really need to track whether the
default partition needs to be scanned on a per-step basis. Maybe, the
result of each step should be simply a Bitmapset storing the set of
bound offsets. We then check in the end if any of the bound offsets
in the final set (that is, after having executed all the steps)
require scanning the default partition, very much like what we did in
the committed patch. ISTM, scan_null wouldn't cause the same problems
as scan_default did, so we can add the null_index to a given step's
result set when executing the step. IOW, we can get rid of the
unnecessary abstraction that is the PruneStepResult struct.
Thanks,
Amit
[1] https://www.postgresql.org/message-id/20190319.152756.202071463.horiguchi.kyotaro%40lab.ntt.co.jp