On 16.06.21 10:48, Peter Eisentraut wrote: > On 15.06.21 10:17, Kyotaro Horiguchi wrote: >> The definitions are not ((type) -1) but ((type) 0xFFFFFFFF) so >> actually they might be different if we forget to widen the constant >> when widening the types. Regarding to the compiler behavior, I think >> we are assuming C99[1] and C99 defines that -1 is converted to >> Uxxx_MAX. (6.3.1.3 Singed and unsigned integers) >> >> I'm +0.2 on it. It might be worthwhile as a matter of style. > > I think since we have the constants we should use them.
I have pushed the InvalidBucket changes.
Nice. Thanks.
The use of InvalidBlockNumber with vac_update_relstats() looks a bit fishy to me. We are using in the same call 0 as the default for num_all_visible_pages, and we generally elsewhere also use 0 as the starting value for relpages, so it's not clear to me why it should be -1 or InvalidBlockNumber here.
It seems to me that the only use in vac_update_relstats is to mark relpages as invalid (dirty = true).
I'd rather leave it "slightly wrong" for now so it can be checked again.