From 771904f1f060a3a33b334f222e0bf0251b361b1b 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 | 23 +++++++++++++--- 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 | 29 +++++++++++++++++++++ test/integration/select_readview_test.lua | 3 ++- test/integration/select_test.lua | 3 ++- 11 files changed, 103 insertions(+), 14 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..b1023dc5 100644 --- a/crud/common/utils.lua +++ b/crud/common/utils.lua @@ -1,10 +1,13 @@ +local bit = require('bit') local errors = require('errors') +local fiber = require('fiber') local ffi = require('ffi') -local vshard = require('vshard') local fun = require('fun') -local bit = require('bit') +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') @@ -23,7 +26,6 @@ local NotInitializedError = errors.new_class('NotInitialized') local StorageInfoError = errors.new_class('StorageInfoError') local VshardRouterError = errors.new_class('VshardRouterError', {capture_stack = false}) local UtilsInternalError = errors.new_class('UtilsInternalError', {capture_stack = false}) -local fiber = require('fiber') local utils = {} @@ -1335,6 +1337,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 55c60494..4e0208ee 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') @@ -422,4 +422,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 2231e553..3dcdb4cf 100644 --- a/test/integration/pairs_readview_test.lua +++ b/test/integration/pairs_readview_test.lua @@ -911,7 +911,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 a83d8bef..fd1f81e6 100644 --- a/test/integration/pairs_test.lua +++ b/test/integration/pairs_test.lua @@ -919,7 +919,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 7d1030cb..968cd3af 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -548,6 +548,34 @@ for space_kind, space_option in pairs(datetime_condition_space_options) do 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) server:exec(function() @@ -631,6 +659,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 24d0e4c0..4c5283bd 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2509,7 +2509,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 b494de5d..b3b38e69 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -2287,7 +2287,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