pg_autovacuum patch for 7.4.2 and HEAD

"Matthew T. O'Connor" <matthew@zeut.net> 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

Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> 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

Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> 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.

Ack... sorry. This time with the attachment....

Patch applied. Thanks.

---------------------------------------------------------------------------

Matthew T. O'Connor wrote:

Tom Lane wrote:

"Matthew T. O'Connor" <matthew@zeut.net> 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

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073