Re: WIP: Covering + unique indexes. - Mailing list pgsql-hackers

28.08.2016 09:13, Amit Kapila:<br /><blockquote
cite="mid:CAA4eK1+0jLkz8P7MVsUAj+RHmXm4BWBjV8Cc+_-Hsyhgth9ELA@mail.gmail.com"type="cite"><pre wrap="">On Mon, Aug 15,
2016at 8:15 PM, Anastasia Lubennikova
 
<a class="moz-txt-link-rfc2396E" href="mailto:a.lubennikova@postgrespro.ru"><a.lubennikova@postgrespro.ru></a>
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:
</pre></blockquote><br /> Thank you for the question. Investigation took a long time)<br /><br /> As far as I
understand,the code above only applies to<br /> the first tuple of each level. While the code you have quoted below<br
/>truncates high keys for all other pages.<br /><br /> There is a comment that clarifies situation:<br />     /*<br />
    * If the new item is the first for its page, stash a copy for later. Note<br />      * this will only happen for
thefirst item on a level; on later pages,<br />      * the first item for a page is copied from the prior page in the
code<br/>      * above.<br />      */<br /><br /><br /> So the patch is correct.<br /> We can go further and remove
thisindex_truncate_tuple() call, because<br /> the first key of any inner (or root) page doesn't need any key at
all.<br/> It simply points out to the leftmost page of the level below.<br /> But it's not a bug, because truncation of
onetuple per level doesn't<br /> add any considerable overhead. So I want to leave the patch in its current state.<br
/><br/><blockquote cite="mid:CAA4eK1+0jLkz8P7MVsUAj+RHmXm4BWBjV8Cc+_-Hsyhgth9ELA@mail.gmail.com" type="cite"><pre
wrap="">@@-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));
+ }
+
..
..
</pre></blockquote><br /><br /><pre class="moz-signature" cols="72">-- 
Anastasia Lubennikova
Postgres Professional: <a class="moz-txt-link-freetext"
href="http://www.postgrespro.com">http://www.postgrespro.com</a>
The Russian Postgres Company</pre>