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.


Ack... sorry. This time with the attachment....
*** ./pg_autovacuum.c.orig      2004-02-28 18:08:10.000000000 -0500
--- ./pg_autovacuum.c   2004-03-13 11:27:35.918998127 -0500
***************
*** 117,125 ****
                 atol(PQgetvalue(res, row, PQfnumber(res, "n_tup_upd"))));
        new_tbl->curr_vacuum_count = new_tbl->CountAtLastVacuum;
  
!       new_tbl->relid = atoi(PQgetvalue(res, row, PQfnumber(res, "oid")));
!       new_tbl->reltuples = atoi(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
!       new_tbl->relpages = atoi(PQgetvalue(res, row, PQfnumber(res, "relpages")));
  
        if (strcmp("t", PQgetvalue(res, row, PQfnumber(res, "relisshared"))))
                new_tbl->relisshared = 0;
--- 117,125 ----
                 atol(PQgetvalue(res, row, PQfnumber(res, "n_tup_upd"))));
        new_tbl->curr_vacuum_count = new_tbl->CountAtLastVacuum;
  
!       new_tbl->relid = atooid(PQgetvalue(res, row, PQfnumber(res, "oid")));
!       new_tbl->reltuples = atof(PQgetvalue(res, row, PQfnumber(res, "reltuples")));
!       new_tbl->relpages = atooid(PQgetvalue(res, row, PQfnumber(res, "relpages")));
  
        if (strcmp("t", PQgetvalue(res, row, PQfnumber(res, "relisshared"))))
                new_tbl->relisshared = 0;
***************
*** 159,166 ****
                if (res != NULL)
                {
                        tbl->reltuples =
!                               atoi(PQgetvalue(res, 0, PQfnumber(res, "reltuples")));
!                       tbl->relpages = atoi(PQgetvalue(res, 0, PQfnumber(res, 
"relpages")));
  
                        /*
                         * update vacuum thresholds only of we just did a vacuum
--- 159,166 ----
                if (res != NULL)
                {
                        tbl->reltuples =
!                               atof(PQgetvalue(res, 0, PQfnumber(res, "reltuples")));
!                       tbl->relpages = atooid(PQgetvalue(res, 0, PQfnumber(res, 
"relpages")));
  
                        /*
                         * update vacuum thresholds only of we just did a vacuum
***************
*** 237,243 ****
                        for (i = 0; i < t; i++)
                        {                                       /* loop through result 
set looking for a
                                                                 * match */
!                               if (tbl->relid == atoi(PQgetvalue(res, i, 
PQfnumber(res, "oid"))))
                                {
                                        found_match = 1;
                                        break;
--- 237,243 ----
                        for (i = 0; i < t; i++)
                        {                                       /* loop through result 
set looking for a
                                                                 * match */
!                               if (tbl->relid == atooid(PQgetvalue(res, i, 
PQfnumber(res, "oid"))))
                                {
                                        found_match = 1;
                                        break;
***************
*** 267,273 ****
                        while (tbl_elem != NULL)
                        {
                                tbl = ((tbl_info *) DLE_VAL(tbl_elem));
!                               if (tbl->relid == atoi(PQgetvalue(res, i, 
PQfnumber(res, "oid"))))
                                {
                                        found_match = 1;
                                        break;
--- 267,273 ----
                        while (tbl_elem != NULL)
                        {
                                tbl = ((tbl_info *) DLE_VAL(tbl_elem));
!                               if (tbl->relid == atooid(PQgetvalue(res, i, 
PQfnumber(res, "oid"))))
                                {
                                        found_match = 1;
                                        break;
***************
*** 361,369 ****
  {
        sprintf(logbuffer, "  table name:     %s.%s", tbl->dbi->dbname, 
tbl->table_name);
        log_entry(logbuffer);
!       sprintf(logbuffer, "     relid: %i;   relisshared: %i", tbl->relid, 
tbl->relisshared);
        log_entry(logbuffer);
!       sprintf(logbuffer, "     reltuples: %i;  relpages: %i", tbl->reltuples, 
tbl->relpages);
        log_entry(logbuffer);
        sprintf(logbuffer, "     curr_analyze_count:  %li; cur_delete_count:   %li",
                        tbl->curr_analyze_count, tbl->curr_vacuum_count);
--- 361,369 ----
  {
        sprintf(logbuffer, "  table name:     %s.%s", tbl->dbi->dbname, 
tbl->table_name);
        log_entry(logbuffer);
!       sprintf(logbuffer, "     relid: %u;   relisshared: %i", tbl->relid, 
tbl->relisshared);
        log_entry(logbuffer);
!       sprintf(logbuffer, "     reltuples: %f;  relpages: %u", tbl->reltuples, 
tbl->relpages);
        log_entry(logbuffer);
        sprintf(logbuffer, "     curr_analyze_count:  %li; cur_delete_count:   %li",
                        tbl->curr_analyze_count, tbl->curr_vacuum_count);
***************
*** 407,414 ****
        if (dbs->conn != NULL)
        {
                res = send_query(FROZENOID_QUERY, dbs);
!               dbs->oid = atoi(PQgetvalue(res, 0, PQfnumber(res, "oid")));
!               dbs->age = atoi(PQgetvalue(res, 0, PQfnumber(res, "age")));
                if (res)
                        PQclear(res);
  
--- 407,414 ----
        if (dbs->conn != NULL)
        {
                res = send_query(FROZENOID_QUERY, dbs);
!               dbs->oid = atooid(PQgetvalue(res, 0, PQfnumber(res, "oid")));
!               dbs->age = atol(PQgetvalue(res, 0, PQfnumber(res, "age")));
                if (res)
                        PQclear(res);
  
***************
*** 421,427 ****
  /* Simple function to create an instance of the dbinfo struct
        Initalizes all the pointers and connects to the database  */
  db_info *
! init_dbinfo(char *dbname, int oid, int age)
  {
        db_info    *newdbinfo = (db_info *) malloc(sizeof(db_info));
  
--- 421,427 ----
  /* Simple function to create an instance of the dbinfo struct
        Initalizes all the pointers and connects to the database  */
  db_info *
! init_dbinfo(char *dbname, Oid oid, long age)
  {
        db_info    *newdbinfo = (db_info *) malloc(sizeof(db_info));
  
***************
*** 500,506 ****
                        for (i = 0; i < t; i++)
                        {                                       /* loop through result 
set looking for a
                                                                 * match */
!                               if (dbi->oid == atoi(PQgetvalue(res, i, PQfnumber(res, 
"oid"))))
                                {
                                        found_match = 1;
  
--- 500,506 ----
                        for (i = 0; i < t; i++)
                        {                                       /* loop through result 
set looking for a
                                                                 * match */
!                               if (dbi->oid == atooid(PQgetvalue(res, i, 
PQfnumber(res, "oid"))))
                                {
                                        found_match = 1;
  
***************
*** 508,514 ****
                                         * update the dbi->age so that we ensure
                                         * xid_wraparound won't happen
                                         */
!                                       dbi->age = atoi(PQgetvalue(res, i, 
PQfnumber(res, "age")));
                                        break;
                                }
                        }
--- 508,514 ----
                                         * update the dbi->age so that we ensure
                                         * xid_wraparound won't happen
                                         */
!                                       dbi->age = atol(PQgetvalue(res, i, 
PQfnumber(res, "age")));
                                        break;
                                }
                        }
***************
*** 536,542 ****
                        while (db_elem != NULL)
                        {
                                dbi = ((db_info *) DLE_VAL(db_elem));
!                               if (dbi->oid == atoi(PQgetvalue(res, i, PQfnumber(res, 
"oid"))))
                                {
                                        found_match = 1;
                                        break;
--- 536,542 ----
                        while (db_elem != NULL)
                        {
                                dbi = ((db_info *) DLE_VAL(db_elem));
!                               if (dbi->oid == atooid(PQgetvalue(res, i, 
PQfnumber(res, "oid"))))
                                {
                                        found_match = 1;
                                        break;
***************
*** 548,555 ****
                        {
                                DLAddTail(db_list, DLNewElem(init_dbinfo
                                                  (PQgetvalue(res, i, PQfnumber(res, 
"datname")),
!                                                atoi(PQgetvalue(res, i, 
PQfnumber(res, "oid"))),
!                                         atoi(PQgetvalue(res, i, PQfnumber(res, 
"age"))))));
                                if (args->debug >= 1)
                                {
                                        sprintf(logbuffer, "added database: %s", 
((db_info *) DLE_VAL(DLGetTail(db_list)))->dbname);
--- 548,555 ----
                        {
                                DLAddTail(db_list, DLNewElem(init_dbinfo
                                                  (PQgetvalue(res, i, PQfnumber(res, 
"datname")),
!                                                atooid(PQgetvalue(res, i, 
PQfnumber(res, "oid"))),
!                                         atol(PQgetvalue(res, i, PQfnumber(res, 
"age"))))));
                                if (args->debug >= 1)
                                {
                                        sprintf(logbuffer, "added database: %s", 
((db_info *) DLE_VAL(DLGetTail(db_list)))->dbname);
***************
*** 681,687 ****
        sprintf(logbuffer, "dbname: %s Username %s Passwd %s", dbi->dbname,
                        dbi->username, dbi->password);
        log_entry(logbuffer);
!       sprintf(logbuffer, " oid %i InsertThresh: %i  DeleteThresh: %i", dbi->oid,
                        dbi->analyze_threshold, dbi->vacuum_threshold);
        log_entry(logbuffer);
        if (dbi->conn != NULL)
--- 681,687 ----
        sprintf(logbuffer, "dbname: %s Username %s Passwd %s", dbi->dbname,
                        dbi->username, dbi->password);
        log_entry(logbuffer);
!       sprintf(logbuffer, " oid %u InsertThresh: %li  DeleteThresh: %li", dbi->oid,
                        dbi->analyze_threshold, dbi->vacuum_threshold);
        log_entry(logbuffer);
        if (dbi->conn != NULL)
***************
*** 1072,1078 ****
                                                {               /* Loop through tables 
in list */
                                                        tbl = ((tbl_info *) 
DLE_VAL(tbl_elem));         /* set tbl_info =
                                                                                       
                                                          * current_table */
!                                                       if (tbl->relid == 
atoi(PQgetvalue(res, j, PQfnumber(res, "oid"))))
                                                        {
                                                                
tbl->curr_analyze_count =
                                                                        
(atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_ins"))) +
--- 1072,1078 ----
                                                {               /* Loop through tables 
in list */
                                                        tbl = ((tbl_info *) 
DLE_VAL(tbl_elem));         /* set tbl_info =
                                                                                       
                                                          * current_table */
!                                                       if (tbl->relid == 
atooid(PQgetvalue(res, j, PQfnumber(res, "oid"))))
                                                        {
                                                                
tbl->curr_analyze_count =
                                                                        
(atol(PQgetvalue(res, j, PQfnumber(res, "n_tup_ins"))) +
*** ./pg_autovacuum.h.orig      2004-02-28 18:08:16.000000000 -0500
--- ./pg_autovacuum.h   2004-03-13 11:29:08.612786893 -0500
***************
*** 37,46 ****
  #define TABLE_STATS_QUERY     "select 
a.oid,a.relname,a.relnamespace,a.relpages,a.relisshared,a.reltuples,b.schemaname,b.n_tup_ins,b.n_tup_upd,b.n_tup_del
 from pg_class a, pg_stat_all_tables b where a.oid=b.relid and a.relkind = 'r'"
  
  #define FRONTEND
! #define PAGES_QUERY "select oid,reltuples,relpages from pg_class where oid=%i"
  #define FROZENOID_QUERY "select oid,age(datfrozenxid) from pg_database where datname 
= 'template1'"
  #define FROZENOID_QUERY2 "select oid,datname,age(datfrozenxid) from pg_database 
where datname!='template0'"
  
  /* define cmd_args stucture */
  struct cmdargs
  {
--- 37,49 ----
  #define TABLE_STATS_QUERY     "select 
a.oid,a.relname,a.relnamespace,a.relpages,a.relisshared,a.reltuples,b.schemaname,b.n_tup_ins,b.n_tup_upd,b.n_tup_del
 from pg_class a, pg_stat_all_tables b where a.oid=b.relid and a.relkind = 'r'"
  
  #define FRONTEND
! #define PAGES_QUERY "select oid,reltuples,relpages from pg_class where oid=%u"
  #define FROZENOID_QUERY "select oid,age(datfrozenxid) from pg_database where datname 
= 'template1'"
  #define FROZENOID_QUERY2 "select oid,datname,age(datfrozenxid) from pg_database 
where datname!='template0'"
  
+ /* define atooid */
+ #define atooid(x)  ((Oid) strtoul((x), NULL, 10))
+ 
  /* define cmd_args stucture */
  struct cmdargs
  {
***************
*** 67,75 ****
        I think we need to guarantee this happens approx every 1Million TX's  */
  struct dbinfo
  {
!       int                     oid,
!                               age;
!       int                     analyze_threshold,
                                vacuum_threshold;               /* Use these as 
defaults for table
                                                                                 * 
thresholds */
        PGconn     *conn;
--- 70,78 ----
        I think we need to guarantee this happens approx every 1Million TX's  */
  struct dbinfo
  {
!       Oid                     oid;
!       long            age;
!       long            analyze_threshold,
                                vacuum_threshold;               /* Use these as 
defaults for table
                                                                                 * 
thresholds */
        PGconn     *conn;
***************
*** 84,92 ****
  {
        char       *schema_name,
                           *table_name;
!       int                     relid,
!                               reltuples,
!                               relisshared,
                                relpages;
        long            analyze_threshold,
                                vacuum_threshold;
--- 87,95 ----
  {
        char       *schema_name,
                           *table_name;
!       float           reltuples;
!       int                     relisshared;
!       Oid                     relid,
                                relpages;
        long            analyze_threshold,
                                vacuum_threshold;
***************
*** 111,117 ****
  
  /* Functions for managing database lists */
  static Dllist *init_db_list(void);
! static db_info *init_dbinfo(char *dbname, int oid, int age);
  static void update_db_list(Dllist *db_list);
  static void remove_db_from_list(Dlelem *db_to_remove);
  static void print_db_info(db_info * dbi, int print_table_list);
--- 114,120 ----
  
  /* Functions for managing database lists */
  static Dllist *init_db_list(void);
! static db_info *init_dbinfo(char *dbname, Oid oid, long age);
  static void update_db_list(Dllist *db_list);
  static void remove_db_from_list(Dlelem *db_to_remove);
  static void print_db_info(db_info * dbi, int print_table_list);
---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
      joining column's datatypes do not match

Reply via email to