Hi, bq. Not sure why would that be an issue Moving the (start > end) check is up to your discretion.
But the midpoint computation should follow text book :-) Cheers On Wed, Feb 3, 2021 at 4:59 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > > > On 2/4/21 1:49 AM, Zhihong Yu wrote: > > Hi, > > For 0007-Remove-the-special-batch-mode-use-a-larger--20210203.patch : > > > > + /* same as preceding value, so store it */ > > + if (compare_values(&range->values[start + i - 1], > > + &range->values[start + i], > > + (void *) &cxt) == 0) > > + continue; > > + > > + range->values[start + n] = range->values[start + i]; > > > > It seems the comment doesn't match the code: the value is stored when > > subsequent value is different from the previous. > > > > Yeah, you're right the comment is wrong - the code is doing exactly the > opposite. I'll need to go through this more carefully. > > > For has_matching_range(): > > + int midpoint = (start + end) / 2; > > > > I think the standard notion for midpoint is start + (end-start)/2. > > > > + /* this means we ran out of ranges in the last step */ > > + if (start > end) > > + return false; > > > > It seems the above should be ahead of computation of midpoint. > > > > Not sure why would that be an issue, as we're not using the value and > the values are just plain integers (so no overflows ...). > > > regards > > -- > Tomas Vondra > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
