Horton

Re: WIP: Covering + unique indexes.

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Teodor Sigaev <teodor@sigaev.ru>
Cc: "pgsql-hackers@postgresql.org" <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>
Date: 2017-03-30T16:49:39Z
Lists: pgsql-hackers 
On Thu, Mar 30, 2017 at 11:26 AM, Teodor Sigaev <teodor@sigaev.ru> wrote:
> I had a look on patch and played with it, seems, it looks fine. I splitted
> it to two patches: core changes (+bloom index fix) and btree itself. All
> docs are left in first patch - I'm too lazy to rewrite documentation which
> is changed in second patch.
> Any objection from reviewers to push both patches?

Has this really had enough review and testing?  The last time it was
pushed, it didn't go too well.  And laziness is not a very good excuse
for not dividing up patches properly.

It seems highly surprising to me that CheckIndexCompatible() only gets
a one line change in this patch.  That seems unlikely to be correct.

Has anybody done some testing of this patch with the WAL consistency
checker?  Like, create some tables with indexes that have INCLUDE
columns, set up a standby, enable consistency checking, pound the
master, and see if the standby bails?

Has anybody tested this patch with amcheck?  Does it break amcheck?

A few minor comments:

-    foreach(lc, constraint->keys)
+    else foreach(lc, constraint->keys)

That doesn't look like a reasonable way of formatting the code.

+    /* Here is some code duplication. But we do need it. */

That is not a very informative comment.

+                        * NOTE It is not crutial for reliability in present,

Spelling, punctuation.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Adjust INCLUDE index truncation comments and code.

  2. Add commentary explaining why MaxIndexTuplesPerPage calculation is safe.

  3. Indexes with INCLUDE columns and their support in B-tree

  4. Add amcheck verification of heap relations belonging to btree indexes.

  5. Doc: move info for btree opclass implementors into main documentation.

  6. Doc: mention that you can't PREPARE TRANSACTION after NOTIFY.

  7. Remove dedicated B-tree root-split record types.

  8. Restructure index access method API to hide most of it at the C level.

  9. Split _bt_insertonpg to two functions.

  10. Major overhaul of btree index code. Eliminate special BTP_CHAIN logic for