Обсуждение: pageinspect forks

I noticed that the documentation for pageinspect lists the different
forks but omits the initialization fork.

Here is a trivial patch to fix that.

--
Vik

From: "Vik Fearing" <vik.fearing@dalibo.com>
>I noticed that the documentation for pageinspect lists the different
> forks but omits the initialization fork.
>
> Here is a trivial patch to fix that.

Could you also improve the doc by changing "table" to "relation" in the
description of get_raw_page()?  Having a quick look at the code, it seems
that the function can also handle indexes.

        table and returns a copy as a <type>bytea</> value.  This allows a

Regards
MauMau

On Wed, Jul 30, 2014 at 9:11 PM, Vik Fearing <vik.fearing@dalibo.com> wrote:
> I noticed that the documentation for pageinspect lists the different
> forks but omits the initialization fork.
>
> Here is a trivial patch to fix that.

ISTM that this fix needs to back-patched to 9.1 where
the initialization fork was introduced.

I found that the document of pg_relation_size() had
the same problem and it has been fixed recently by
2d00190495b22e0d0ba351b2cda9c95fb2e3d083,
but it has not been back-patched to 9.1, 9.2 and 9.3.
I think that we should back-patch that to those versions.

That commit also fixed the HINT message
"HINT:  Valid fork names are "main", "fsm", and "vm".",
so that the initialization fork is included in that message.
This also needs to be back-patched to those versions.

Regards,

--
Fujii Masao

On 08/10/2014 03:24 PM, MauMau wrote:
> From: "Vik Fearing" <vik.fearing@dalibo.com>
>> I noticed that the documentation for pageinspect lists the different
>> forks but omits the initialization fork.
>>
>> Here is a trivial patch to fix that.
>
> Could you also improve the doc by changing "table" to "relation" in the
> description of get_raw_page()?  Having a quick look at the code, it
> seems that the function can also handle indexes.
>
>        table and returns a copy as a <type>bytea</> value.  This allows a


Good catch.  I wasn't sure whether using "relation" or "table or index"
was better, but after a quick test it seems it works for sequences, too,
so I've gone with "relation".

It doesn't work with views, of course, but it does work with
materialized views.

Modified patch attached.
--
Vik

From: "Vik Fearing" <vik.fearing@dalibo.com>
> On 08/10/2014 03:24 PM, MauMau wrote:
>> Could you also improve the doc by changing "table" to "relation" in the
>> description of get_raw_page()?  Having a quick look at the code, it
>> seems that the function can also handle indexes.
>>
>>        table and returns a copy as a <type>bytea</> value.  This allows a
>
>
> Good catch.  I wasn't sure whether using "relation" or "table or index"
> was better, but after a quick test it seems it works for sequences, too,
> so I've gone with "relation".
>
> It doesn't work with views, of course, but it does work with
> materialized views.
>
> Modified patch attached.

Thanks, I marked this as ready for committer.

I think I'll leave it up to you to decide whether you improve the HINT
message Fujii-san pointed out, which is in src/backend/catalog/catalog.c.

Regards
MauMau

On 08/11/2014 12:22 AM, MauMau wrote:
> From: "Vik Fearing" <vik.fearing@dalibo.com>
>> On 08/10/2014 03:24 PM, MauMau wrote:
>>> Could you also improve the doc by changing "table" to "relation" in the
>>> description of get_raw_page()?  Having a quick look at the code, it
>>> seems that the function can also handle indexes.
>>>
>>>        table and returns a copy as a <type>bytea</> value.  This
>>> allows a
>>
>>
>> Good catch.  I wasn't sure whether using "relation" or "table or index"
>> was better, but after a quick test it seems it works for sequences, too,
>> so I've gone with "relation".
>>
>> It doesn't work with views, of course, but it does work with
>> materialized views.
>>
>> Modified patch attached.
>
> Thanks, I marked this as ready for committer.
>
> I think I'll leave it up to you to decide whether you improve the HINT
> message Fujii-san pointed out, which is in src/backend/catalog/catalog.c.

I think you're looking at an old version of the code.  My understanding
of his message was that that patch had already been applied but not
backpatched and he wants it backpatched.  To wit, that hint message does
include "init" on master and it's located in src/common/relpath.c.

However, I did find another occurence in the documentation, so third
patch attached.
--
Vik

From: "Vik Fearing" <vik.fearing@dalibo.com>
> I think you're looking at an old version of the code.  My understanding
> of his message was that that patch had already been applied but not
> backpatched and he wants it backpatched.  To wit, that hint message does
> include "init" on master and it's located in src/common/relpath.c.
>
> However, I did find another occurence in the documentation, so third
> patch attached.

You are right, I was looking at the code of 9.3.4 at hand.  The latest code
certainly has the accurate HINT message.

Your v3 patch looks reasonable.

Regards
MauMau

On Mon, Aug 11, 2014 at 10:42 PM, MauMau <maumau307@gmail.com> wrote:
> From: "Vik Fearing" <vik.fearing@dalibo.com>
>
>> I think you're looking at an old version of the code.  My understanding
>> of his message was that that patch had already been applied but not
>> backpatched and he wants it backpatched.  To wit, that hint message does
>> include "init" on master and it's located in src/common/relpath.c.
>>
>> However, I did find another occurence in the documentation, so third
>> patch attached.
>
>
> You are right, I was looking at the code of 9.3.4 at hand.  The latest code
> certainly has the accurate HINT message.
>
> Your v3 patch looks reasonable.

Thanks for the patch! Applied.

Regards,

--
Fujii Masao