Re: [PATCHES] pg_autovacuum patch for 7.4.2 and HEAD

[Tom Lane]

Matthew T. O'Connor [EMAIL PROTECTED] writes:
 pg_autovacuum.c has some problems with int overflow and not using appropriate
 datatypes to track certain table values.  This patch attempts to fix all 
 these issues.  Someone should take a look and make sure its valid.
  
 ! new_tbl-relid = atol(PQgetvalue(res, row, PQfnumber(res, oid)));
 ! new_tbl-reltuples = atof(PQgetvalue(res, row, PQfnumber(res, reltuples)));
 ! new_tbl-relpages = atol(PQgetvalue(res, row, PQfnumber(res, relpages)));

This ignores the fact that relid and relpages are unsigned.  I would
suggest adopting the same convention for OID as is used in pg_dump and
other places:

#define atooid(x)  ((Oid) strtoul((x), NULL, 10))

You could actually use this same macro for reading relpages, but that's
probably abusing the notation.  I'd use strtoul directly for relpages,
I think.

 ! init_dbinfo(char *dbname, int oid, int age)
 ...
 ! init_dbinfo(char *dbname, uint oid, uint age)

This (and other declarations) should be Oid oid.

regards, tom lane

---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send unregister YourEmailAddressHere to [EMAIL PROTECTED])

Re: [PATCHES] pg_autovacuum patch for 7.4.2 and HEAD

[Matthew T. O'Connor]

Tom Lane wrote:

Matthew T. O'Connor [EMAIL PROTECTED] writes:  
 

! 	new_tbl-relid = atol(PQgetvalue(res, row, PQfnumber(res, oid)));
! 	new_tbl-reltuples = atof(PQgetvalue(res, row, PQfnumber(res, reltuples)));
! 	new_tbl-relpages = atol(PQgetvalue(res, row, PQfnumber(res, relpages)));
   

This ignores the fact that relid and relpages are unsigned.  I would
suggest adopting the same convention for OID as is used in pg_dump and
other places:
#define atooid(x)  ((Oid) strtoul((x), NULL, 10))

You could actually use this same macro for reading relpages, but that's
probably abusing the notation.  I'd use strtoul directly for relpages,
I think.
 

! init_dbinfo(char *dbname, int oid, int age)
...
! init_dbinfo(char *dbname, uint oid, uint age)
   

This (and other declarations) should be Oid oid.

 

Thanks for the help / review.  Here is my 2nd cut at fixing this.  I 
believe I have addressed the above concernes.  Please review again and 
(hopefully) apply.

Matthew



---(end of broadcast)---
TIP 6: Have you searched our list archives?
  http://archives.postgresql.org