From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Andrey Borodin <amborodin@acm.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Anastasia Lubennikova <lubennikovaav@gmail.com>
Subject: Re: WIP: Covering + unique indexes.
Date: Tue, 6 Sep 2016 19:48:31 +0300
Message-ID: <e3d49c6f-3d66-ffa3-29ad-c4719350f049@postgrespro.ru> (raw)
In-Reply-To: <CAA4eK1+0jLkz8P7MVsUAj+RHmXm4BWBjV8Cc+_-Hsyhgth9ELA@mail.gmail.com>
References: <56168952.4010101@postgrespro.ru>
<CA+TgmoZ3SUbfGBFas7=G8Fs5S6uzbqgrWE_PFwQPD-=E7-2EOA@mail.gmail.com>
<20160814171131.21390.75752.pgcf@coridan.postgresql.org>
<f90aa60a-b67f-95b5-d9f5-f5d8ced178c6@postgrespro.ru>
<CAA4eK1+0jLkz8P7MVsUAj+RHmXm4BWBjV8Cc+_-Hsyhgth9ELA@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
28.08.2016 09:13, Amit Kapila:
> On Mon, Aug 15, 2016 at 8:15 PM, Anastasia Lubennikova
> <a.lubennikova@postgrespro.ru> wrote:
> @@ -590,7 +622,14 @@ _bt_buildadd(BTWriteState *wstate, BTPageState
> *state, IndexTuple itup)
> if (last_off == P_HIKEY)
> {
> Assert(state->btps_minkey == NULL);
> - state->btps_minkey = CopyIndexTuple(itup);
> + /*
> + * Truncate the tuple that we're going to insert
> + * into the parent page as a downlink
> + */
>
> + if (indnkeyatts != indnatts && P_ISLEAF(pageop))
> + state->btps_minkey = index_truncate_tuple(wstate->index, itup);
> + else
> + state->btps_minkey = CopyIndexTuple(itup);
>
> It seems that above code always ensure that for leaf pages, high key
> is a truncated tuple. What is less clear is if that is true, why you
> need to re-ensure it again for the old page in below code:
Thank you for the question. Investigation took a long time)
As far as I understand, the code above only applies to
the first tuple of each level. While the code you have quoted below
truncates high keys for all other pages.
There is a comment that clarifies situation:
/*
* If the new item is the first for its page, stash a copy for
later. Note
* this will only happen for the first item on a level; on later pages,
* the first item for a page is copied from the prior page in the code
* above.
*/
So the patch is correct.
We can go further and remove this index_truncate_tuple() call, because
the first key of any inner (or root) page doesn't need any key at all.
It simply points out to the leftmost page of the level below.
But it's not a bug, because truncation of one tuple per level doesn't
add any considerable overhead. So I want to leave the patch in its
current state.
> @@ -510,6 +513,8 @@ _bt_buildadd(BTWriteState *wstate, BTPageState
> *state, IndexTuple itup)
> {
> ..
> + if (indnkeyatts != indnatts && P_ISLEAF(opageop))
> + {
> + /*
> + * It's essential to truncate High key here.
> + * The purpose is not just to save more space on this particular page,
> + * but to keep whole b-tree structure consistent. Subsequent insertions
> + * assume that hikey is already truncated, and so they should not
> + * worry about it, when copying the high key into the parent page
> + * as a downlink.
> + * NOTE It is not crutial for reliability in present,
> + * but maybe it will be that in the future.
> + */
> + keytup = index_truncate_tuple(wstate->index, oitup);
> +
> + /* delete "wrong" high key, insert keytup as P_HIKEY. */
> + PageIndexTupleDelete(opage, P_HIKEY);
> +
> + if (!_bt_pgaddtup(opage, IndexTupleSize(keytup), keytup, P_HIKEY))
> + elog(ERROR, "failed to rewrite compressed item in index \"%s\"",
> + RelationGetRelationName(wstate->index));
> + }
> +
> ..
> ..
--
Anastasia Lubennikova
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.lubennikova@postgrespro.ru, amit.kapila16@gmail.com, amborodin@acm.org, lubennikovaav@gmail.com
Subject: Re: WIP: Covering + unique indexes.
In-Reply-To: <e3d49c6f-3d66-ffa3-29ad-c4719350f049@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox