From 7b10250ef283ccdd7684529c1ebdf7a2e8cd5d68 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 15 Mar 2024 18:12:29 +0300 Subject: [PATCH 1/8] utils: determine features on require There seem to be no reason to determine features on request instead of require since this operation is cheap. Part of #422 --- crud/common/utils.lua | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/crud/common/utils.lua b/crud/common/utils.lua index b1023dc5..d06fb9a8 100644 --- a/crud/common/utils.lua +++ b/crud/common/utils.lua @@ -664,51 +664,29 @@ local function determine_enabled_features() 2, 2, 0, nil) end -function utils.tarantool_supports_fieldpaths() - if enabled_tarantool_features.fieldpaths == nil then - determine_enabled_features() - end +determine_enabled_features() +function utils.tarantool_supports_fieldpaths() return enabled_tarantool_features.fieldpaths end function utils.tarantool_supports_uuids() - if enabled_tarantool_features.uuids == nil then - determine_enabled_features() - end - return enabled_tarantool_features.uuids end function utils.tarantool_supports_jsonpath_indexes() - if enabled_tarantool_features.jsonpath_indexes == nil then - determine_enabled_features() - end - return enabled_tarantool_features.jsonpath_indexes end function utils.tarantool_has_builtin_merger() - if enabled_tarantool_features.builtin_merger == nil then - determine_enabled_features() - end - return enabled_tarantool_features.builtin_merger end function utils.tarantool_supports_external_merger() - if enabled_tarantool_features.external_merger == nil then - determine_enabled_features() - end - return enabled_tarantool_features.external_merger end function utils.tarantool_supports_netbox_skip_header_option() - if enabled_tarantool_features.netbox_skip_header_option == nil then - determine_enabled_features() - end - return enabled_tarantool_features.netbox_skip_header_option end From b228899f907e12697fe966ce771d23b5e80ef089 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 15 Mar 2024 18:14:28 +0300 Subject: [PATCH 2/8] utils: generate support check utils Generate `tarantool_supports_X` utils instead of creating a manual function for each key with copypaste. Part of #422 --- crud/common/utils.lua | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/crud/common/utils.lua b/crud/common/utils.lua index d06fb9a8..1f95b8aa 100644 --- a/crud/common/utils.lua +++ b/crud/common/utils.lua @@ -666,28 +666,17 @@ end determine_enabled_features() -function utils.tarantool_supports_fieldpaths() - return enabled_tarantool_features.fieldpaths -end - -function utils.tarantool_supports_uuids() - return enabled_tarantool_features.uuids -end - -function utils.tarantool_supports_jsonpath_indexes() - return enabled_tarantool_features.jsonpath_indexes -end - -function utils.tarantool_has_builtin_merger() - return enabled_tarantool_features.builtin_merger -end +for feature_name, feature_enabled in pairs(enabled_tarantool_features) do + local util_name + if feature_name == 'builtin_merger' then + util_name = ('tarantool_has_%s'):format(feature_name) + else + util_name = ('tarantool_supports_%s'):format(feature_name) + end -function utils.tarantool_supports_external_merger() - return enabled_tarantool_features.external_merger -end + local util_func = function() return feature_enabled end -function utils.tarantool_supports_netbox_skip_header_option() - return enabled_tarantool_features.netbox_skip_header_option + utils[util_name] = util_func end local function add_nullable_fields_recursive(operations, operations_map, space_format, tuple, id) From cae3fb62338fd524c24b5a754886a1764adbb8e6 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 15 Mar 2024 18:27:55 +0300 Subject: [PATCH 3/8] filters: use proper checks for type support `pcall(require, module)` check is insufficient for external types since module presence often precedes box format support and index support for a type. Sometimes the nature of the module may change between versions: for example, there is a `'uuid'` module in 1.10.x, but a separate type was introduced only in 2.4.1 Part of #422 --- crud/common/utils.lua | 32 ++++++++++++++++++++++++++------ crud/compare/filters.lua | 10 +++++----- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/crud/common/utils.lua b/crud/common/utils.lua index 1f95b8aa..fbfc071a 100644 --- a/crud/common/utils.lua +++ b/crud/common/utils.lua @@ -6,8 +6,6 @@ local fun = require('fun') local vshard = require('vshard') local log = require('log') -local datetime_supported, datetime = pcall(require, 'datetime') - local is_cartridge, cartridge = pcall(require, 'cartridge') local is_cartridge_hotreload, cartridge_hotreload = pcall(require, 'cartridge.hotreload') @@ -605,10 +603,34 @@ local function determine_enabled_features() enabled_tarantool_features.fieldpaths = is_version_ge(major, minor, patch, suffix, 2, 3, 1, nil) - -- since Tarantool 2.4.1 + -- Full support (Lua type, space format type and indexes) for decimal type + -- is since Tarantool 2.3.1 [1] + -- + -- [1] https://github.com/tarantool/tarantool/commit/485439e33196e26d120e622175f88b4edc7a5aa1 + enabled_tarantool_features.decimals = is_version_ge(major, minor, patch, suffix, + 2, 3, 1, nil) + + -- Full support (Lua type, space format type and indexes) for uuid type + -- is since Tarantool 2.4.1 [1] + -- + -- [1] https://github.com/tarantool/tarantool/commit/b238def8065d20070dcdc50b54c2536f1de4c7c7 enabled_tarantool_features.uuids = is_version_ge(major, minor, patch, suffix, 2, 4, 1, nil) + -- Full support (Lua type, space format type and indexes) for datetime type + -- is since Tarantool 2.10.0-beta2 [1] + -- + -- [1] https://github.com/tarantool/tarantool/commit/3bd870261c462416c29226414fe0a2d79aba0c74 + enabled_tarantool_features.datetimes = is_version_ge(major, minor, patch, suffix, + 2, 10, 0, 'beta2') + + -- Full support (Lua type, space format type and indexes) for datetime type + -- is since Tarantool 2.10.0-rc1 [1] + -- + -- [1] https://github.com/tarantool/tarantool/commit/38f0c904af4882756c6dc802f1895117d3deae6a + enabled_tarantool_features.intervals = is_version_ge(major, minor, patch, suffix, + 2, 10, 0, 'rc1') + -- since Tarantool 2.6.3 / 2.7.2 / 2.8.1 enabled_tarantool_features.jsonpath_indexes = is_version_ge(major, minor, patch, suffix, 2, 8, 1, nil) @@ -1304,9 +1326,7 @@ function utils.is_cartridge_hotreload_supported() return true, cartridge_hotreload end -local interval_supported = datetime_supported and (datetime.interval ~= nil) - -if interval_supported then +if utils.tarantool_supports_intervals() then -- https://github.com/tarantool/tarantool/blob/0510ffa07afd84a70c9c6f1a4c28aacd73a393d6/src/lua/datetime.lua#L175-179 local interval_t = ffi.typeof('struct interval') diff --git a/crud/compare/filters.lua b/crud/compare/filters.lua index e3c8eb66..985a053e 100644 --- a/crud/compare/filters.lua +++ b/crud/compare/filters.lua @@ -1,8 +1,8 @@ -local datetime_supported, datetime = pcall(require, 'datetime') -local decimal_supported, decimal = pcall(require, 'decimal') - local errors = require('errors') +local _, datetime = pcall(require, 'datetime') +local _, decimal = pcall(require, 'decimal') + local utils = require('crud.common.utils') local dev_checks = require('crud.common.dev_checks') local collations = require('crud.common.collations') @@ -160,12 +160,12 @@ local function format_value(value) return tostring(value) elseif type(value) == 'boolean' then return tostring(value) - elseif decimal_supported and decimal.is_decimal(value) then + elseif utils.tarantool_supports_decimals() and decimal.is_decimal(value) then -- decimal supports comparison with string. return ("%q"):format(tostring(value)) elseif utils.is_uuid(value) then return ("%q"):format(value) - elseif datetime_supported and datetime.is_datetime(value) then + elseif utils.tarantool_supports_datetimes() and datetime.is_datetime(value) then return ("%q"):format(value:format()) elseif utils.is_interval(value) then -- As for Tarantool 3.0 and older, datetime intervals From 3fdd95081e0218e4d7fdf631aef018d8861d1b2e Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 15 Mar 2024 18:28:09 +0300 Subject: [PATCH 4/8] tests: reuse proper checks for type support Part of #422 --- test/entrypoint/srv_select/storage_init.lua | 10 +++------- test/helper.lua | 22 ++++++++++++++------- test/integration/read_scenario.lua | 9 +++++---- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/test/entrypoint/srv_select/storage_init.lua b/test/entrypoint/srv_select/storage_init.lua index 4e0208ee..cfcea23f 100644 --- a/test/entrypoint/srv_select/storage_init.lua +++ b/test/entrypoint/srv_select/storage_init.lua @@ -1,6 +1,3 @@ -local datetime_supported, datetime = pcall(require, 'datetime') -local decimal_supported, _ = pcall(require, 'decimal') - local crud_utils = require('crud.common.utils') return function() @@ -231,7 +228,7 @@ return function() if_not_exists = true, }) - if decimal_supported then + if crud_utils.tarantool_supports_decimals() then local decimal_format = { {name = 'id', type = 'unsigned'}, {name = 'bucket_id', type = 'unsigned'}, @@ -327,7 +324,7 @@ return function() }) end - if datetime_supported then + if crud_utils.tarantool_supports_datetimes() then local datetime_format = { {name = 'id', type = 'unsigned'}, {name = 'bucket_id', type = 'unsigned'}, @@ -423,8 +420,7 @@ return function() }) end - local interval_supported = datetime_supported and (datetime.interval ~= nil) - if interval_supported then + if crud_utils.tarantool_supports_intervals() then -- Interval is non-indexable. local interval_space = box.schema.space.create('interval', { if_not_exists = true, diff --git a/test/helper.lua b/test/helper.lua index 1bd0b281..24efb0e4 100644 --- a/test/helper.lua +++ b/test/helper.lua @@ -958,20 +958,28 @@ function helpers.prepare_ordered_data(g, space, expected_objects, bucket_id, ord t.assert_equals(objects, expected_objects) end +function helpers.is_decimal_supported() + return crud_utils.tarantool_supports_decimals() +end + +function helpers.is_datetime_supported() + return crud_utils.tarantool_supports_datetimes() +end + +function helpers.is_interval_supported() + return crud_utils.tarantool_supports_intervals() +end + function helpers.skip_decimal_unsupported() - local module_available, _ = pcall(require, 'decimal') - t.skip_if(not module_available, 'decimal is not supported') + t.skip_if(not helpers.is_decimal_supported(), 'decimal is not supported') end function helpers.skip_datetime_unsupported() - local module_available, _ = pcall(require, 'datetime') - t.skip_if(not module_available, 'datetime is not supported') + t.skip_if(not helpers.is_datetime_supported(), 'datetime is not supported') end function helpers.skip_interval_unsupported() - local datetime_supported, datetime = pcall(require, 'datetime') - local interval_supported = datetime_supported and (datetime.interval ~= nil) - t.skip_if(not interval_supported, 'interval is not supported') + t.skip_if(not helpers.is_interval_supported(), 'interval is not supported') end function helpers.merge_tables(t1, t2, ...) diff --git a/test/integration/read_scenario.lua b/test/integration/read_scenario.lua index 968cd3af..667bfb31 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -5,8 +5,9 @@ -- Scenarios here are for `srv_select` entrypoint. local t = require('luatest') -local datetime_supported, datetime = pcall(require, 'datetime') -local decimal_supported, decimal = pcall(require, 'decimal') + +local _, datetime = pcall(require, 'datetime') +local _, decimal = pcall(require, 'decimal') local helpers = require('test.helper') @@ -98,7 +99,7 @@ end local decimal_vals = {} -if decimal_supported then +if helpers.is_decimal_supported() then decimal_vals = { smallest_negative = decimal.new('-123456789012345678.987431234678392'), bigger_negative = decimal.new('-123456789012345678.987431234678391'), @@ -318,7 +319,7 @@ end local datetime_vals = {} -if datetime_supported then +if helpers.is_datetime_supported() then datetime_vals = { yesterday = datetime.new{ year = 2024, From 6e66c38eb4f8e284948f55dd8a29b28d397b9b58 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Mon, 18 Mar 2024 18:12:15 +0300 Subject: [PATCH 5/8] test: always restart cluster after server fail Test scenarios which imitate "server fail" leave test cluster malfunctioning even in cartridge case. Part of #422 --- test/integration/select_readview_test.lua | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua index 4c5283bd..cd4048a9 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2335,11 +2335,9 @@ end pgroup.after_test('test_stop_select', function(g) -- It seems more easy to restart the cluster rather then restore it -- original state. - if g.params.backend == helpers.backend.VSHARD then - helpers.stop_cluster(g.cluster, g.params.backend) - g.cluster = nil - init_cluster(g) - end + helpers.stop_cluster(g.cluster, g.params.backend) + g.cluster = nil + init_cluster(g) end) pgroup.test_select_switch_master = function(g) From e2cf222cfb538e3eae5e891ffe1baf6db13bd74c Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Mon, 18 Mar 2024 22:34:29 +0300 Subject: [PATCH 6/8] test: restore master after switchover tests Part of #422 --- test/integration/select_readview_test.lua | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua index cd4048a9..399ba834 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2392,6 +2392,11 @@ pgroup.test_select_switch_master = function(g) end +pgroup.after_test('test_select_switch_master', function(g) + local replicasets = helpers.get_test_cartridge_replicasets() + set_master(g.cluster, replicasets[2].uuid, replicasets[2].servers[1].instance_uuid) +end) + -- TODO: https://github.com/tarantool/crud/issues/383 pgroup.test_select_switch_master_first = function(g) helpers.skip_not_cartridge_backend(g.params.backend) @@ -2448,6 +2453,11 @@ pgroup.test_select_switch_master_first = function(g) end +pgroup.after_test('test_select_switch_master', function(g) + local replicasets = helpers.get_test_cartridge_replicasets() + set_master(g.cluster, replicasets[2].uuid, replicasets[2].servers[1].instance_uuid) +end) + -- TODO: https://github.com/tarantool/crud/issues/383 pgroup.test_select_closed_readview = function(g) helpers.insert_objects(g, 'customers', { From c70483d81e4a9bd44e64f4d8f4565fd9b78926d7 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Mon, 18 Mar 2024 22:39:57 +0300 Subject: [PATCH 7/8] test: disable fullscan warnings in read tests Part of #422 --- test/integration/select_readview_test.lua | 3 +++ test/integration/select_test.lua | 1 + 2 files changed, 4 insertions(+) diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua index 399ba834..cb3718ef 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2496,6 +2496,9 @@ end local function read_impl(cg, space, conditions, opts) return cg.cluster.main_server:exec(function(space, conditions, opts) + opts = table.deepcopy(opts) or {} + opts.fullscan = true + local crud = require('crud') local rv, err = crud.readview() t.assert_equals(err, nil) diff --git a/test/integration/select_test.lua b/test/integration/select_test.lua index b3b38e69..fed8064d 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -2271,6 +2271,7 @@ end local function read_impl(cg, space, conditions, opts) opts = table.deepcopy(opts) or {} opts.mode = 'write' + opts.fullscan = true local resp, err = cg.cluster.main_server:call('crud.select', {space, conditions, opts}) From 17dd0bb70d239c1409dfa2f6ab21738f497f1403 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 15 Mar 2024 18:32:04 +0300 Subject: [PATCH 8/8] scan: fix nil filters Before this patch, some nil conditions failed with internal filter library build error. This patch fixes this internal error, as well as normalize filters to behave similar to core indexes. The logic in core Tarantool select is as follows. `nil` condition in index select is an absence of condition, thus all data is returned disregarding the condition (condition may affect the order). `box.NULL` condition is a condition for the null value -- in case of EQ, only records with null index value are returned, in case of GT, all non-null values are returned since nulls are in the beginning of an index and so on. `nil`s and `box.NULL`s in tuple are both satisfy `box.NULL` equity. After this patch, `nil` filter condition is treated as no condition. This is a breaking change since conditions for `'>'` and `'<'` operator with `nil` operand had resulted with empty response before this patch. But since it was inconsistent with scanning index conditions and wasn't intentional, we change it here. Closes #422 --- CHANGELOG.md | 2 + crud/compare/filters.lua | 36 ++- test/helper.lua | 4 + test/integration/count_test.lua | 8 + test/integration/pairs_readview_test.lua | 8 + test/integration/pairs_test.lua | 8 + test/integration/read_scenario.lua | 327 ++++++++++++++++++++++ test/integration/select_readview_test.lua | 8 + test/integration/select_test.lua | 8 + 9 files changed, 396 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20928876..243646f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. non-iterating indexes (#373). * Passing errors from storages for merger operations (`crud.select`, `crud.pairs`, `readview:select`, `readview:pairs`) (#423). +* Working with `nil` operand conditions in case of non-indexed fields or + non-iterating indexes (#422). ## [1.4.3] - 05-02-24 diff --git a/crud/compare/filters.lua b/crud/compare/filters.lua index 985a053e..95061a0d 100644 --- a/crud/compare/filters.lua +++ b/crud/compare/filters.lua @@ -345,9 +345,14 @@ local function gen_eq_func_code(func_name, cond, func_args_code) local header = LIB_FUNC_HEADER_TEMPLATE:format(func_name, func_args_code) table.insert(func_code_lines, header) - local return_line = string.format( - ' return %s', concat_conditions(eq_conds, 'and') - ) + local return_line + if #eq_conds > 0 then + return_line = string.format( + ' return %s', concat_conditions(eq_conds, 'and') + ) + else -- nil condition is treated as no condition + return_line = ' return true' + end table.insert(func_code_lines, return_line) table.insert(func_code_lines, 'end') @@ -398,18 +403,23 @@ local function gen_cmp_array_func_code(operator, func_name, cond, func_args_code local results = results_by_operators[operator] assert(results ~= nil) - for i = 1, #eq_conds do - local comp_value_code = table.concat({ - string.format(' if %s then return %s end', lt_conds[i], results.le), - string.format(' if not %s then return %s end', eq_conds[i], results.not_eq), - '', - }, '\n') + if #eq_conds > 0 then + for i = 1, #eq_conds do + local comp_value_code = table.concat({ + string.format(' if %s then return %s end', lt_conds[i], results.le), + string.format(' if not %s then return %s end', eq_conds[i], results.not_eq), + '', + }, '\n') - table.insert(func_code_lines, comp_value_code) - end + table.insert(func_code_lines, comp_value_code) + end - local return_code = (' return %s'):format(results.default) - table.insert(func_code_lines, return_code) + local return_code = (' return %s'):format(results.default) + table.insert(func_code_lines, return_code) + else -- nil condition is treated as no condition + local return_line = ' return true' + table.insert(func_code_lines, return_line) + end table.insert(func_code_lines, 'end') diff --git a/test/helper.lua b/test/helper.lua index 24efb0e4..0e10eee5 100644 --- a/test/helper.lua +++ b/test/helper.lua @@ -962,6 +962,10 @@ function helpers.is_decimal_supported() return crud_utils.tarantool_supports_decimals() end +function helpers.is_uuid_supported() + return crud_utils.tarantool_supports_uuids() +end + function helpers.is_datetime_supported() return crud_utils.tarantool_supports_datetimes() end diff --git a/test/integration/count_test.lua b/test/integration/count_test.lua index dfbce6a1..6d1668de 100644 --- a/test/integration/count_test.lua +++ b/test/integration/count_test.lua @@ -893,3 +893,11 @@ for case_name_template, case in pairs(gh_373_types_cases) do case(g, read_impl) end end + +for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do + local case_name = 'test_' .. case_name_template:format('count') + + pgroup[case_name] = function(g) + case(g, read_impl) + end +end diff --git a/test/integration/pairs_readview_test.lua b/test/integration/pairs_readview_test.lua index 3dcdb4cf..4577bc98 100644 --- a/test/integration/pairs_readview_test.lua +++ b/test/integration/pairs_readview_test.lua @@ -936,3 +936,11 @@ pgroup.after_test( 'test_pairs_merger_process_storage_error', read_scenario.after_merger_process_storage_error ) + +for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do + local case_name = 'test_' .. case_name_template:format('pairs') + + pgroup[case_name] = function(g) + case(g, read_impl) + end +end diff --git a/test/integration/pairs_test.lua b/test/integration/pairs_test.lua index fd1f81e6..d5f850ba 100644 --- a/test/integration/pairs_test.lua +++ b/test/integration/pairs_test.lua @@ -944,3 +944,11 @@ pgroup.after_test( 'test_pairs_merger_process_storage_error', read_scenario.after_merger_process_storage_error ) + +for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do + local case_name = 'test_' .. case_name_template:format('pairs') + + pgroup[case_name] = function(g) + case(g, read_impl) + end +end diff --git a/test/integration/read_scenario.lua b/test/integration/read_scenario.lua index 667bfb31..75a21e0d 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -5,9 +5,11 @@ -- Scenarios here are for `srv_select` entrypoint. local t = require('luatest') +local checks = require('checks') local _, datetime = pcall(require, 'datetime') local _, decimal = pcall(require, 'decimal') +local _, uuid = pcall(require, 'uuid') local helpers = require('test.helper') @@ -656,6 +658,330 @@ local function after_merger_process_storage_error(cg) end) end + +local sample = { + number = 1, +} + +if helpers.is_decimal_supported() then + sample.decimal = decimal.new('1.234') +end + +if helpers.is_uuid_supported() then + sample.uuid = uuid.fromstr('b5ed8123-8685-479b-93c0-021cccd2608e') +end + +if helpers.is_datetime_supported() then + sample.datetime = datetime.new{year = 2024, month = 3, day = 15} +end + +if helpers.is_interval_supported() then + sample.interval = datetime.interval.new{hour = -3} +end + +local function inherit(self, object) + setmetatable(object, self) + self.__index = self + return object +end + +local SimpleStorage = {inherit = inherit} + +local function space_string_repr(field_type, field_indexed, field_nullable) + -- Example: nonindexed_nullable_number_space + + local indexing_repr = field_indexed and 'indexed' or 'nonindexed' + local nullability_repr = field_nullable and 'nullable' or 'nonnullable' + + return ('%s_%s_%s_space'):format( + indexing_repr, + nullability_repr, + field_type + ) +end + +function SimpleStorage:new(opts) + checks('table', { + field_type = 'string', + field_nullable = 'boolean', + field_indexed = 'boolean', + }) + + local object = table.deepcopy(opts) + + -- Example: simple_number_nonindexed_nullable_nil + local space_name = space_string_repr(opts.field_type, opts.field_indexed, opts.field_indexed) + + local space_cfg = {space_name, opts.field_type, opts.field_indexed, opts.field_nullable} + + local data = {{id = 1, field = sample[opts.field_type]}} + if opts.field_nullable then + -- For Tarantool, box.NULL and nil fields are indistinguishable in comparison. + table.insert(data, {id = 2, field = box.NULL}) + end + + object.space_name = space_name + object._space_cfg = space_cfg + object._data = data + object._space_on_storage_prepared = {} + + self:inherit(object) + return object +end + +function SimpleStorage:prepare_space_on_storages(cg) + if self._space_on_storage_prepared[cg] then + return + end + + local function init_space_on_storage(space_name, field_type, field_indexed, field_nullable) + if box.info.ro == true then + return + end + + local engine = os.getenv('ENGINE') or 'memtx' + + local space = box.schema.space.create(space_name, { + if_not_exists = true, + engine = engine, + }) + + space:format({ + {name = 'id', type = 'unsigned'}, + {name = 'bucket_id', type = 'unsigned'}, + {name = 'field', type = field_type, is_nullable = field_nullable}, + }) + + space:create_index('pk', { + parts = { 'id' }, + if_not_exists = true, + }) + + space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + if field_indexed then + space:create_index('field_index', { + parts = { 'field' }, + unique = false, + if_not_exists = true, + }) + end + end + + helpers.call_on_storages(cg.cluster, function(server) + server:exec(init_space_on_storage, self._space_cfg) + end) + + helpers.insert_objects(cg, self.space_name, self:get_all_records()) + + self._space_on_storage_prepared[cg] = true +end + +function SimpleStorage:get_all_records() + return self._data +end + +function SimpleStorage:get_none_records() -- luacheck: ignore + return {} +end + +function SimpleStorage:get_all_null_records() + local resp = {} + for _, v in ipairs(self._data) do + if v['field'] == nil then + table.insert(resp, v) + end + end + + return resp +end + +function SimpleStorage:get_all_nonnull_records() + local resp = {} + for _, v in ipairs(self._data) do + if v['field'] ~= nil then + table.insert(resp, v) + end + end + + return resp +end + +local gh_422_nullability_space_cases = {} + +local types_for_simple_spaces = {'number'} + +if helpers.is_decimal_supported() then + table.insert(types_for_simple_spaces, 'decimal') +end + +if helpers.is_uuid_supported() then + table.insert(types_for_simple_spaces, 'uuid') +end + +if helpers.is_datetime_supported() then + table.insert(types_for_simple_spaces, 'datetime') +end + +if helpers.is_interval_supported() then + table.insert(types_for_simple_spaces, 'interval') +end + +for _, field_type in ipairs(types_for_simple_spaces) do + local indexing_cases = {false, true} + if field_type == 'interval' then + indexing_cases = {false} + end + + for _, field_indexed in ipairs(indexing_cases) do + local nullability_cases = {false, true} + + for _, field_nullable in ipairs(nullability_cases) do + local storage = SimpleStorage:new{ + field_type = field_type, + field_indexed = field_indexed, + field_nullable = field_nullable, + } + gh_422_nullability_space_cases[storage.space_name] = storage + end + end +end + +local gh_422_nullability_condition_cases = {} + +local function get_conditions(operator, operand, opts) + checks('string', '?', {secondary_condition = 'boolean'}) + + local null_condition = {operator, 'field', operand} + + local conditions + if opts.secondary_condition then + conditions = {{'>=', 'id', 1}, null_condition} + else + conditions = {null_condition} + end + + return conditions +end + +local operator_cases = { + -- The logic in core Tarantool select is as follows. + -- nil condition is an absence of condition, thus all data is returned + -- disregarding the condition (condition may affect the order). + -- box.NULL condition is a condition for the null value -- in case of EQ ('=='), + -- only records with null index value are returned, in case of GT ('>') + -- all non-null values are returned since nulls are in the beginning of an index + -- and so on. Nils and box.NULLs in tuple are both satisfy box.NULL condition. + lt_condition_with_nil_operand = { + operator = '<', + operand = nil, + expected_objects_getter = 'get_all_records', + }, + le_condition_with_nil_operand = { + operator = '<=', + operand = nil, + expected_objects_getter = 'get_all_records', + }, + eq_condition_with_nil_operand = { + operator = '==', + operand = nil, + expected_objects_getter = 'get_all_records', + }, + ge_condition_with_nil_operand = { + operator = '>=', + operand = nil, + expected_objects_getter = 'get_all_records', + }, + gt_condition_with_nil_operand = { + operator = '>', + operand = nil, + expected_objects_getter = 'get_all_records', + }, + lt_condition_with_boxNULL_operand = { + operator = '<', + operand = box.NULL, + expected_objects_getter = 'get_none_records', + }, + le_condition_with_boxNULL_operand = { + operator = '<=', + operand = box.NULL, + expected_objects_getter = 'get_all_null_records', + }, + eq_condition_with_boxNULL_operand = { + operator = '==', + operand = box.NULL, + expected_objects_getter = 'get_all_null_records', + }, + ge_condition_with_boxNULL_operand = { + operator = '>=', + operand = box.NULL, + expected_objects_getter = 'get_all_records', + }, + gt_condition_with_boxNULL_operand = { + operator = '>', + operand = box.NULL, + expected_objects_getter = 'get_all_nonnull_records', + }, +} + +local secondarity_cases = {false, true} +for _, secondary_condition in ipairs(secondarity_cases) do + local secondarity_string_repr = secondary_condition and 'second' or 'single' + + for op_case_name, op_case in pairs(operator_cases) do + local case_name = ('%s_%s'):format(secondarity_string_repr, op_case_name) + + local conditions = get_conditions( + op_case.operator, + op_case.operand, + {secondary_condition = secondary_condition} + ) + + gh_422_nullability_condition_cases[case_name] = { + conditions = conditions, + expected_objects_getter = op_case.expected_objects_getter, + } + end +end + +local gh_422_nullability_cases = {} + +for space_case_name, space_case in pairs(gh_422_nullability_space_cases) do + for condition_case_name, condition_case in pairs(gh_422_nullability_condition_cases) do + local case_name_template = ('gh_422_%%s_%s_with_%s'):format(space_case_name, condition_case_name) + + local function case(cg, read) + -- Skip not needed since unsupported cases are not generated. + space_case:prepare_space_on_storages(cg) + + local result, err = read(cg, space_case.space_name, condition_case.conditions) + t.assert_equals(err, nil) + + -- Order may vary depending on indexes and conditions. + local getter = condition_case.expected_objects_getter + local expected_objects_without_bucket_id = space_case[getter](space_case) + + if type(result) == 'number' then -- crud.count + t.assert_equals(result, #expected_objects_without_bucket_id) + else + local actual_objects_without_bucket_id = {} + for k, v in pairs(result) do + v['bucket_id'] = nil + actual_objects_without_bucket_id[k] = v + end + + t.assert_items_equals(actual_objects_without_bucket_id, expected_objects_without_bucket_id) + end + end + + gh_422_nullability_cases[case_name_template] = case + end +end + return { gh_418_read_with_secondary_noneq_index_condition = gh_418_read_with_secondary_noneq_index_condition, gh_373_read_with_decimal_condition_cases = gh_373_read_with_decimal_condition_cases, @@ -664,4 +990,5 @@ return { before_merger_process_storage_error = before_merger_process_storage_error, merger_process_storage_error = merger_process_storage_error, after_merger_process_storage_error = after_merger_process_storage_error, + gh_422_nullability_cases = gh_422_nullability_cases, } diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua index cb3718ef..87a5b7d6 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2545,3 +2545,11 @@ pgroup.after_test( 'test_select_merger_process_storage_error', read_scenario.after_merger_process_storage_error ) + +for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do + local case_name = 'test_' .. case_name_template:format('select') + + pgroup[case_name] = function(g) + case(g, read_impl) + end +end diff --git a/test/integration/select_test.lua b/test/integration/select_test.lua index fed8064d..70b834e9 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -2313,3 +2313,11 @@ pgroup.after_test( 'test_select_merger_process_storage_error', read_scenario.after_merger_process_storage_error ) + +for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do + local case_name = 'test_' .. case_name_template:format('select') + + pgroup[case_name] = function(g) + case(g, read_impl) + end +end