|From:||Andrey Borodin <x4mmm(at)yandex-team(dot)ru>|
|To:||Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>|
|Subject:||Re: WIP: Covering + unique indexes.|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
+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  and then
./pgbench -i -s 50
create index on pgbench_accounts (abalance) include (bid,aid,filler);
create extension amcheck;
SELECT bt_index_check(c.oid), c.relname, c.relpages
FROM pg_index i
JOIN pg_opclass op ON i.indclass = 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
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  . 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 
I don't think something of these items require fixing.
Thanks for working on this, I believe it is important.
Best regards, Andrey Borodin.
|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?|