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

On 2 December 2015 at 01:53, Anastasia Lubennikova <a.lubennikova@postgrespro.ru> wrote:

Finally, completed patch "covering_unique_3.0.patch" is here.
It includes the functionality discussed above in the thread, regression tests and docs update.
I think it's quite ready for review.

Hi Anastasia,

I've maybe mentioned before that I think this is a great feature and I think it will be very useful to have, so I've signed up to review the patch, and below is the results of my first pass from reading the code. Apologies if some of the things seem like nitpicks, I've basically just listed everything I've noticed during, no matter how small.

+   An access method that supports this feature sets <structname>pg_am</>.<structfield>amcanincluding</> true.

I don't think this belongs under the "Index Uniqueness Checks" title. I think the "Columns included with clause INCLUDING  aren't used to enforce uniqueness." that you've added before it is a good idea, but perhaps the details of amcanincluding are best explained elsewhere.

+        This clause specifies additional columns to be appended to the set of index columns.

+        Included columns don't support any constraints <literal>(UNIQUE, PRMARY KEY, EXCLUSION CONSTRAINT)</>.

+        These columns can improve the performance of some queries  through using advantages of index-only scan

+        (Or so called <firstterm>covering</firstterm> indexes. Covering index is the index that

+        covers all columns required in the query and prevents a table access).

+        Besides that, included attributes are not stored in index inner pages.

+        It allows to decrease index size and furthermore it provides a way to extend included

+        columns to store atttributes without suitable opclass (not implemented yet).

+        This clause could be applied to both unique and nonunique indexes.

+        It's possible to have non-unique covering index, which behaves as a regular

+        multi-column index with a bit smaller index-size.

+        Currently, only the B-tree access method supports this feature.

"PRMARY KEY" should be "PRIMARY KEY". I ended up rewriting this paragraph as follows.

"An optional <literal>INCLUDING</> clause allows a list of columns to be specified which will be included in the index, in the non-key portion of the index. Columns which are part of this clause cannot also exist in the indexed columns portion of the index, and vice versa. The <literal>INCLUDING</> columns exist solely to allow more queries to benefit from <firstterm>index only scans</> by including certain columns in the index, the value of which would otherwise have to be obtained by reading the table's heap. Having these columns in the <literal>INCLUDING</> clause in some cases allows <productname>PostgreSQL</> to skip the heap read completely. This also allows <literal>UNIQUE</> indexes to be defined on one set of columns, which can include another set of column in the <literal>INCLUDING</> clause, on which the uniqueness is not enforced upon. This can also be useful for non-unique indexes as any columns which are not required for the searching or ordering of records can defined in the <literal>INCLUDING</> clause, which can often reduce the size of the index."

Maybe not perfect, but maybe it's an improvement?

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