On Fri, Mar 11, 2016 at 1:03 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> This 001 patch looks so little like what I was expecting that I
> decided to start over from scratch. The new version I wrote is
> attached here. I don't understand why your version tinkers with the
> logic for setting the all-frozen bit; I thought that what I already
> committed dealt with that already, and in any case, your version
> doesn't even compile against latest sources. Your version also leaves
> the scan_all terminology intact even though it's not accurate any
> more, and I am not very convinced that the updates to the
> page-skipping logic are actually correct. Please have a look over
> this version and see what you think.
Thank you for your advise.
Sorry, optimising logic of previous patch was old by mistake.
Attached latest patch incorporated your suggestions with a little revising.
>
> I think that's kind of pointless. We need to test that this
> conversion code works, but once it does, I don't think we should make
> everybody pay the overhead of retesting that. Anyway, the test code
> could have bugs, too.
>
> Here's an updated version of your patch with that code removed and
> some cosmetic cleanups like fixing typos and stuff like that. I think
> this is mostly ready to commit, but I noticed one problem: your
> conversion code always produces two output pages for each input page
> even if one of them would be empty. In particular, if you have a
> large number of small relations and run pg_upgrade, all of their
> visibility maps will go from 8kB to 16kB. That isn't the end of the
> world, maybe, but I think you should see if you can't fix it
> somehow....
Thank you for updating patch.
To deal with this problem, I've changed it so that pg_upgrade checks
file size before conversion.
And if fork file does not exist or size is 0 (empty), ignore.
Attached latest patch.
Regards,
--
Masahiko Sawada