On 2017/09/07 13:09, Michael Paquier wrote:
> On Thu, Sep 7, 2017 at 12:59 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>> The idea I'd had was to apply the masking only if pd_lower >=
>> SizeOfPageHeaderData, or if you wanted to be stricter, only if
>> pd_lower != 0.
> 
> If putting a check, it seems to me that the stricter one makes the
> most sense.

OK, patches updated that way.

> pd_lower should remain at 0 on pre-10 servers.

Doesn't PageInit(), which is where any page gets initialized, has always
set pd_lower to SizeOfPageHeaderData?

Thanks,
Amit
From 681c0ea82d0d9acd37f1979ebe1918d8636b0d26 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 23 Jun 2017 11:20:41 +0900
Subject: [PATCH 1/3] Set pd_lower correctly in the GIN metapage.

---
 src/backend/access/gin/ginutil.c |  7 +++++++
 src/backend/access/gin/ginxlog.c | 24 +++++++++---------------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index 136ea27718..ebac391818 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -374,6 +374,13 @@ GinInitMetabuffer(Buffer b)
        metadata->nDataPages = 0;
        metadata->nEntries = 0;
        metadata->ginVersion = GIN_CURRENT_VERSION;
+
+       /*
+        * Set pd_lower just past the end of the metadata.  This is not 
essential
+        * but it makes the page look compressible to xlog.c.
+        */
+       ((PageHeader) page)->pd_lower =
+                       ((char *) metadata + sizeof(GinMetaPageData)) - (char 
*) page;
 }
 
 /*
diff --git a/src/backend/access/gin/ginxlog.c b/src/backend/access/gin/ginxlog.c
index 7ba04e324f..f5c11b2d9a 100644
--- a/src/backend/access/gin/ginxlog.c
+++ b/src/backend/access/gin/ginxlog.c
@@ -514,7 +514,7 @@ ginRedoUpdateMetapage(XLogReaderState *record)
        Assert(BufferGetBlockNumber(metabuffer) == GIN_METAPAGE_BLKNO);
        metapage = BufferGetPage(metabuffer);
 
-       GinInitPage(metapage, GIN_META, BufferGetPageSize(metabuffer));
+       GinInitMetabuffer(metabuffer);
        memcpy(GinPageGetMeta(metapage), &data->metadata, 
sizeof(GinMetaPageData));
        PageSetLSN(metapage, lsn);
        MarkBufferDirty(metabuffer);
@@ -656,7 +656,7 @@ ginRedoDeleteListPages(XLogReaderState *record)
        Assert(BufferGetBlockNumber(metabuffer) == GIN_METAPAGE_BLKNO);
        metapage = BufferGetPage(metabuffer);
 
-       GinInitPage(metapage, GIN_META, BufferGetPageSize(metabuffer));
+       GinInitMetabuffer(metabuffer);
 
        memcpy(GinPageGetMeta(metapage), &data->metadata, 
sizeof(GinMetaPageData));
        PageSetLSN(metapage, lsn);
@@ -768,6 +768,7 @@ void
 gin_mask(char *pagedata, BlockNumber blkno)
 {
        Page            page = (Page) pagedata;
+       PageHeader      pagehdr = (PageHeader) page;
        GinPageOpaque opaque;
 
        mask_page_lsn(page);
@@ -776,18 +777,11 @@ gin_mask(char *pagedata, BlockNumber blkno)
        mask_page_hint_bits(page);
 
        /*
-        * GIN metapage doesn't use pd_lower/pd_upper. Other page types do. 
Hence,
-        * we need to apply masking for those pages.
+        * For GIN_DELETED page, the page is initialized to empty. Hence, mask
+        * the page content.
         */
-       if (opaque->flags != GIN_META)
-       {
-               /*
-                * For GIN_DELETED page, the page is initialized to empty. 
Hence, mask
-                * the page content.
-                */
-               if (opaque->flags & GIN_DELETED)
-                       mask_page_content(page);
-               else
-                       mask_unused_space(page);
-       }
+       if (opaque->flags & GIN_DELETED)
+               mask_page_content(page);
+       else if (pagehdr->pd_lower != 0)
+               mask_unused_space(page);
 }
-- 
2.11.0

From a605ae99664138aa827500216567137be97d0460 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 26 Jun 2017 15:13:32 +0900
Subject: [PATCH 2/3] Set pd_lower correctly in the BRIN index metapage

---
 src/backend/access/brin/brin_pageops.c | 7 +++++++
 src/backend/access/brin/brin_xlog.c    | 9 +++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/brin/brin_pageops.c 
b/src/backend/access/brin/brin_pageops.c
index 80f803e438..8762356433 100644
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -491,6 +491,13 @@ brin_metapage_init(Page page, BlockNumber pagesPerRange, 
uint16 version)
         * revmap page to be created when the index is.
         */
        metadata->lastRevmapPage = 0;
+
+       /*
+        * Set pd_lower just past the end of the metadata.  This is to log full
+        * page image of metapage in xloginsert.c.
+        */
+       ((PageHeader) page)->pd_lower =
+               ((char *) metadata + sizeof(BrinMetaPageData)) - (char *) page;
 }
 
 /*
diff --git a/src/backend/access/brin/brin_xlog.c 
b/src/backend/access/brin/brin_xlog.c
index dff7198a39..1309d44b04 100644
--- a/src/backend/access/brin/brin_xlog.c
+++ b/src/backend/access/brin/brin_xlog.c
@@ -331,14 +331,19 @@ void
 brin_mask(char *pagedata, BlockNumber blkno)
 {
        Page            page = (Page) pagedata;
+       PageHeader      pagehdr = (PageHeader) page;
 
        mask_page_lsn(page);
 
        mask_page_hint_bits(page);
 
-       if (BRIN_IS_REGULAR_PAGE(page))
+       /*
+        * Regular brin pages contain unused space which needs to be masked.
+        * Similarly for meta pages, but only if pd_lower has been set.
+        */
+       if (BRIN_IS_REGULAR_PAGE(page) ||
+               (BRIN_IS_META_PAGE(page) && pagehdr->pd_lower != 0))
        {
-               /* Regular brin pages contain unused space which needs to be 
masked. */
                mask_unused_space(page);
        }
 }
-- 
2.11.0

From e01d37b77b3e761a499850aa80de1c403f5dab86 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 26 Jun 2017 15:23:34 +0900
Subject: [PATCH 3/3] Set pd_lower correctly in the SP-GiST index metapage

---
 src/backend/access/spgist/spgutils.c | 7 +++++++
 src/backend/access/spgist/spgxlog.c  | 8 +++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/spgist/spgutils.c 
b/src/backend/access/spgist/spgutils.c
index 22f64b0103..2f21f623da 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -534,6 +534,13 @@ SpGistInitMetapage(Page page)
        /* initialize last-used-page cache to empty */
        for (i = 0; i < SPGIST_CACHED_PAGES; i++)
                metadata->lastUsedPages.cachedPage[i].blkno = 
InvalidBlockNumber;
+
+       /*
+        * Set pd_lower just past the end of the metadata.  This is to log full
+        * page image of metapage in xloginsert.c.
+        */
+       ((PageHeader) page)->pd_lower =
+               ((char *) metadata + sizeof(SpGistMetaPageData)) - (char *) 
page;
 }
 
 /*
diff --git a/src/backend/access/spgist/spgxlog.c 
b/src/backend/access/spgist/spgxlog.c
index c440d21715..bf209416f2 100644
--- a/src/backend/access/spgist/spgxlog.c
+++ b/src/backend/access/spgist/spgxlog.c
@@ -1033,15 +1033,13 @@ void
 spg_mask(char *pagedata, BlockNumber blkno)
 {
        Page            page = (Page) pagedata;
+       PageHeader      pagehdr = (PageHeader) page;
 
        mask_page_lsn(page);
 
        mask_page_hint_bits(page);
 
-       /*
-        * Any SpGist page other than meta contains unused space which needs to 
be
-        * masked.
-        */
-       if (!SpGistPageIsMeta(page))
+       /* Mask the unused space, provided the page's pd_lower is set. */
+       if (pagehdr->pd_lower != 0)
                mask_unused_space(page);
 }
-- 
2.11.0

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

Reply via email to