Re: BlockNumber fixes

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I did some research on this and generated the following patch. I didn't
> > find much in the way of problems except two vacuum.c fields that should
> > probably be BlockNumber. freespace.c also has a numPages field in
> > FSMRelation that is int. Should that be BlockNumber?
>
> Not necessary, since the freespace map will never be large enough to
> overflow a signed int (it wouldn't fit in the address space if it were).
> I think that your changes in vacuum.c are probably unnecessary for the
> same reason. I am generally wary of changing values from signed to
> unsigned without close analysis of how they are used --- did you look
> at *every* comparison involving these fields? How about arithmetic
> that might compute a negative result?

The only computation I saw was:

vacuumed_pages = vacuum_pages->num_pages - vacuum_pages->empty_end_pages;

vacuumed_pages is signed, the others are unsigned. However, we print
these values as %u so there is a certain confusion there.

If you say it isn't a problem here, I will just mark the item as done
and that we are handling the block numbers correctly. The only other
unusual case I saw was tid outputing block number as %d and not %u. Is
that OK?

sprintf(buf, "(%d,%d)", (int) blockNumber, (int) offsetNumber);

tidin uses atoi:

blockNumber = (BlockNumber) atoi(coord[0]);

so at least it is consistent. ;-) Doesn't seem right, however.

Also, pg_class.relpages is an int. We don't have unsigned int columns.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026