On 2021-Jul-02, Justin Pryzby wrote: > On Fri, Jul 02, 2021 at 12:09:23PM +0200, Peter Eisentraut wrote:
> > 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. I'd rather leave it "slightly wrong" for now so it > > can be checked again. > |commit 0e69f705cc1a3df273b38c9883fb5765991e04fe > |Author: Alvaro Herrera <alvhe...@alvh.no-ip.org> > |Date: Fri Apr 9 11:29:08 2021 -0400 > | > | Set pg_class.reltuples for partitioned tables > > 3d35 also affects partitioned tables, and 0e69 appears to do the right thing > by > preserving relpages=-1 during auto-analyze. I suppose the question is what is the value used for. BlockNumber is typedef'd uint32, an unsigned variable, so using -1 for it is quite fishy. The weird thing is that in vac_update_relstats we cast it to (int32) when storing it in the pg_class tuple, so that's quite fishy too. What we really want is for table_block_relation_estimate_size to work properly. What that does is get the signed-int32 value from pg_class and cast it back to BlockNumber. If that assignment gets -1 again, then it's all fine. I didn't test it. I think changing the vac_update_relstats call I added in 0e69f705cc1a to InvalidBlockNumber is fine. I didn't verify any other places. I think storing BlockNumber values >= 2^31 in an int32 catalog column is asking for trouble. We'll have to fix that at some point. -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
