On Tue, Mar 8, 2016 at 8:16 PM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
> Shulgin, Oleksandr wrote: > > > Alright. I'm attaching the latest version of this patch split in two > > parts: the first one is NULLs-related bugfix and the second is the > > "improvement" part, which applies on top of the first one. > > I went over patch 0001 and it seems pretty reasonable. It's missing > some comment updates -- at least the large comments that talk about Duj1 > should be modified to indicate why the code is now subtracting the null > count. Good point. > Also, I can't quite figure out why the "else" now in line 2131 > is now "else if track_cnt != 0". What happens if track_cnt is zero? > The comment above the "if" block doesn't provide any guidance. > It is there only to avoid potentially dividing zero by zero when calculating avgcount (which will not be used after that anyway). I agree it deserves a comment. Thank you! -- Alex
