Re: WIP: Covering + unique indexes.
Teodor Sigaev <teodor@sigaev.ru>
Peter Geoghegan wrote:
> On Tue, Apr 10, 2018 at 5:45 PM, Peter Geoghegan <pg@bowt.ie> wrote:
>> I did find another problem, though. Looks like the idea to explicitly
>> represent the number of attributes directly has paid off already:
>>
>> pg@~[3711]=# create table covering_bug (f1 int, f2 int, f3 text);
>> create unique index cov_idx on covering_bug (f1) include(f2);
>> insert into covering_bug select i, i * random() * 1000, i * random() *
>> 100000 from generate_series(0,100000) i;
>> DEBUG: building index "pg_toast_16451_index" on table "pg_toast_16451" serially
>> CREATE TABLE
>> DEBUG: building index "cov_idx" on table "covering_bug" serially
>> CREATE INDEX
>> ERROR: tuple has wrong number of attributes in index "cov_idx"
>
> Actually, this was an error on my part (though I'd still maintain that
> the check paid off here!). I'll still add defensive assertions inside
> _bt_newroot(), and anywhere else that they're needed. There is no
> reason to not add defensive assertions in all code that handles page
> splits, and needs to fetch a highkey from some other page. We missed a
> few of those.
Agree, I prefer to add more Assert, even. may be, more than actually
needed. Assert-documented code :)
>
> I'll add an item to "Decisions to Recheck Mid-Beta" section of the
> open items page for this patch. We should review the decision to make
> a call to _bt_check_natts() within _bt_compare(). It might work just
> as well as an assertion, and it would be unfortunate if workloads that
> don't use covering indexes had to pay a price for the
> _bt_check_natts() call, even if it was a small price. I've seen
> _bt_compare() appear prominently in profiles quite a few times.
>
Could you show a patch?
I think, we need move _bt_check_natts() and its call under
USE_ASSERT_CHECKING to prevent performance degradation. Users shouldn't
pay for unused feature.
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
Commits
-
Adjust INCLUDE index truncation comments and code.
- 075aade4361b 11.0 landed
-
Add commentary explaining why MaxIndexTuplesPerPage calculation is safe.
- 2a67d6440db4 11.0 cited
-
Indexes with INCLUDE columns and their support in B-tree
- 8224de4f42cc 11.0 landed
-
Add amcheck verification of heap relations belonging to btree indexes.
- 7f563c09f890 11.0 cited
-
Doc: move info for btree opclass implementors into main documentation.
- 3785f7eee3d9 11.0 cited
-
Doc: mention that you can't PREPARE TRANSACTION after NOTIFY.
- e4fbf22831c2 11.0 cited
-
Remove dedicated B-tree root-split record types.
- 0c504a80cf2e 11.0 cited
-
Restructure index access method API to hide most of it at the C level.
- 65c5fcd353a8 9.6.0 cited
-
Split _bt_insertonpg to two functions.
- bc292937ae6a 8.3.0 cited
-
Major overhaul of btree index code. Eliminate special BTP_CHAIN logic for
- 9e85183bfc31 7.1.1 cited