Re: Setting visibility map in VACUUM's second phase

On Sun, Feb 3, 2013 at 2:31 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:

> Hi Pavan,
>
> I get this warning:
> vacuumlazy.c:890: warning: passing argument 6 of 'lazy_vacuum_page'
> makes pointer from integer without a cast
>
> and make check then fails.
>
> I've added '&' to that line, and it now passes make check with --enable-cassert.
>

Stupid me. Obviously I did not run make check before submitting the
patch, but I'm surprised my short pgbench test did not catch this.
Thanks a lot for finding and fixing this.

>
> At line 1096, when you release the vmbuffer, you don't set it to
> InvalidBuffer like the other places in the code do. It seems like
> this does would lead to a crash or assertion failure, but it does not
> seem to do so.
>

That's harmless because vmbuffer is just a local variable in that
function and we are at the end of the function and that variable is
not used again. But it helps to just be consistent. So I'm OK with
your change.

>
> Also, the "Note: If you change anything below, also look at" should
> probably say "Note: If you change anything in the for loop below, also
> look at". Otherwise I'd be wondering how far below the caveat
> applies.

Ok.

>
> I've attached a patch with these changes made. Does this look OK?
>

Looks good to me. I also repeated pgbench and make check and they work
as expected. I'll add it to the CF and also mark the patch "ready for
committer"

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee