On Tue, Mar 8, 2016 at 7:26 AM, Masahiko Sawada <sawada.m...@gmail.com> wrote: > Regarding pg_visibility module, I'd like to share some bugs and > propose to add a relation type condition to each functions.
OK, thanks. > Including it, I've attached remaining 2 patches; one is removing page > conversion code from pg_upgarde, and another is supporting pg_upgrade > for frozen bit. Committed 001 with minor tweaks. I find rewrite_vm_table to be pretty opaque. There's not even a comment explaining what it is supposed to do. And I wonder why we really need to be this efficient about it anyway. Like, would it be too expensive to just do this: for (i = 0; i < BITS_PER_BYTE; ++i) if ((old & (1 << i)) != 0) new |= 1 << (2 * i); And how about adding some more comments explaining why we are doing this rewriting, like this: In versions of PostgreSQL prior to catversion 201602181, PostgreSQL's visibility map included one bit per heap page; it now includes two. When upgrading a cluster from before that time to a current PostgreSQL version, we could refuse to copy visibility maps from the old cluster to the new cluster; the next VACUUM would recreate them, but at the price of scanning the entire table. So, instead, we rewrite the old visibility maps in the new format. That way, the all-visible bit remains set for the pages for which it was set previously. The all-frozen bit is never set by this conversion; we leave that to VACUUM. Also, I'm slightly perplexed by the fact that I can't see how this code succeeds in turning each page into two pages, which is something that it seems like it would need to do. Wouldn't we need to write out the old page header twice, one for the first of the two new pages and again for the second? I probably need more caffeine here, so please tell me what I'm missing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers