Re: WIP: Covering + unique indexes.

From: Andrey Borodin <x4mmm(at)yandex-team(dot)ru>
To: Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: WIP: Covering + unique indexes.
Date: 2017-11-12 11:40:46
Message-ID: 7294DCE2-7368-4E28-BC29-2050E199A072@yandex-team.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!
+1 for pushing this. I'm really looking forward to see this in 11.

> 31 окт. 2017 г., в 13:21, Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru> написал(а):
>
> Updated version is attached. It applies to the commit e4fbf22831c2bbcf032ee60a327b871d2364b3f5.
> The first patch patch contains changes in general index routines
> and the second one contains btree specific changes.
>
> This version contains fixes of the issues mentioned in the thread above and passes all existing tests.
> But still it requires review and testing, because the merge was quite uneasy.
> Especially I worry about integration with partitioning. I'll add some more tests in the next message.

I've been doing benchmark tests a year ago, so I skip this part in this review.

I've done some stress tests with pgbench, replication etc. Everything was fine until I plugged in amcheck.
If I create cluster with this [0] and then
./pgbench -i -s 50

create index on pgbench_accounts (abalance) include (bid,aid,filler);
create extension amcheck;

--and finally
SELECT bt_index_check(c.oid), c.relname, c.relpages
FROM pg_index i
JOIN pg_opclass op ON i.indclass[0] = op.oid
JOIN pg_am am ON op.opcmethod = am.oid
JOIN pg_class c ON i.indexrelid = c.oid
JOIN pg_namespace n ON c.relnamespace = n.oid
WHERE am.amname = 'btree' AND n.nspname = 'public'
AND c.relpersistence != 't'
AND i.indisready AND i.indisvalid
ORDER BY c.relpages DESC LIMIT 100;
--just copypasted from amcheck docs with minor corrections

Postgres crashes:
TRAP: FailedAssertion("!(((const void*)(&isNull) != ((void*)0)) && (scankey->sk_attno) > 0)", File: "nbtsearch.c", Line: 466)

May be I'm doing something wrong or amcheck support will go with different patch?

Few minor nitpicks:
0. PgAdmin fails to understand what is going on [1] . It is clearly problem of PgAdmin, pg_dump works as expected.
1. ISTM index_truncate_tuple() can be optimized. We only need to reset tuple length and infomask. But this should not be hotpath, anyway, so I propose ignoring this for current version.
2. I've done grammarly checking :) This comma seems redundant [2]
I don't think something of these items require fixing.

Thanks for working on this, I believe it is important.

Best regards, Andrey Borodin.

[0] https://github.com/x4m/pgscripts/blob/master/install.sh
[1] https://yadi.sk/i/ro9YKFqo3PcwFT
[2] https://github.com/x4m/postgres_g/commit/657c28952d923d8c150e6cabb3bdcbbc44a641b6?diff=unified#diff-640baf2937029728a8d51cccd554c2eeR1291

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Cramer 2017-11-12 15:01:47 Re: PSA: don't be in a hurry to update to XCode 9.0
Previous Message Thomas Munro 2017-11-12 08:50:44 Re: A GUC to prevent leader processes from running subplans?