Ok I have fixed most of what Mangnus mentioned and attached an updated
diff file.
On Wed, 2004-06-16 at 05:14, Magnus Hagander wrote:
> Some nitpicking on details:
>
> The comment above AutoVacMain() claims:
> ! * Main entry point for bgwriter process
Oops... Can you tell where I cribbed the code from? Fixed in attached
patch.
> I also see a bunch of // comments, I think those are not appreciated.
Yeah ok fixed in attached patch.
> Also, isn't it a bit unnecessary to do:
> sprintf(logbuffer,"foo bar %s",whatever);
> elog(ERROR,logbuffer);
>
> why not just
> elog(ERROR,"foo bar %s",whatever);
> [assuming I read the patch right - I still need some practice reading
> patches...]
Sorry this was due to my ignorance of the elog / ereport api, I'll fixed
in attached patch.
> I also noticed:
> ! elog(ERROR, "pg_autovacuum: GUC variable stats_row_level
> must be enabled.");
> ! elog(ERROR, " Please fix the problems and try
> again.");
>
> If you use the ereport() call instead of elog, you can set the second
> one as a HINT. Since it's really the same error, I don't think you want
> multiple errors logged...
Ok, I didn't have time to change hings to ereport, I'll look into this tonight.
> I'll leave it to others to comment on the actual code - I'll take the
> easy way out and do nitpicking instead :-) I'll try to test this on
> win32 as soon as I have my tree back in compiling order.
Thanks for the feedback. Please do test this on win32.
Thanks again,