Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: tarantool-patches@freelists.org
Subject: [PATCH 4/4] schema: explicitly forbid setting sequence for json path key part
Date: Wed, 15 May 2019 13:33:49 +0300	[thread overview]
Message-ID: <e8aa7c1d8c2a06fa08583c2740031f41e8baa6fd.1557916311.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1557916311.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1557916311.git.vdavydov.dev@gmail.com>

When a space has a sequence, we substitute NULL in the corresponding
primary index part with the next value generated by the sequence. We do
this by patching raw msgpack, see request_handle_sequence. The problem
is we can't do it easily if the field is nested. For example, consider
field [1].a.b. In Lua it is impossible to create tuple {{a = {b = nil}}}
so we would need to restore the whole path from an empty tuple. This
isn't trivial to do. Let's forbid it for now as nobody has requested
this feature yet.

See #4210

@TarantoolBot document
Title: Document that a sequence can't be used with json path key part

This will result in index create/alter error:

```
box.schema.space.create('test')
box.space.test:create_index('primary', {
    parts = {{'[1][1]', 'unsigned'}},
    sequence = true
})
```

One can set a sequence for a plain part of a json path index though,
i.e. this is okay:

```
box.schema.space.create('test')
box.space.test:create_index('primary', {
    parts = {{'[1][1]', 'unsigned'}, {2, 'unsigned}},
    sequence = true, sequence_part = 2
})
```
---
 src/box/alter.cc           |  8 +++++-
 src/box/lua/schema.lua     |  4 +++
 test/box/sequence.result   | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 test/box/sequence.test.lua | 20 ++++++++++++++
 4 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 2d43a9d2..a07a0845 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -131,7 +131,13 @@ index_def_check_sequence(struct index_def *index_def, uint32_t sequence_part,
 		tnt_raise(ClientError, ER_MODIFY_INDEX, index_def->name,
 			  space_name, "sequence part is out of bounds");
 	}
-	enum field_type type = index_def->key_def->parts[sequence_part].type;
+	struct key_part *part = &index_def->key_def->parts[sequence_part];
+	if (part->path != NULL) {
+		tnt_raise(ClientError, ER_MODIFY_INDEX, index_def->name,
+			  space_name, "sequence cannot be used with "
+			  "a json path key");
+	}
+	enum field_type type = part->type;
 	if (type != FIELD_TYPE_UNSIGNED && type != FIELD_TYPE_INTEGER) {
 		tnt_raise(ClientError, ER_MODIFY_INDEX, index_def->name,
 			  space_name, "sequence cannot be used with "
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 91900395..8f54f444 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -755,6 +755,10 @@ local function check_sequence_part(parts, sequence_part,
                   "sequence part is out of bounds")
     end
     sequence_part = parts[sequence_part]
+    if sequence_part.path ~= nil then
+        box.error(box.error.MODIFY_INDEX, index_name, space_name,
+                  "sequence cannot be used with a json path key")
+    end
     local sequence_part_type = sequence_part.type or sequence_part[2]
     if sequence_part_type ~= 'integer' and sequence_part_type ~= 'unsigned' then
         box.error(box.error.MODIFY_INDEX, index_name, space_name,
diff --git a/test/box/sequence.result b/test/box/sequence.result
index 4f962347..5d2d8a4f 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -2033,3 +2033,72 @@ s:insert{'d', box.null, 1001}
 s:drop()
 ---
 ...
+--
+-- gh-4210: sequence cannot be used with json path indexes.
+--
+sq = box.schema.sequence.create('test')
+---
+...
+s = box.schema.space.create('test')
+---
+...
+s:create_index('pk', {parts = {{'[1][1]', 'unsigned'}}, sequence = true}) -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+s:create_index('pk', {parts = {{1, 'unsigned', path = '[1]'}}, sequence = true}) -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+_ = s:create_index('pk', {parts = {{1, 'unsigned'}, {'[2][1]', 'unsigned'}}, sequence = true}) -- ok
+---
+...
+s.index.pk:alter{sequence_part = 2} -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+box.space._space_sequence:update(s.id, {{'=', 4, 1}}) -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+s.index.pk:alter{parts = {{'[1][1]', 'unsigned'}}} -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+s.index.pk:alter{parts = {{1, 'unsigned', path = '[1]'}}} -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+box.space._index:update({s.id, 0}, {{'=', 6, {{field = 1, type = 'unsigned', path = '[1]'}}}}) -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+s.index.pk:alter{sequence = false} -- ok
+---
+...
+s.index.pk:alter{sequence = true, sequence_part = 2} -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+box.space._space_sequence:insert{s.id, sq.id, false, 1} -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a json path key'
+...
+s.index.pk:alter{sequence = true} -- ok
+---
+...
+s:drop()
+---
+...
+sq:drop()
+---
+...
diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua
index d419e369..3fa0bef7 100644
--- a/test/box/sequence.test.lua
+++ b/test/box/sequence.test.lua
@@ -685,3 +685,23 @@ s.index.pk.sequence_id == sequence_id
 s:insert{'d', 1000, 1000}
 s:insert{'d', box.null, 1001}
 s:drop()
+
+--
+-- gh-4210: sequence cannot be used with json path indexes.
+--
+sq = box.schema.sequence.create('test')
+s = box.schema.space.create('test')
+s:create_index('pk', {parts = {{'[1][1]', 'unsigned'}}, sequence = true}) -- error
+s:create_index('pk', {parts = {{1, 'unsigned', path = '[1]'}}, sequence = true}) -- error
+_ = s:create_index('pk', {parts = {{1, 'unsigned'}, {'[2][1]', 'unsigned'}}, sequence = true}) -- ok
+s.index.pk:alter{sequence_part = 2} -- error
+box.space._space_sequence:update(s.id, {{'=', 4, 1}}) -- error
+s.index.pk:alter{parts = {{'[1][1]', 'unsigned'}}} -- error
+s.index.pk:alter{parts = {{1, 'unsigned', path = '[1]'}}} -- error
+box.space._index:update({s.id, 0}, {{'=', 6, {{field = 1, type = 'unsigned', path = '[1]'}}}}) -- error
+s.index.pk:alter{sequence = false} -- ok
+s.index.pk:alter{sequence = true, sequence_part = 2} -- error
+box.space._space_sequence:insert{s.id, sq.id, false, 1} -- error
+s.index.pk:alter{sequence = true} -- ok
+s:drop()
+sq:drop()
-- 
2.11.0

  parent reply	other threads:[~2019-05-15 10:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-15 10:33 [PATCH 0/4] A few fixes/improvements for autoincrement indexes Vladimir Davydov
2019-05-15 10:33 ` [PATCH 1/4] schema: use tuple field names in Lua Vladimir Davydov
2019-05-15 10:33 ` [PATCH 2/4] schema: fix error while altering index with sequence Vladimir Davydov
2019-05-16  7:45   ` [tarantool-patches] " Konstantin Osipov
2019-05-15 10:33 ` [PATCH 3/4] schema: allow to set sequence for any index part, not just the first Vladimir Davydov
2019-05-16  7:45   ` [tarantool-patches] " Konstantin Osipov
2019-05-16  8:02     ` Vladimir Davydov
2019-05-15 10:33 ` Vladimir Davydov [this message]
2019-05-15 13:00   ` [tarantool-patches] Re: [PATCH 4/4] schema: explicitly forbid setting sequence for json path key part Konstantin Osipov
2019-05-15 13:11     ` Vladimir Davydov
2019-05-15 13:16       ` Vladimir Davydov
2019-05-15 13:44         ` [PATCH] box: fix autoincrement for json path indexes Vladimir Davydov
2019-05-16  7:42           ` [tarantool-patches] " Konstantin Osipov
2019-05-21 13:28             ` Vladimir Davydov
2019-05-21 10:42 ` [tarantool-patches] Re: [PATCH 0/4] A few fixes/improvements for autoincrement indexes Kirill Yukhin
2019-05-21 14:58   ` Konstantin Osipov
2019-05-21 16:02     ` Kirill Yukhin

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=e8aa7c1d8c2a06fa08583c2740031f41e8baa6fd.1557916311.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 4/4] schema: explicitly forbid setting sequence for json path key part' \
    /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