From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Fri, 1 Mar 2019 15:57:36 +0300 From: Vladimir Davydov Subject: Re: [PATCH 07/12] vinyl: sanitize full/empty key stmt detection Message-ID: <20190301125736.bppcf26bt3xvxozx@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: On Thu, Feb 21, 2019 at 01:26:07PM +0300, Vladimir Davydov wrote: > Historically, we use tuple_field_count to check whether a statement > represents an empty key (match all) or a full key (point lookup): if > the number of fields in a tuple is greater than or equal to the number > of parts in a key definition, it can be used as a full key; if the > number of fields is zero, then the statement represents an empty key. > > While this used to be correct not so long ago, appearance of JSON > indexes changed the rules of the game: now a tuple can have nested > indexed fields so that the same field number appears in the key > definition multiple times. This means tuple_field_count can be less > than the number of key parts and hence the full key check won't work > for a statement representing a tuple. > > Actually, any tuple in vinyl can be used as a full key as it has all > key parts by definition, there's no need to use tuple_field_count for > such statements - we only need to do that for statements representing > keys. Keeping that in mind, let's introduce helpers for checking > whether a statement can be used as a full/empty key and use them > throughout the code. > --- > src/box/vinyl.c | 2 +- > src/box/vy_cache.c | 14 +++++++++----- > src/box/vy_mem.c | 2 +- > src/box/vy_point_lookup.c | 5 ++--- > src/box/vy_range.c | 5 ++--- > src/box/vy_read_iterator.c | 6 +++--- > src/box/vy_read_set.c | 18 ++++++------------ > src/box/vy_run.c | 2 +- > src/box/vy_stmt.h | 38 ++++++++++++++++++++++++++++++++++++++ > src/box/vy_tx.c | 4 ++-- > 10 files changed, 65 insertions(+), 31 deletions(-) Pushed to 2.1 and 1.10. Also, implemented a test that demonstrates the issues with JSON indexes in vinyl and pushed it to 2.1: >From 2f14800131340621c0818fc326ea618aa4296c63 Mon Sep 17 00:00:00 2001 From: Vladimir Davydov Date: Fri, 1 Mar 2019 15:45:32 +0300 Subject: [PATCH] test: check vinyl/json corner cases Follow-up 5993e149d90e vinyl: sanitize full/empty key stmt detection 4273ec52e122 box: introduce JSON Indexes diff --git a/test/vinyl/json.result b/test/vinyl/json.result new file mode 100644 index 00000000..f17619f4 --- /dev/null +++ b/test/vinyl/json.result @@ -0,0 +1,141 @@ +test_run = require('test_run').new() +--- +... +-- +-- Lookup in the primary index when applying a deferred DELETE +-- for a secondary index on commit. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +--- +... +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +--- +... +sk = s:create_index('sk', {unique = false, parts = {2, 'unsigned'}}) +--- +... +s:replace{{a = 1, b = 2, c = 3}, 10} +--- +- [{'b': 2, 'a': 1, 'c': 3}, 10] +... +sk:select() +--- +- - [{'b': 2, 'a': 1, 'c': 3}, 10] +... +s:drop() +--- +... +-- +-- Lookup on INSERT to check the unique constraint. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +--- +... +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +--- +... +s:replace{{a = 1, b = 2, c = 3}, 1} +--- +- [{'b': 2, 'a': 1, 'c': 3}, 1] +... +box.snapshot() +--- +- ok +... +s:replace{{a = 1, b = 2, c = 3}, 2} +--- +- [{'b': 2, 'a': 1, 'c': 3}, 2] +... +s:insert{{a = 1, b = 2, c = 3}, 3} +--- +- error: Duplicate key exists in unique index 'pk' in space 'test' +... +pk:stat().disk.iterator.lookup -- 0 (served from memory) +--- +- 0 +... +s:drop() +--- +... +-- +-- Gap locks coalescing. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +--- +... +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +--- +... +s:replace{{a = 1, b = 1, c = 1}} +--- +- [{'b': 1, 'a': 1, 'c': 1}] +... +s:replace{{a = 1, b = 1, c = 2}} +--- +- [{'b': 1, 'a': 1, 'c': 2}] +... +box.begin() +--- +... +gap_locks_1 = box.stat.vinyl().tx.gap_locks +--- +... +s:select({1, 1}, {iterator = 'ge', limit = 1}) +--- +- - [{'b': 1, 'a': 1, 'c': 1}] +... +s:select({1, 1}, {iterator = 'gt'}) +--- +- [] +... +gap_locks_2 = box.stat.vinyl().tx.gap_locks +--- +... +gap_locks_2 - gap_locks_1 -- 2 (tracking intervals must not be coalesced) +--- +- 2 +... +box.commit() +--- +... +s:drop() +--- +... +-- +-- Cache iterator stop condition. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +--- +... +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +--- +... +s:replace{{a = 1, b = 1, c = 1}} +--- +- [{'b': 1, 'a': 1, 'c': 1}] +... +s:replace{{a = 1, b = 1, c = 2}} +--- +- [{'b': 1, 'a': 1, 'c': 2}] +... +s:replace{{a = 1, b = 1, c = 3}} +--- +- [{'b': 1, 'a': 1, 'c': 3}] +... +s:insert{{a = 1, b = 1, c = 3}} +--- +- error: Duplicate key exists in unique index 'pk' in space 'test' +... +s:select{1, 1, 1} +--- +- - [{'b': 1, 'a': 1, 'c': 1}] +... +s:select{1, 1} +--- +- - [{'b': 1, 'a': 1, 'c': 1}] + - [{'b': 1, 'a': 1, 'c': 2}] + - [{'b': 1, 'a': 1, 'c': 3}] +... +s:drop() +--- +... diff --git a/test/vinyl/json.test.lua b/test/vinyl/json.test.lua new file mode 100644 index 00000000..2f9f9f6e --- /dev/null +++ b/test/vinyl/json.test.lua @@ -0,0 +1,53 @@ +test_run = require('test_run').new() + +-- +-- Lookup in the primary index when applying a deferred DELETE +-- for a secondary index on commit. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +sk = s:create_index('sk', {unique = false, parts = {2, 'unsigned'}}) +s:replace{{a = 1, b = 2, c = 3}, 10} +sk:select() +s:drop() + +-- +-- Lookup on INSERT to check the unique constraint. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +s:replace{{a = 1, b = 2, c = 3}, 1} +box.snapshot() +s:replace{{a = 1, b = 2, c = 3}, 2} +s:insert{{a = 1, b = 2, c = 3}, 3} +pk:stat().disk.iterator.lookup -- 0 (served from memory) +s:drop() + +-- +-- Gap locks coalescing. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +s:replace{{a = 1, b = 1, c = 1}} +s:replace{{a = 1, b = 1, c = 2}} +box.begin() +gap_locks_1 = box.stat.vinyl().tx.gap_locks +s:select({1, 1}, {iterator = 'ge', limit = 1}) +s:select({1, 1}, {iterator = 'gt'}) +gap_locks_2 = box.stat.vinyl().tx.gap_locks +gap_locks_2 - gap_locks_1 -- 2 (tracking intervals must not be coalesced) +box.commit() +s:drop() + +-- +-- Cache iterator stop condition. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +pk = s:create_index('pk', {parts = { {'[1].a', 'unsigned'}, {'[1].b', 'unsigned'}, {'[1].c', 'unsigned'} }}) +s:replace{{a = 1, b = 1, c = 1}} +s:replace{{a = 1, b = 1, c = 2}} +s:replace{{a = 1, b = 1, c = 3}} +s:insert{{a = 1, b = 1, c = 3}} +s:select{1, 1, 1} +s:select{1, 1} +s:drop()