Re: Few comments on commit 857f9c36 (skip full index scans ) - Mailing list pgsql-hackers

From Alexander Korotkov
Subject Re: Few comments on commit 857f9c36 (skip full index scans )
Date
Msg-id CAPpHfdsOhLyRaLQWenf=P8adbnwMet-BTWsvM8PVG_dX2ekkzQ@mail.gmail.com
Whole thread Raw
In response to Few comments on commit 857f9c36 (skip full index scans )  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
Hi!

On Mon, May 28, 2018 at 11:52 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
Review comments on commit 857f9c36:
1.
@@ -2049,6 +2055,10 @@ _bt_newroot(Relation rel, Buffer lbuf, Buffer rbuf)
    metapg = BufferGetPage(metabuf);
    metad = BTPageGetMeta(metapg);

+   /* upgrade metapage if needed */
+   if (metad->btm_version < BTREE_VERSION)
+       _bt_upgrademetapage(metapg);
+

The metapage upgrade should be performed under critical section.

2.
@@ -191,6 +304,10 @@ _bt_getroot(Relation rel, int access)
        LockBuffer(metabuf, BUFFER_LOCK_UNLOCK);
        LockBuffer(metabuf, BT_WRITE);

+       /* upgrade metapage if needed */
+       if (metad->btm_version < BTREE_VERSION)
+           _bt_upgrademetapage(metapg);
+

Same as above.

In other cases like _bt_insertonpg, the upgrade is done inside the
critical section.  It seems the above cases are missed.

3.
+   TransactionId btm_oldest_btpo_xact; /* oldest btpo_xact among of
+                                        * deleted pages */

/among of/among all

Attached patch to fix the above comments.

Thank you for catching this!
Your patch looks good for me.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company 

pgsql-hackers by date:

Previous
From: Manuel Kniep
Date:
Subject: PATCH pass PGOPTIONS to pg_regress
Next
From: Alvaro Herrera
Date:
Subject: Re: adding tab completions