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: http://www.postgresql.org/mailpref/pgsql-hackers
