From 2aa73df7813980973f08ff8f322a1a0ef3bf64b9 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Tue, 12 Mar 2024 19:17:47 +0300 Subject: [PATCH] scan: forbid interval conditions As for Tarantool 3.0 and older, datetime intervals are not comparable [1]. They are also don't supported in indexes. This patch explicitly forbids to use them in conditions. 1. https://github.com/tarantool/tarantool/issues/7659 Closes #373 --- CHANGELOG.md | 3 +++ crud/common/utils.lua | 16 ++++++++++++ crud/compare/filters.lua | 8 +++++- test/entrypoint/srv_select/storage_init.lua | 28 ++++++++++++++++++++- test/helper.lua | 6 +++++ test/integration/count_test.lua | 8 +++--- test/integration/pairs_readview_test.lua | 3 ++- test/integration/pairs_test.lua | 3 ++- test/integration/read_scenario.lua | 28 +++++++++++++++++++++ test/integration/select_readview_test.lua | 3 ++- test/integration/select_test.lua | 3 ++- 11 files changed, 98 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bad9343..20928876 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Unreleased +### Changed +* Explicitly forbid datetime interval conditions (#373). + ### Fixed * Working with datetime conditions in case of non-indexed fields or non-iterating indexes (#373). diff --git a/crud/common/utils.lua b/crud/common/utils.lua index 6af07b41..32930809 100644 --- a/crud/common/utils.lua +++ b/crud/common/utils.lua @@ -4,6 +4,7 @@ local vshard = require('vshard') local fun = require('fun') local bit = require('bit') 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') @@ -1335,6 +1336,21 @@ function utils.is_cartridge_hotreload_supported() return true, cartridge_hotreload end +local interval_supported = datetime_supported and (datetime.interval ~= nil) + +if interval_supported then + -- https://github.com/tarantool/tarantool/blob/0510ffa07afd84a70c9c6f1a4c28aacd73a393d6/src/lua/datetime.lua#L175-179 + local interval_t = ffi.typeof('struct interval') + + utils.is_interval = function(o) + return ffi.istype(interval_t, o) + end +else + utils.is_interval = function() + return false + end +end + for k, v in pairs(require('crud.common.vshard_utils')) do utils[k] = v end diff --git a/crud/compare/filters.lua b/crud/compare/filters.lua index fd9d5ef7..e3c8eb66 100644 --- a/crud/compare/filters.lua +++ b/crud/compare/filters.lua @@ -167,10 +167,16 @@ local function format_value(value) return ("%q"):format(value) elseif datetime_supported 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 + -- are not comparable. It's better to explicitly forbid them + -- for now. + -- https://github.com/tarantool/tarantool/issues/7659 + GenFiltersError:assert(false, ('datetime interval conditions are not supported')) elseif type(value) == 'cdata' then return tostring(value) end - assert(false, ('Unexpected value %s (type %s)'):format(value, type(value))) + GenFiltersError:assert(false, ('Unexpected value %s (type %s)'):format(value, type(value))) end local PARSE_ARGS_TEMPLATE = 'local tuple = ...' diff --git a/test/entrypoint/srv_select/storage_init.lua b/test/entrypoint/srv_select/storage_init.lua index 72409d16..dd088263 100644 --- a/test/entrypoint/srv_select/storage_init.lua +++ b/test/entrypoint/srv_select/storage_init.lua @@ -1,4 +1,4 @@ -local datetime_supported, _ = pcall(require, 'datetime') +local datetime_supported, datetime = pcall(require, 'datetime') local decimal_supported, _ = pcall(require, 'decimal') local crud_utils = require('crud.common.utils') @@ -421,4 +421,30 @@ return function() if_not_exists = true, }) end + + local interval_supported = datetime_supported and (datetime.interval ~= nil) + if interval_supported then + -- Interval is non-indexable. + local interval_space = box.schema.space.create('interval', { + if_not_exists = true, + engine = engine, + }) + + interval_space:format({ + {name = 'id', type = 'unsigned'}, + {name = 'bucket_id', type = 'unsigned'}, + {name = 'interval_field', type = 'interval'}, + }) + + interval_space:create_index('id_index', { + parts = { 'id' }, + if_not_exists = true, + }) + + interval_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + end end diff --git a/test/helper.lua b/test/helper.lua index 9bbf47e8..1bd0b281 100644 --- a/test/helper.lua +++ b/test/helper.lua @@ -968,6 +968,12 @@ function helpers.skip_datetime_unsupported() t.skip_if(not module_available, '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') +end + function helpers.merge_tables(t1, t2, ...) if t2 == nil then return t1 diff --git a/test/integration/count_test.lua b/test/integration/count_test.lua index 1a8ae46c..dfbce6a1 100644 --- a/test/integration/count_test.lua +++ b/test/integration/count_test.lua @@ -873,10 +873,7 @@ local read_impl = function(cg, space, conditions, opts) opts = table.deepcopy(opts) or {} opts.mode = 'write' - local resp, err = cg.cluster.main_server:call('crud.count', {space, conditions, opts}) - t.assert_equals(err, nil) - - return resp + return cg.cluster.main_server:call('crud.count', {space, conditions, opts}) end pgroup.test_gh_418_count_with_secondary_noneq_index_condition = function(g) @@ -885,7 +882,8 @@ end local gh_373_types_cases = helpers.merge_tables( read_scenario.gh_373_read_with_decimal_condition_cases, - read_scenario.gh_373_read_with_datetime_condition_cases + read_scenario.gh_373_read_with_datetime_condition_cases, + read_scenario.gh_373_read_with_interval_condition_cases ) for case_name_template, case in pairs(gh_373_types_cases) do diff --git a/test/integration/pairs_readview_test.lua b/test/integration/pairs_readview_test.lua index b19b364c..1976a1cc 100644 --- a/test/integration/pairs_readview_test.lua +++ b/test/integration/pairs_readview_test.lua @@ -912,7 +912,8 @@ end local gh_373_types_cases = helpers.merge_tables( read_scenario.gh_373_read_with_decimal_condition_cases, - read_scenario.gh_373_read_with_datetime_condition_cases + read_scenario.gh_373_read_with_datetime_condition_cases, + read_scenario.gh_373_read_with_interval_condition_cases ) for case_name_template, case in pairs(gh_373_types_cases) do diff --git a/test/integration/pairs_test.lua b/test/integration/pairs_test.lua index ddba820f..bf281e93 100644 --- a/test/integration/pairs_test.lua +++ b/test/integration/pairs_test.lua @@ -920,7 +920,8 @@ end local gh_373_types_cases = helpers.merge_tables( read_scenario.gh_373_read_with_decimal_condition_cases, - read_scenario.gh_373_read_with_datetime_condition_cases + read_scenario.gh_373_read_with_datetime_condition_cases, + read_scenario.gh_373_read_with_interval_condition_cases ) for case_name_template, case in pairs(gh_373_types_cases) do diff --git a/test/integration/read_scenario.lua b/test/integration/read_scenario.lua index 62dc8b76..0c631aae 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -555,6 +555,33 @@ for space_kind, space_option in pairs(datetime_condition_space_options) do end end +local gh_373_read_with_interval_condition_cases = { + ['gh_373_%s_with_interval_single_condition_is_forbidden'] = function(cg, read) + helpers.skip_interval_unsupported() + + local _, err = read(cg, + 'interval', + {{'>=', 'interval_field', datetime.interval.new{}}} + ) + t.assert_not_equals(err, nil) + + local err_msg = err.err or tostring(err) + t.assert_str_contains(err_msg, "datetime interval conditions are not supported") + end, + ['gh_373_%s_with_interval_second_condition_is_forbidden'] = function(cg, read) + helpers.skip_interval_unsupported() + + local _, err = read(cg, + 'interval', + {{'>=', 'id', 1}, {'>=', 'interval_field', datetime.interval.new{}}} + ) + t.assert_not_equals(err, nil) + + local err_msg = err.err or tostring(err) + t.assert_str_contains(err_msg, "datetime interval conditions are not supported") + end, +} + local function before_merger_process_storage_error(cg) helpers.call_on_storages(cg.cluster, function(server) @@ -620,6 +647,7 @@ 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, gh_373_read_with_datetime_condition_cases = gh_373_read_with_datetime_condition_cases, + gh_373_read_with_interval_condition_cases = gh_373_read_with_interval_condition_cases, 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, diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua index 72f61988..7c44a88c 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2512,7 +2512,8 @@ end local gh_373_types_cases = helpers.merge_tables( read_scenario.gh_373_read_with_decimal_condition_cases, - read_scenario.gh_373_read_with_datetime_condition_cases + read_scenario.gh_373_read_with_datetime_condition_cases, + read_scenario.gh_373_read_with_interval_condition_cases ) for case_name_template, case in pairs(gh_373_types_cases) do diff --git a/test/integration/select_test.lua b/test/integration/select_test.lua index 95ded061..a52f3969 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -2288,7 +2288,8 @@ end local gh_373_types_cases = helpers.merge_tables( read_scenario.gh_373_read_with_decimal_condition_cases, - read_scenario.gh_373_read_with_datetime_condition_cases + read_scenario.gh_373_read_with_datetime_condition_cases, + read_scenario.gh_373_read_with_interval_condition_cases ) for case_name_template, case in pairs(gh_373_types_cases) do