From 6fb36d4e0bf7103f9ba1637fba1b9abe00e415c5 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Thu, 8 Jun 2023 10:55:07 +0300 Subject: [PATCH] ddl: add basic fieldno path support for indexes It is the compatibility layer for Tarantool spaces created not through ddl: users are not encouraged to create fieldno path indexes in ddl schema explicitly, since we do not support them as sharding keys. Closes #108 --- CHANGELOG.md | 5 +++ ddl/check.lua | 32 ++++++++++---- test/check_schema_test.lua | 60 ++++++++++++++++++++++++--- test/get_schema_test.lua | 85 ++++++++++++++++++++++++++++++++++++++ test/set_schema_test.lua | 80 +++++++++++++++++++++++++++++++++++ 5 files changed, 250 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c81c62..fbf9a3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- Add basic fieldno path support for indexes (#108). + It is the compatibility layer for Tarantool spaces created not through ddl: + users are not encouraged to create fieldno path indexes in schema, + since we do not support them as sharding keys. + ## [1.6.2] - 2022-08-22 ### Fixed diff --git a/ddl/check.lua b/ddl/check.lua index d76fd61..a07a664 100644 --- a/ddl/check.lua +++ b/ddl/check.lua @@ -99,10 +99,14 @@ local function check_field(i, field, space) end local function is_path_multikey(path) - return string.find(path, '[*]', 1, true) ~= nil + return (type(path) == 'string') and (string.find(path, '[*]', 1, true) ~= nil) end local function get_path_info(path) + if type(path) == 'number' then + return {field_name = path, type = 'regular'} + end + local multikey_start, _ = string.find(path, '[*]', 1, true) local json_start, _ = string.find(path, '.', 1, true) @@ -130,16 +134,19 @@ end local function check_index_part_path(path, index, space) - if type(path) ~= 'string' then + if (type(path) ~= 'string') and (type(path) ~= 'number') then return nil, string.format( - "bad value (string expected, got %s)", + "bad value (string|number expected, got %s)", type(path) ) end local path_info = get_path_info(path) - do -- check index.part.path references to existing field - if not space.fields[path_info.field_name] then + do + -- Check index.part.path references to existing field. + -- If index path is a fieldno, core Tarantool space rules + -- allows it to be outside the format. + if type(path_info.field_name) == 'string' and not space.fields[path_info.field_name] then return nil, string.format( "path (%s) referencing to unknown field", path @@ -286,6 +293,8 @@ local function check_index_part(i, index, space) end local path_info = get_path_info(part.path) + -- If field_name is fieldno, it is allowed to be outside of format. + -- space_format_field will be nil in this case. local space_format_field = space.fields[path_info.field_name] do -- check part.type is valid and it is valid for index type local ok, err = check_index_part_type(part.type, index.type) @@ -312,7 +321,12 @@ local function check_index_part(i, index, space) } do -- check index.part.type equals format.field.type - if path_info.type == 'regular' and space_format_field.type ~= 'any' then + if ( + path_info.type == 'regular' and + space_format_field ~= nil and + space_format_field.type ~= 'any' + ) + then if not ( (part.type == 'scalar' and scalar_types[space_format_field.type]) or (space_format_field.type == 'scalar' and scalar_types[part.type]) @@ -358,7 +372,11 @@ local function check_index_part(i, index, space) end do -- check the same nullability - if space_format_field.is_nullable ~= part.is_nullable then + if ( + space_format_field ~= nil and + space_format_field.is_nullable ~= part.is_nullable + ) + then return nil, string.format( "spaces[%q].indexes[%q].parts[%d].is_nullable: has different nullability with " .. "spaces[%q].format[%q].is_nullable (%s expected, got %s)", diff --git a/test/check_schema_test.lua b/test/check_schema_test.lua index 26d3a21..e52f950 100644 --- a/test/check_schema_test.lua +++ b/test/check_schema_test.lua @@ -203,11 +203,7 @@ function g.test_index_part_path() local ok, err = ddl_check.check_index_part_path(nil, index_info, test_space_info) t.assert_not(ok) - t.assert_equals(err, "bad value (string expected, got nil)") - - local ok, err = ddl_check.check_index_part_path(5, index_info, test_space_info) - t.assert_not(ok) - t.assert_equals(err, "bad value (string expected, got number)") + t.assert_equals(err, "bad value (string|number expected, got nil)") local ok, err = ddl_check.check_index_part_path('unsigned_nonnull', index_info, test_space_info) @@ -1404,3 +1400,57 @@ function g.test_transactional_ddl() end t.assert_not(box.is_in_txn()) end + +g.test_gh_108_fieldno_index_no_space_format = function() + local _, err = ddl.check_schema({spaces = {weird_space = { + engine = 'memtx', + is_local = false, + temporary = false, + format = {}, + indexes = {{ + name = 'pk', + type = 'TREE', + parts = { + {path = 1, type = 'unsigned', is_nullable = false}, + }, + unique = true, + }}, + }}}) + t.assert_equals(err, nil) +end + +g.test_gh_108_fieldno_index_in_space_format = function() + local _, err = ddl.check_schema({spaces = {weird_space = { + engine = 'memtx', + is_local = false, + temporary = false, + format = {{name = 'id', type = 'unsigned', is_nullable = false}}, + indexes = {{ + name = 'pk', + type = 'TREE', + parts = { + {path = 1, type = 'unsigned', is_nullable = false}, + }, + unique = true, + }}, + }}}) + t.assert_equals(err, nil) +end + +g.test_gh_108_fieldno_index_outside_space_format = function() + local _, err = ddl.check_schema({spaces = {weird_space = { + engine = 'memtx', + is_local = false, + temporary = false, + format = {{name = 'id', type = 'unsigned', is_nullable = false}}, + indexes = {{ + name = 'pk', + type = 'TREE', + parts = { + {path = 2, type = 'string', is_nullable = false}, + }, + unique = true, + }}, + }}}) + t.assert_equals(err, nil) +end diff --git a/test/get_schema_test.lua b/test/get_schema_test.lua index 88ca0c1..7d0f871 100644 --- a/test/get_schema_test.lua +++ b/test/get_schema_test.lua @@ -803,3 +803,88 @@ function g.test_get_schema_with_default_values() } }) end + +g.test_gh_108_fieldno_index_no_space_format = function() + box.schema.space.create('weird_space') + box.space['weird_space']:create_index('pk') + + local schema, err = ddl.get_schema() + t.assert_equals(err, nil) + t.assert_equals(schema, {spaces = {weird_space = { + engine = "memtx", + format = {}, + indexes = { + { + name = "pk", + parts = {{is_nullable = false, path = 1, type = "unsigned"}}, + type = "TREE", + unique = true, + }, + }, + is_local = false, + temporary = false, + }}}) + + local _, err = ddl.check_schema(schema) + t.assert_equals(err, nil) +end + +g.test_gh_108_fieldno_index_in_space_format = function() + box.schema.space.create('weird_space', { + format = {{name = 'id', type = 'unsigned', is_nullable = false}} + }) + box.space['weird_space']:create_index('pk', { + unique = true, + parts = {{field = 1, type = 'unsigned', is_nullable = false}} + }) + + local schema, err = ddl.get_schema() + t.assert_equals(err, nil) + t.assert_equals(schema, {spaces = {weird_space = { + engine = "memtx", + format = {{is_nullable = false, name = "id", type = "unsigned"}}, + indexes = { + { + name = "pk", + parts = {{is_nullable = false, path = "id", type = "unsigned"}}, + type = "TREE", + unique = true, + }, + }, + is_local = false, + temporary = false, + }}}) + + local _, err = ddl.check_schema(schema) + t.assert_equals(err, nil) +end + +g.test_gh_108_fieldno_index_outside_space_format = function() + box.schema.space.create('weird_space', { + format = {{name = 'id', type = 'unsigned', is_nullable = false}} + }) + box.space['weird_space']:create_index('pk', { + unique = true, + parts = {{field = 2, type = 'string', is_nullable = false}} + }) + + local schema, err = ddl.get_schema() + t.assert_equals(err, nil) + t.assert_equals(schema, {spaces = {weird_space = { + engine = "memtx", + format = {{is_nullable = false, name = "id", type = "unsigned"}}, + indexes = { + { + name = "pk", + parts = {{is_nullable = false, path = 2, type = "string"}}, + type = "TREE", + unique = true, + }, + }, + is_local = false, + temporary = false, + }}}) + + local _, err = ddl.check_schema(schema) + t.assert_equals(err, nil) +end diff --git a/test/set_schema_test.lua b/test/set_schema_test.lua index 97a4a59..e02098e 100644 --- a/test/set_schema_test.lua +++ b/test/set_schema_test.lua @@ -1307,3 +1307,83 @@ function g.test_transactional_ddl() box.space._index:on_replace(nil, actual_failure) end + +g.test_gh_108_fieldno_index_no_space_format = function() + local _, err = ddl.set_schema({spaces = {weird_space = { + engine = 'memtx', + is_local = false, + temporary = false, + format = {}, + indexes = {{ + name = 'pk', + type = 'TREE', + parts = { + {path = 1, type = 'unsigned', is_nullable = false}, + }, + unique = true, + }}, + }}}) + t.assert_equals(err, nil) + + t.assert_equals(box.space['weird_space']:format(), {}) + t.assert_equals( + box.space['weird_space'].index['pk'].parts, + {{fieldno = 1, type = 'unsigned', is_nullable = false}}) + + t.assert_equals(box.space['weird_space']:insert{1}, {1}) + t.assert_equals(box.space['weird_space']:insert{2, 'val'}, {2, 'val'}) +end + +g.test_gh_108_fieldno_index_in_space_format = function() + local _, err = ddl.set_schema({spaces = {weird_space = { + engine = 'memtx', + is_local = false, + temporary = false, + format = {{name = 'id', type = 'unsigned', is_nullable = false}}, + indexes = {{ + name = 'pk', + type = 'TREE', + parts = { + {path = 1, type = 'unsigned', is_nullable = false}, + }, + unique = true, + }}, + }}}) + t.assert_equals(err, nil) + + t.assert_equals(box.space['weird_space']:format(), + {{is_nullable = false, name = "id", type = "unsigned"}}) + t.assert_equals( + box.space['weird_space'].index['pk'].parts, + {{fieldno = 1, type = 'unsigned', is_nullable = false}}) + + t.assert_equals(box.space['weird_space']:insert{1}, {1}) + t.assert_equals(box.space['weird_space']:insert{2, 'val'}, {2, 'val'}) +end + +g.test_gh_108_fieldno_index_outside_space_format = function() + local _, err = ddl.set_schema({spaces = {weird_space = { + engine = 'memtx', + is_local = false, + temporary = false, + format = {{name = 'id', type = 'unsigned', is_nullable = false}}, + indexes = {{ + name = 'pk', + type = 'TREE', + parts = { + {path = 2, type = 'string', is_nullable = false}, + }, + unique = true, + }}, + }}}) + t.assert_equals(err, nil) + + t.assert_equals(box.space['weird_space']:format(), + {{is_nullable = false, name = "id", type = "unsigned"}}) + t.assert_equals( + box.space['weird_space'].index['pk'].parts, + {{fieldno = 2, type = 'string', is_nullable = false}}) + + t.assert_equals(box.space['weird_space']:insert{1, 'val'}, {1, 'val'}) + t.assert_equals(box.space['weird_space']:insert{2, 'val2', 3}, {2, 'val2', 3}) +end