On Fri, Oct 2, 2015 at 7:30 AM, Josh Berkus <josh@agliodbs.com> wrote:
> On 10/01/2015 07:43 AM, Robert Haas wrote:
>> On Thu, Oct 1, 2015 at 9:44 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>> I wonder how much it's worth renaming only the file extension while
>>> there are many places where "visibility map" and "vm" are used,
>>> for example, log messages, function names, variables, etc.
>>
>> I'd be inclined to keep calling it the visibility map (vm) even if it
>> also contains freeze information.
>>
>
> -1 to rename. Visibility Map is a perfectly good name.
>
Thank you for taking time to review this patch.
Attached latest v14 patch.
v14 patch is changed so that I don't change file name of visibilitymap
to "vfm", and contains some bug fix.
> +#include "access/visibilitymap.h"
> visibilitymap.h doesn't need to be included in cluster.c.
Fixed.
> - errmsg("table row type and query-specified row type do not match"),
> + errmsg("table row type and query-specified row type
> do not match"),
> This change doesn't seem to be necessary.
Fixed.
> +#define Anum_pg_class_relallfrozen 12
> Why is pg_class.relallfrozen necessary? ISTM that there is no user of it now.
The relallfrozen would be useful for user to estimate time to vacuum
freeze or anti-wrapping vacuum before being done them actually.
(Also this value is used on regression test.)
But this information is not used on planning like relallvisible, so it
would be good to move this information to another system view like
pg_stat_*_tables.
> lazy_scan_heap() calls PageClearAllVisible() when the page containing
> dead tuples is marked as all-visible. Shouldn't PageClearAllFrozen() be
> called at the same time?
Fixed.
> - "vm", /* VISIBILITYMAP_FORKNUM */
> + "vfm", /* VISIBILITYMAP_FORKNUM */
> I wonder how much it's worth renaming only the file extension while
> there are many places where "visibility map" and "vm" are used,
> for example, log messages, function names, variables, etc.
>
> I'd be inclined to keep calling it the visibility map (vm) even if it
> also contains freeze information.
>
> -1 to rename. Visibility Map is a perfectly good name.
Yeah, I agree with this.
The latest v14 patch is changed so.
Regards,
--
Masahiko Sawada