From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: Re: [PATCH 07/12] vinyl: sanitize full/empty key stmt detection Date: Fri, 1 Mar 2019 15:57:36 +0300 [thread overview] Message-ID: <20190301125736.bppcf26bt3xvxozx@esperanza> (raw) In-Reply-To: <b89cd218b341a05d5f4d28f7aeb43df594433d4e.1550744027.git.vdavydov.dev@gmail.com> 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 <vdavydov.dev@gmail.com> 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()
next prev parent reply other threads:[~2019-03-01 12:57 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-21 10:26 [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov 2019-02-21 10:26 ` [PATCH 01/12] vinyl: use vy_lsm_env::empty_key where appropriate Vladimir Davydov 2019-02-21 10:59 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 10:26 ` [PATCH 02/12] vinyl: make vy_tuple_delete static Vladimir Davydov 2019-02-21 11:00 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 10:26 ` [PATCH 03/12] key_def: cleanup virtual function initialization Vladimir Davydov 2019-02-21 11:01 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 12:05 ` Vladimir Davydov 2019-02-21 10:26 ` [PATCH 04/12] key_def: move cmp and hash functions declarations to key_def.h Vladimir Davydov 2019-02-21 11:02 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 10:26 ` [PATCH 05/12] vinyl: move vy_tuple_key_contains_null to generic code Vladimir Davydov 2019-02-21 11:02 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 10:26 ` [PATCH 06/12] vinyl: move vy_key_dup " Vladimir Davydov 2019-02-21 11:04 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 11:52 ` Vladimir Davydov 2019-02-21 10:26 ` [PATCH 07/12] vinyl: sanitize full/empty key stmt detection Vladimir Davydov 2019-02-21 11:10 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 12:11 ` Vladimir Davydov 2019-03-01 12:57 ` Vladimir Davydov [this message] 2019-02-21 10:26 ` [PATCH 08/12] vinyl: remove optimized comparators Vladimir Davydov 2019-02-21 11:11 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 10:26 ` [PATCH 09/12] vinyl: introduce statement environment Vladimir Davydov 2019-02-21 11:14 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 10:26 ` [PATCH 10/12] vinyl: rename key stmt construction routine Vladimir Davydov 2019-02-21 11:15 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 12:14 ` Vladimir Davydov 2019-02-21 10:26 ` [PATCH 11/12] vinyl: don't use IPROTO_SELECT type for key statements Vladimir Davydov 2019-02-21 11:16 ` [tarantool-patches] " Konstantin Osipov 2019-02-21 10:26 ` [PATCH 12/12] vinyl: do not fill secondary tuples with nulls when decoded Vladimir Davydov 2019-02-21 15:39 ` [PATCH 00/12] vinyl: do not fill secondary tuples with nulls Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190301125736.bppcf26bt3xvxozx@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH 07/12] vinyl: sanitize full/empty key stmt detection' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox