Subject: Re: WIP: Covering + unique indexes.
To: Andres Freund <andres@anarazel.de>
References: <56168952.4010101@postgrespro.ru>
 <46e1f13e-6887-3851-a525-f046029341eb@postgrespro.ru>
 <20170324124709.22419.51178.pgcf@coridan.postgresql.org>
 <c1f7c059-597f-44e8-7308-73d76ffd493c@sigaev.ru>
 <20170330191138.zawsxgsxnt2qy2as@alap3.anarazel.de>
 <a976fff4-29a9-c799-5c2d-19a371031325@postgrespro.ru>
 <20170331174728.4hpwklu46qwcrfwa@alap3.anarazel.de>
Cc: Teodor Sigaev <teodor@sigaev.ru>, pgsql-hackers@postgresql.org,
 Anastasia Lubennikova <lubennikovaav@gmail.com>,
 Peter Geoghegan <pg@heroku.com>, David Steele <david@pgmasters.net>,
 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 Aleksander Alekseev <a.alekseev@postgrespro.ru>
From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
Message-ID: <bb4b24dd-eb80-97c8-5cb1-63dca80c8ec5@postgrespro.ru>
Date: Fri, 31 Mar 2017 21:04:46 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.0
MIME-Version: 1.0
In-Reply-To: <20170331174728.4hpwklu46qwcrfwa@alap3.anarazel.de>
Content-Type: multipart/alternative;
 boundary="------------94B78ACEB10257BEA3451152"
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

This is a multi-part message in MIME format.
--------------94B78ACEB10257BEA3451152
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit

31.03.2017 20:47, Andres Freund:
>>> Maybe it would be better to modify index_form_tuple() to accept a new
>>> argument with a number of attributes, then you can just Assert that
>>> this number is never higher than the number of attributes in the
>>> TupleDesc.
>> Good point.
>> I agree that this function is a bit strange. I have to set
>> tupdesc->nattrs to support compatibility with index_form_tuple().
>> I didn't want to add neither a new field to tupledesc nor a new
>> parameter to index_form_tuple(), because they are used widely.
>>
>>
>> But I haven't considered the possibility of index_form_tuple() failure.
>> Fixed in this version of the patch. Now it creates a copy of tupledesc to
>> pass it to index_form_tuple.
> That seems like it'd actually be a noticeable increase in memory
> allocator overhead.  I think we should just add (as just proposed in
> separate thread), a _extended version of it that allows to specify the
> number of columns.

The function is called not that often. Only once per page split for 
indexes with included columns.
Doesn't look like dramatic overhead. So I decided that a wrapper 
function would be more appropriate than refactoring of all 
index_form_tuple() calls.
But index_form_tuple_extended() looks like a better solution.

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--------------94B78ACEB10257BEA3451152
Content-Type: text/html; charset=windows-1252
Content-Transfer-Encoding: 7bit

<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    31.03.2017 20:47, Andres Freund:<br>
    <blockquote
      cite="mid:20170331174728.4hpwklu46qwcrfwa@alap3.anarazel.de"
      type="cite">
      <blockquote type="cite" style="color: #000000;">
        <blockquote type="cite" style="color: #000000;">
          <pre wrap="">Maybe it would be better to modify index_form_tuple() to accept a new
argument with a number of attributes, then you can just Assert that
this number is never higher than the number of attributes in the
TupleDesc.
</pre>
        </blockquote>
        <pre wrap="">Good point.
I agree that this function is a bit strange. I have to set
tupdesc-&gt;nattrs to support compatibility with index_form_tuple().
I didn't want to add neither a new field to tupledesc nor a new
parameter to index_form_tuple(), because they are used widely.


But I haven't considered the possibility of index_form_tuple() failure.
Fixed in this version of the patch. Now it creates a copy of tupledesc to
pass it to index_form_tuple.
</pre>
      </blockquote>
      <pre wrap="">That seems like it'd actually be a noticeable increase in memory
allocator overhead.  I think we should just add (as just proposed in
separate thread), a _extended version of it that allows to specify the
number of columns.</pre>
    </blockquote>
    <br>
    The function is called not that often. Only once per page split for
    indexes with included columns.<br>
    Doesn't look like dramatic overhead. So I decided that a wrapper
    function would be more appropriate than refactoring of all
    index_form_tuple() calls.<br>
    But index_form_tuple_extended() looks like a better solution.<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>
  </body>
</html>

--------------94B78ACEB10257BEA3451152--