On Tue, Jun 20, 2017 at 1:50 PM, Amit Langote
<langote_amit...@lab.ntt.co.jp> wrote:
> On 2017/06/19 23:31, Tom Lane wrote:
>> Amit Kapila <amit.kapil...@gmail.com> writes:
>>> On Mon, Jun 19, 2017 at 11:37 AM, Amit Langote
>>> <langote_amit...@lab.ntt.co.jp> wrote:
>>>> What are some arguments against setting pd_lower in the GIN metapage as
>>>> follows?
>>> Actually, hash index also has similar code (See _hash_init_metabuffer)
>>> and I see no harm in doing this at similar other places.
>> Seems reasonable.
> Here is a patch that does it for the GIN metapage.  (I am not sure if the
> changes to gin_mask() that are included in the patch are really necessary.)
>>>> How about porting such a change to the back-branches if we do this at all?
>>>> The reason I'm asking is that a certain backup tool relies on pd_lower
>>>> values of data pages (disk blocks in relation files that are known to have
>>>> a valid PageHeaderData) to be correct to discard the portion of every page
>>>> that supposedly does not contain any useful information.  The assumption
>>>> doesn't hold in the case of GIN metapage, so any GIN indexes contain
>>>> corrupted metapage after recovery (metadata overwritten with zeros).
>> I'm not in favor of back-porting such a change.  Even if we did, it would
>> only affect subsequently-created indexes not existing ones.  That means
>> your tool has to cope with an unset pd_lower in any case --- and will for
>> the foreseeable future, because of pg_upgrade.
>> I'd suggest a rule like "if pd_lower is smaller than SizeOfPageHeaderData
>> then don't trust it, but assume all of the page is valid data".
> Actually, such a check is already in place in the tool, whose condition
> looks like:
>     if (PageGetPageSize(header) == BLCKSZ &&
>         PageGetPageLayoutVersion(header) == PG_PAGE_LAYOUT_VERSION &&
>         (header->pd_flags & ~PD_VALID_FLAG_BITS) == 0 &&
>         header->pd_lower >= SizeOfPageHeaderData &&
>         header->pd_lower <= header->pd_upper &&
>         header->pd_upper <= header->pd_special &&
>         header->pd_special <= BLCKSZ &&
>         header->pd_special == MAXALIGN(header->pd_special) && ...
> which even GIN metapage passes, making it an eligible data page and hence
> for omitting the hole between pd_lower and pd_upper.

Won't checking for GIN_META in header->pd_flags gives you what you want?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to