Re: [PATCH v9 6/6] box: specify indexes in user-friendly form

[Vladimir Davydov <vdavydov.dev@gmail.com>]

On Sun, Feb 03, 2019 at 01:20:26PM +0300, Kirill Shcherbatov wrote:
> Implemented a more convenient interface for creating an index
> by JSON path. Instead of specifying fieldno and relative path
> it is now possible to pass full JSON path to data.
> 
> Closes #1012
> 
> @TarantoolBot document
> Title: Indexes by JSON path
> Sometimes field data could have complex document structure.
> When this structure is consistent across whole space,
> you are able to create an index by JSON path.
> 
> Example:
> s = box.schema.space.create('sample')
> format = {{'id', 'unsigned'}, {'data', 'map'}}
> s:format(format)
> -- explicit JSON index creation
> age_idx = s:create_index('age', {{2, 'number', path = "age"}})
> -- user-friendly syntax for JSON index creation
> parts = {{'data.FIO["fname"]', 'str'}, {'data.FIO["sname"]', 'str'},
>      {'data.age', 'number'}}
> info_idx = s:create_index('info', {parts = parts}})
> s:insert({1, {FIO={fname="James", sname="Bond"}, age=35}})
> ---
>  src/box/lua/schema.lua    | 100 ++++++++++++++++++++++++++++++--------
>  test/engine/json.result   |  94 +++++++++++++++++++++++++++++++++++
>  test/engine/json.test.lua |  27 ++++++++++
>  3 files changed, 202 insertions(+), 19 deletions(-)
> 
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 8a804f0ba..aa9fd4b96 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -575,6 +575,78 @@ local function update_index_parts_1_6_0(parts)
>      return result
>  end
>  
> +--
> +-- Get field index by format field name.
> +--
> +local function format_field_index_by_name(format, name)
> +    for k, v in pairs(format) do
> +        if v.name == name then
> +            return k
> +        end
> +    end
> +    return nil
> +end
> +
> +--
> +-- Get field 0-based index and relative JSON path to data by
> +-- field 1-based index or full JSON path. A particular case of a
> +-- full JSON path is the format field name.
> +--
> +local function format_field_resolve(format, path, part_idx)
> +    assert(type(path) == 'number' or type(path) == 'string')
> +    local idx = nil
> +    local relative_path = nil
> +    local field_name = nil
> +    -- Path doesn't require resolve.
> +    if type(path) == 'number' then
> +        idx = path
> +        goto done
> +    end
> +    -- An attempt to interpret a path as the full field name.
> +    idx = format_field_index_by_name(format, path)
> +    if idx ~= nil then
> +        relative_path = nil
> +        goto done
> +    end
> +    -- Check if the initial part of the JSON path is a token of
> +    -- the form [%d].
> +    field_name = string.match(path, "^%[(%d+)%]")
> +    idx = tonumber(field_name)
> +    if idx ~= nil then
> +        relative_path = string.sub(path, string.len(field_name) + 3)
> +        goto done
> +    end
> +    -- Check if the initial part of the JSON path is a token of
> +    -- the form ["%s"] or ['%s'].
> +    field_name = string.match(path, '^%[\"([^%]]+)\"%].*') or
> +                 string.match(path, "^%[\'([^%]]+)\'%].*")

Masking quotes is redundant here, as well as .* at the end of the regexp.
I removed them.

> diff --git a/test/engine/json.test.lua b/test/engine/json.test.lua
> index 181eae02c..7a67e341e 100644
> --- a/test/engine/json.test.lua
> +++ b/test/engine/json.test.lua
> @@ -34,6 +34,33 @@ idx:min()
>  idx:max()
>  s:drop()
>  
> +-- Test user-friendly index creation interface.
> +s = box.schema.space.create('withdata', {engine = engine})
> +format = {{'data', 'map'}, {'meta', 'str'}}
> +s:format(format)
> +s:create_index('pk_invalid', {parts = {{']sad.FIO["sname"]', 'str'}}})
> +s:create_index('pk_unexistent', {parts = {{'unexistent.FIO["sname"]', 'str'}}})
> +pk = s:create_index('pk', {parts = {{'data.FIO["sname"]', 'str'}}})
> +pk ~= nil
> +sk2 = s:create_index('sk2', {parts = {{'["data"]FIO["sname"]', 'str'}}})
> +sk2 ~= nil
> +sk3 = s:create_index('sk3', {parts = {{'[\'data\']FIO["sname"]', 'str'}}})
> +sk3 ~= nil
> +sk4 = s:create_index('sk4', {parts = {{'[1]FIO["sname"]', 'str'}}})

Strictly speaking, this isn't a correct JSON path - a dot is missing
before FIO. I guess it's OK that we allow this, but in tests we'd better
use the correct form so as not to confuse the reader. I added the
missing dots.

Pushed to 2.1. My incremental diff is below.

diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index aa9fd4b9..34428c91 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -618,8 +618,8 @@ local function format_field_resolve(format, path, part_idx)
     end
     -- Check if the initial part of the JSON path is a token of
     -- the form ["%s"] or ['%s'].
-    field_name = string.match(path, '^%[\"([^%]]+)\"%].*') or
-                 string.match(path, "^%[\'([^%]]+)\'%].*")
+    field_name = string.match(path, '^%["([^%]]+)"%]') or
+                 string.match(path, "^%['([^%]]+)'%]")
     idx = format_field_index_by_name(format, field_name)
     if idx ~= nil then
         relative_path = string.sub(path, string.len(field_name) + 5)
diff --git a/test/engine/json.result b/test/engine/json.result
index 2d50976e..1bac85ed 100644
--- a/test/engine/json.result
+++ b/test/engine/json.result
@@ -147,21 +147,21 @@ pk ~= nil
 ---
 - true
 ...
-sk2 = s:create_index('sk2', {parts = {{'["data"]FIO["sname"]', 'str'}}})
+sk2 = s:create_index('sk2', {parts = {{'["data"].FIO["sname"]', 'str'}}})
 ---
 ...
 sk2 ~= nil
 ---
 - true
 ...
-sk3 = s:create_index('sk3', {parts = {{'[\'data\']FIO["sname"]', 'str'}}})
+sk3 = s:create_index('sk3', {parts = {{'[\'data\'].FIO["sname"]', 'str'}}})
 ---
 ...
 sk3 ~= nil
 ---
 - true
 ...
-sk4 = s:create_index('sk4', {parts = {{'[1]FIO["sname"]', 'str'}}})
+sk4 = s:create_index('sk4', {parts = {{'[1].FIO["sname"]', 'str'}}})
 ---
 ...
 sk4 ~= nil
diff --git a/test/engine/json.test.lua b/test/engine/json.test.lua
index 7a67e341..9afa3daa 100644
--- a/test/engine/json.test.lua
+++ b/test/engine/json.test.lua
@@ -42,11 +42,11 @@ s:create_index('pk_invalid', {parts = {{']sad.FIO["sname"]', 'str'}}})
 s:create_index('pk_unexistent', {parts = {{'unexistent.FIO["sname"]', 'str'}}})
 pk = s:create_index('pk', {parts = {{'data.FIO["sname"]', 'str'}}})
 pk ~= nil
-sk2 = s:create_index('sk2', {parts = {{'["data"]FIO["sname"]', 'str'}}})
+sk2 = s:create_index('sk2', {parts = {{'["data"].FIO["sname"]', 'str'}}})
 sk2 ~= nil
-sk3 = s:create_index('sk3', {parts = {{'[\'data\']FIO["sname"]', 'str'}}})
+sk3 = s:create_index('sk3', {parts = {{'[\'data\'].FIO["sname"]', 'str'}}})
 sk3 ~= nil
-sk4 = s:create_index('sk4', {parts = {{'[1]FIO["sname"]', 'str'}}})
+sk4 = s:create_index('sk4', {parts = {{'[1].FIO["sname"]', 'str'}}})
 sk4 ~= nil
 pk.fieldno == sk2.fieldno
 sk2.fieldno == sk3.fieldno