Thread: Re: [BUGS] Bug in pg_autovacuum ?
Would someone review these problems and submit a patch? Thanks. --------------------------------------------------------------------------- Tom Lane wrote: > Cott Lang <cott@internetstaff.com> writes: > > If the number of tuples is sufficiently high, pg reports 'reltuples' > > back in TABLE_STATS_QUERY in scientific notation instead of an integer. > > Right, because that column is actually a float4. > > > Changing from atoi() to atof() solves the problem completely. > > > new_tbl->reltuples = > > atof(PQgetvalue(res, row, PQfnumber(res, "reltuples"))); > > > new_tbl->relpages = > > atof(PQgetvalue(res, row, PQfnumber(res, "relpages"))); > > I should think this would break in different ways once reltuples exceeds > INT_MAX. A full fix would require changing new_tbl->reltuples to be > float or double, and coping with any downstream changes that implies. > > Also, relpages *is* an integer, though it's best interpreted as an > unsigned one. (Ditto for relid.) Looks like this code is 0-for-3 on > getting the datatypes right :-( > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > -- 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, Pennsylvania19073
Yeah, I'll take a look at it and submit a patch. Sorry I didn't see it sooner, but I don't read the bugs mailing list. On Wed, 2004-02-11 at 17:29, Bruce Momjian wrote: > Would someone review these problems and submit a patch? Thanks. > > --------------------------------------------------------------------------- > > Tom Lane wrote: > > Cott Lang <cott@internetstaff.com> writes: > > > If the number of tuples is sufficiently high, pg reports 'reltuples' > > > back in TABLE_STATS_QUERY in scientific notation instead of an integer. > > > > Right, because that column is actually a float4. > > > > > Changing from atoi() to atof() solves the problem completely. > > > > > new_tbl->reltuples = > > > atof(PQgetvalue(res, row, PQfnumber(res, "reltuples"))); > > > > > new_tbl->relpages = > > > atof(PQgetvalue(res, row, PQfnumber(res, "relpages"))); > > > > I should think this would break in different ways once reltuples exceeds > > INT_MAX. A full fix would require changing new_tbl->reltuples to be > > float or double, and coping with any downstream changes that implies. > > > > Also, relpages *is* an integer, though it's best interpreted as an > > unsigned one. (Ditto for relid.) Looks like this code is 0-for-3 on > > getting the datatypes right :-( > > > > regards, tom lane > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 4: Don't 'kill -9' the postmaster > >
I have my original changes + Tom's recommended changes applied to 7.4.1 if you're interested. On Wed, 2004-02-11 at 15:57, Matthew T. O'Connor wrote: > Yeah, I'll take a look at it and submit a patch. Sorry I didn't see it > sooner, but I don't read the bugs mailing list. > > On Wed, 2004-02-11 at 17:29, Bruce Momjian wrote: > > Would someone review these problems and submit a patch? Thanks. > > > > --------------------------------------------------------------------------- > > > > Tom Lane wrote: > > > Cott Lang <cott@internetstaff.com> writes: > > > > If the number of tuples is sufficiently high, pg reports 'reltuples' > > > > back in TABLE_STATS_QUERY in scientific notation instead of an integer. > > > > > > Right, because that column is actually a float4. > > > > > > > Changing from atoi() to atof() solves the problem completely. > > > > > > > new_tbl->reltuples = > > > > atof(PQgetvalue(res, row, PQfnumber(res, "reltuples"))); > > > > > > > new_tbl->relpages = > > > > atof(PQgetvalue(res, row, PQfnumber(res, "relpages"))); > > > > > > I should think this would break in different ways once reltuples exceeds > > > INT_MAX. A full fix would require changing new_tbl->reltuples to be > > > float or double, and coping with any downstream changes that implies. > > > > > > Also, relpages *is* an integer, though it's best interpreted as an > > > unsigned one. (Ditto for relid.) Looks like this code is 0-for-3 on > > > getting the datatypes right :-( > > > > > > regards, tom lane > > > > > > ---------------------------(end of broadcast)--------------------------- > > > TIP 4: Don't 'kill -9' the postmaster > > > >
Cott Lang wrote: > I have my original changes + Tom's recommended changes applied to 7.4.1 > if you're interested. Sure, shoot them over to hackers or patches. The pg_autovacuum author is looking into this. -- 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, Pennsylvania19073
On Wed, 2004-02-11 at 16:25, Bruce Momjian wrote: > > Sure, shoot them over to hackers or patches. The pg_autovacuum author > is looking into this. Here they are. They've worked well for me, but someone wiser in the ways of C should certainly look them over. :)
Attachment
Not sure if anyone reported to you but we fixed these in current CVS and will have the fix in 7.4.3. --------------------------------------------------------------------------- Cott Lang wrote: > On Wed, 2004-02-11 at 16:25, Bruce Momjian wrote: > > > > Sure, shoot them over to hackers or patches. The pg_autovacuum author > > is looking into this. > > Here they are. They've worked well for me, but someone wiser in the ways > of C should certainly look them over. :) > > [ Attachment, skipping... ] > > ---------------------------(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, Pennsylvania19073