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