From 453983bf08754a4a47eee1a489e183d31a1b3d0e Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Tue, 12 Mar 2024 14:32:49 +0300 Subject: [PATCH 1/6] test: check count erroneous early exit The test was missing from PR #419, which had solved the original issue. Follows #418 --- test/integration/count_test.lua | 15 +++++++++++++++ test/integration/read_scenario.lua | 10 +++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/test/integration/count_test.lua b/test/integration/count_test.lua index a4014423..5047bd5f 100644 --- a/test/integration/count_test.lua +++ b/test/integration/count_test.lua @@ -3,6 +3,7 @@ local clock = require('clock') local t = require('luatest') local helpers = require('test.helper') +local read_scenario = require('test.integration.read_scenario') local pgroup = t.group('count', helpers.backend_matrix({ {engine = 'memtx'}, @@ -867,3 +868,17 @@ pgroup.test_count_force_map_call = function(g) t.assert_equals(err, nil) t.assert_equals(result, 2) end + +pgroup.test_gh_418_count_with_secondary_noneq_index_condition = function(g) + local read = 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 + end + + read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read) +end diff --git a/test/integration/read_scenario.lua b/test/integration/read_scenario.lua index 7adc2cfd..ff0e2462 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -1,4 +1,4 @@ --- crud.select/crud.pairs/readview:select/readview:pairs +-- crud.select/crud.pairs/crud.count/readview:select/readview:pairs -- have a lot of common scenarios, which are mostly tested with -- four nearly identical copypasted test functions now. -- This approach is expected to improve it at least for new test cases. @@ -48,13 +48,17 @@ local function gh_418_read_with_secondary_noneq_index_condition(cg, read) -- iterator had erroneously expected tuples to be sorted by `last_login` -- index while iterating on `city` index. Before the issue had beed fixed, -- user had received only one record instead of two. - local objects = read(cg, + local result = read(cg, 'logins', {{'=', 'city', 'Tatsumi Port Island'}, {'<=', 'last_login', 42}}, {bucket_id = PINNED_BUCKET_NO} ) - t.assert_equals(objects, {expected_objects[1], expected_objects[3]}) + if type(result) == 'number' then -- crud.count + t.assert_equals(result, 2) + else + t.assert_equals(result, {expected_objects[1], expected_objects[3]}) + end end return { From dd95975ea2ad9fce24ffd0cdaebd51cee0754ef1 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Mon, 11 Mar 2024 15:11:30 +0300 Subject: [PATCH 2/6] scan: support datetime filters Before this patch, datetime conditions were supported only in iterators, but not filters. (Any non-indexed condition or condition for any non-iterating index is a filter. Any condition index except for the first one is non-iterating.) This patch introduce datetime comparison support to filter codegen library, similar to uuid one. Part of #373 --- CHANGELOG.md | 6 + crud/compare/filters.lua | 50 ++++ test/entrypoint/srv_select/storage_init.lua | 98 +++++++ test/helper.lua | 5 + test/integration/count_test.lua | 24 +- test/integration/pairs_readview_test.lua | 42 +-- test/integration/pairs_test.lua | 38 ++- test/integration/read_scenario.lua | 264 ++++++++++++++++++ test/integration/select_readview_test.lua | 34 ++- test/integration/select_test.lua | 24 +- test/unit/select_filters_datetime_test.lua | 287 ++++++++++++++++++++ 11 files changed, 811 insertions(+), 61 deletions(-) create mode 100644 test/unit/select_filters_datetime_test.lua diff --git a/CHANGELOG.md b/CHANGELOG.md index 455a0727..338830d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed +* Working with datetime conditions in case of non-indexed fields or + non-iterating indexes (#373). + ## [1.4.3] - 05-02-24 ### Fixed diff --git a/crud/compare/filters.lua b/crud/compare/filters.lua index c2e39868..87ec0b9a 100644 --- a/crud/compare/filters.lua +++ b/crud/compare/filters.lua @@ -1,3 +1,5 @@ +local datetime_supported, datetime = pcall(require, 'datetime') + local errors = require('errors') local utils = require('crud.common.utils') @@ -159,6 +161,8 @@ local function format_value(value) return tostring(value) elseif utils.is_uuid(value) then return ("%q"):format(value) + elseif datetime_supported and datetime.is_datetime(value) then + return ("%q"):format(value:format()) elseif type(value) == 'cdata' then return tostring(value) end @@ -283,6 +287,8 @@ local function format_eq(cond) end elseif value_type == 'uuid' then func_name = 'eq_uuid' + elseif value_type == 'datetime' then + func_name = 'eq_datetime' end table.insert(cond_strings, format_comp_with_value(field, func_name, value)) @@ -309,6 +315,8 @@ local function format_lt(cond) func_name = add_collation_postfix('lt', value_opts) elseif value_type == 'uuid' then func_name = 'lt_uuid' + elseif value_type == 'datetime' then + func_name = 'lt_datetime' end func_name = add_strict_postfix(func_name, value_opts) @@ -533,6 +541,30 @@ local function lt_uuid_strict(lhs, rhs) return tostring(lhs) < tostring(rhs) end +local function opt_datetime_parse(v) + if type(v) == 'string' then + return datetime.parse(v) + end + + return v +end + +local function lt_datetime_nullable(lhs, rhs) + if lhs == nil and rhs ~= nil then + return true + elseif rhs == nil then + return false + end + return opt_datetime_parse(lhs) < opt_datetime_parse(rhs) +end + +local function lt_datetime_strict(lhs, rhs) + if rhs == nil then + return false + end + return opt_datetime_parse(lhs) < opt_datetime_parse(rhs) +end + local function lt_unicode_ci_nullable(lhs, rhs) if lhs == nil and rhs ~= nil then return true @@ -567,6 +599,20 @@ local function eq_uuid_strict(lhs, rhs) return tostring(lhs) == tostring(rhs) end +local function eq_datetime(lhs, rhs) + if lhs == nil then + return rhs == nil + end + return opt_datetime_parse(lhs) == opt_datetime_parse(rhs) +end + +local function eq_datetime_strict(lhs, rhs) + if rhs == nil then + return false + end + return opt_datetime_parse(lhs) == opt_datetime_parse(rhs) +end + local function eq_unicode_nullable(lhs, rhs) if lhs == nil and rhs == nil then return true @@ -604,6 +650,8 @@ local library = { eq = eq, eq_uuid = eq_uuid, eq_uuid_strict = eq_uuid_strict, + eq_datetime = eq_datetime, + eq_datetime_strict = eq_datetime_strict, -- nullable eq_unicode = eq_unicode_nullable, eq_unicode_ci = eq_unicode_ci_nullable, @@ -618,12 +666,14 @@ local library = { lt_unicode_ci = lt_unicode_ci_nullable, lt_boolean = lt_boolean_nullable, lt_uuid = lt_uuid_nullable, + lt_datetime = lt_datetime_nullable, -- strict lt_strict = lt_strict, lt_unicode_strict = lt_unicode_strict, lt_unicode_ci_strict = lt_unicode_ci_strict, lt_boolean_strict = lt_boolean_strict, lt_uuid_strict = lt_uuid_strict, + lt_datetime_strict = lt_datetime_strict, utf8 = utf8, diff --git a/test/entrypoint/srv_select/storage_init.lua b/test/entrypoint/srv_select/storage_init.lua index ce49d82e..ebdf1de9 100644 --- a/test/entrypoint/srv_select/storage_init.lua +++ b/test/entrypoint/srv_select/storage_init.lua @@ -1,3 +1,5 @@ +local datetime_supported, _ = pcall(require, 'datetime') + local crud_utils = require('crud.common.utils') return function() @@ -227,4 +229,100 @@ return function() unique = false, if_not_exists = true, }) + + if datetime_supported then + local datetime_format = { + {name = 'id', type = 'unsigned'}, + {name = 'bucket_id', type = 'unsigned'}, + {name = 'datetime_field', type = 'datetime'}, + } + + + local datetime_nonindexed_space = box.schema.space.create('datetime_nonindexed', { + if_not_exists = true, + engine = engine, + }) + + datetime_nonindexed_space:format(datetime_format) + + datetime_nonindexed_space:create_index('id_index', { + parts = { 'id' }, + if_not_exists = true, + }) + + datetime_nonindexed_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + + local datetime_indexed_space = box.schema.space.create('datetime_indexed', { + if_not_exists = true, + engine = engine, + }) + + datetime_indexed_space:format(datetime_format) + + datetime_indexed_space:create_index('id_index', { + parts = { 'id' }, + if_not_exists = true, + }) + + datetime_indexed_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + datetime_indexed_space:create_index('datetime_index', { + parts = { 'datetime_field' }, + unique = false, + if_not_exists = true, + }) + + + local datetime_pk_space = box.schema.space.create('datetime_pk', { + if_not_exists = true, + engine = engine, + }) + + datetime_pk_space:format(datetime_format) + + datetime_pk_space:create_index('datetime_index', { + parts = { 'datetime_field' }, + if_not_exists = true, + }) + + datetime_pk_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + + local datetime_multipart_index_space = box.schema.space.create('datetime_multipart_index', { + if_not_exists = true, + engine = engine, + }) + + datetime_multipart_index_space:format(datetime_format) + + datetime_multipart_index_space:create_index('id_index', { + parts = { 'id' }, + if_not_exists = true, + }) + + datetime_multipart_index_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + datetime_multipart_index_space:create_index('datetime_index', { + parts = { 'id', 'datetime_field' }, + unique = false, + if_not_exists = true, + }) + end end diff --git a/test/helper.lua b/test/helper.lua index 30c8cde3..67bd9b94 100644 --- a/test/helper.lua +++ b/test/helper.lua @@ -958,4 +958,9 @@ function helpers.prepare_ordered_data(g, space, expected_objects, bucket_id, ord t.assert_equals(objects, expected_objects) end +function helpers.skip_datetime_unsupported() + local module_available, _ = pcall(require, 'datetime') + t.skip_if(not module_available, 'datetime is not supported') +end + return helpers diff --git a/test/integration/count_test.lua b/test/integration/count_test.lua index 5047bd5f..575298ae 100644 --- a/test/integration/count_test.lua +++ b/test/integration/count_test.lua @@ -869,16 +869,24 @@ pgroup.test_count_force_map_call = function(g) t.assert_equals(result, 2) end +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 +end + pgroup.test_gh_418_count_with_secondary_noneq_index_condition = function(g) - local read = function(cg, space, conditions, opts) - opts = table.deepcopy(opts) or {} - opts.mode = 'write' + read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) +end - local resp, err = cg.cluster.main_server:call('crud.count', {space, conditions, opts}) - t.assert_equals(err, nil) +for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_cases) do + local case_name = 'test_' .. case_name_template:format('count') - return resp + pgroup[case_name] = function(g) + case(g, read_impl) end - - read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read) end diff --git a/test/integration/pairs_readview_test.lua b/test/integration/pairs_readview_test.lua index 29ab2c4a..b35581c3 100644 --- a/test/integration/pairs_readview_test.lua +++ b/test/integration/pairs_readview_test.lua @@ -882,27 +882,35 @@ pgroup.test_pairs_no_map_reduce = function(g) t.assert_equals(diff_2, 0, 'Select request was not a map reduce') end -pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g) - local read = function(cg, space, conditions, opts) - opts = table.deepcopy(opts) or {} - opts.use_tomap = true +local function read_impl(cg, space, conditions, opts) + opts = table.deepcopy(opts) or {} + opts.use_tomap = true - return cg.cluster.main_server:exec(function(space, conditions, opts) - local crud = require('crud') + return cg.cluster.main_server:exec(function(space, conditions, opts) + local crud = require('crud') - local rv, err = crud.readview() - t.assert_equals(err, nil) + local rv, err = crud.readview() + t.assert_equals(err, nil) - local status, resp = pcall(function() - return rv:pairs(space, conditions, opts):totable() - end) - t.assert(status, resp) + local status, resp = pcall(function() + return rv:pairs(space, conditions, opts):totable() + end) + t.assert(status, resp) - rv:close() + rv:close() - return resp, nil - end, {space, conditions, opts}) - end + return resp, nil + end, {space, conditions, opts}) +end - read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read) +pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g) + read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) +end + +for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_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 60d1b527..a05464f2 100644 --- a/test/integration/pairs_test.lua +++ b/test/integration/pairs_test.lua @@ -893,23 +893,31 @@ pgroup.test_pairs_no_map_reduce = function(g) t.assert_equals(diff_2, 0, 'Select request was not a map reduce') end -pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g) - local read = function(cg, space, conditions, opts) - opts = table.deepcopy(opts) or {} - opts.mode = 'write' - opts.use_tomap = true +local function read_impl(cg, space, conditions, opts) + opts = table.deepcopy(opts) or {} + opts.mode = 'write' + opts.use_tomap = true - return cg.cluster.main_server:exec(function(space, conditions, opts) - local crud = require('crud') + return cg.cluster.main_server:exec(function(space, conditions, opts) + local crud = require('crud') - local status, resp = pcall(function() - return crud.pairs(space, conditions, opts):totable() - end) - t.assert(status, resp) + local status, resp = pcall(function() + return crud.pairs(space, conditions, opts):totable() + end) + t.assert(status, resp) - return resp, nil - end, {space, conditions, opts}) - end + return resp, nil + end, {space, conditions, opts}) +end - read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read) +pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g) + read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) +end + +for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_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 ff0e2462..a08816fe 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -5,6 +5,7 @@ -- Scenarios here are for `srv_select` entrypoint. local t = require('luatest') +local datetime_supported, datetime = pcall(require, 'datetime') local helpers = require('test.helper') @@ -61,6 +62,269 @@ local function gh_418_read_with_secondary_noneq_index_condition(cg, read) end end + +local function build_condition_case( + skip_test_condition, + space_name, + space_objects, + conditions, + expected_objects_without_bucket_id +) + return function(cg, read) + skip_test_condition() + + helpers.truncate_space_on_cluster(cg.cluster, space_name) + helpers.insert_objects(cg, space_name, space_objects) + + local result, err = read(cg, space_name, conditions) + t.assert_equals(err, nil) + + 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 +end + +local datetime_vals = {} + +if datetime_supported then + datetime_vals = { + yesterday = datetime.new{ + year = 2024, + month = 3, + day = 10, + }, + today = datetime.new{ + year = 2024, + month = 3, + day = 11, + }, + tomorrow = datetime.new{ + year = 2024, + month = 3, + day = 12, + }, + } + + assert(datetime_vals.yesterday < datetime_vals.today) + assert(datetime_vals.today < datetime_vals.tomorrow) +end + +local datetime_data = { + { + id = 1, + datetime_field = datetime_vals.yesterday, + }, + { + id = 2, + datetime_field = datetime_vals.today, + }, + { + id = 3, + datetime_field = datetime_vals.tomorrow, + }, +} + +local function today_condition(operator, operand, is_multipart) + if is_multipart then + return {operator, operand, {2, datetime_vals.today}} + else + return {operator, operand, datetime_vals.today} + end +end + +local datetime_condition_operator_options = { + single_lt = function(operand, is_multipart) + return { + conditions = {today_condition('<', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + datetime_field = datetime_vals.yesterday, + }, + }, + } + end, + single_le = function(operand, is_multipart) + return { + conditions = {today_condition('<=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + datetime_field = datetime_vals.yesterday, + }, + { + id = 2, + datetime_field = datetime_vals.today, + }, + }, + } + end, + single_eq = function(operand, is_multipart) + return { + conditions = {today_condition('==', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + datetime_field = datetime_vals.today, + }, + }, + } + end, + single_ge = function(operand, is_multipart) + return { + conditions = {today_condition('>=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + datetime_field = datetime_vals.today, + }, + { + id = 3, + datetime_field = datetime_vals.tomorrow, + }, + }, + } + end, + single_gt = function(operand, is_multipart) + return { + conditions = {today_condition('>', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 3, + datetime_field = datetime_vals.tomorrow, + }, + }, + } + end, + second_lt = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, today_condition('<', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + datetime_field = datetime_vals.yesterday, + }, + }, + } + end, + second_le = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, today_condition('<=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + datetime_field = datetime_vals.yesterday, + }, + { + id = 2, + datetime_field = datetime_vals.today, + }, + }, + } + end, + second_eq = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, today_condition('==', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + datetime_field = datetime_vals.today, + }, + }, + } + end, + second_ge = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, today_condition('>=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + datetime_field = datetime_vals.today, + }, + { + id = 3, + datetime_field = datetime_vals.tomorrow, + }, + }, + } + end, + second_gt = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, today_condition('>', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 3, + datetime_field = datetime_vals.tomorrow, + }, + }, + } + end, +} + +local datetime_condition_space_options = { + nonindexed = { + space_name = 'datetime_nonindexed', + index_kind = nil, + }, + indexed = { + space_name = 'datetime_indexed', + index_kind = 'secondary', + }, + pk = { + space_name = 'datetime_pk', + index_kind = 'primary', + }, + multipart_indexed = { + space_name = 'datetime_multipart_index', + index_kind = 'multipart', + is_multipart = true, + }, +} + +local gh_373_read_with_datetime_condition_cases = {} + +for space_kind, space_option in pairs(datetime_condition_space_options) do + for operator_kind, operator_options_builder in pairs(datetime_condition_operator_options) do + local field_case_name_template = ('gh_373_%%s_with_datetime_%s_field_%s_condition'):format( + space_kind, operator_kind) + + local field_operator_options = operator_options_builder('datetime_field', false) + + gh_373_read_with_datetime_condition_cases[field_case_name_template] = build_condition_case( + helpers.skip_datetime_unsupported, + space_option.space_name, + datetime_data, + field_operator_options.conditions, + field_operator_options.expected_objects_without_bucket_id + ) + + if space_option.index_kind ~= nil then + local index_case_name_template = ('gh_373_%%s_with_datetime_%s_index_%s_condition'):format( + space_option.index_kind, operator_kind) + + local index_operator_options = operator_options_builder('datetime_index', space_option.is_multipart) + + gh_373_read_with_datetime_condition_cases[index_case_name_template] = build_condition_case( + helpers.skip_datetime_unsupported, + space_option.space_name, + datetime_data, + index_operator_options.conditions, + index_operator_options.expected_objects_without_bucket_id + ) + end + end +end + return { gh_418_read_with_secondary_noneq_index_condition = gh_418_read_with_secondary_noneq_index_condition, + gh_373_read_with_datetime_condition_cases = gh_373_read_with_datetime_condition_cases, } diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua index 796fcffe..c7a5cc25 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2486,21 +2486,29 @@ pgroup.test_select_closed_readview = function(g) t.assert_str_contains(err.str, 'Read view is closed') end -pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g) - local read = function(cg, space, conditions, opts) - return cg.cluster.main_server:exec(function(space, conditions, opts) - local crud = require('crud') - local rv, err = crud.readview() - t.assert_equals(err, nil) +local function read_impl(cg, space, conditions, opts) + return cg.cluster.main_server:exec(function(space, conditions, opts) + local crud = require('crud') + local rv, err = crud.readview() + t.assert_equals(err, nil) - local resp, err = rv:select(space, conditions, opts) - t.assert_equals(err, nil) + local resp, err = rv:select(space, conditions, opts) + t.assert_equals(err, nil) - rv:close() + rv:close() - return crud.unflatten_rows(resp.rows, resp.metadata) - end, {space, conditions, opts}) - end + return crud.unflatten_rows(resp.rows, resp.metadata) + end, {space, conditions, opts}) +end - read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read) +pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g) + read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) +end + +for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_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 dff15ee8..166698a9 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -2268,16 +2268,24 @@ pgroup.test_pairs_yield_every_0 = function(g) end) end +local function read_impl(cg, space, conditions, opts) + opts = table.deepcopy(opts) or {} + opts.mode = 'write' + + local resp, err = cg.cluster.main_server:call('crud.select', {space, conditions, opts}) + t.assert_equals(err, nil) + + return crud.unflatten_rows(resp.rows, resp.metadata) +end + pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g) - local read = function(cg, space, conditions, opts) - opts = table.deepcopy(opts) or {} - opts.mode = 'write' + read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) +end - local resp, err = cg.cluster.main_server:call('crud.select', {space, conditions, opts}) - t.assert_equals(err, nil) +for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_cases) do + local case_name = 'test_' .. case_name_template:format('select') - return crud.unflatten_rows(resp.rows, resp.metadata) + pgroup[case_name] = function(g) + case(g, read_impl) end - - read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read) end diff --git a/test/unit/select_filters_datetime_test.lua b/test/unit/select_filters_datetime_test.lua new file mode 100644 index 00000000..2159b6b0 --- /dev/null +++ b/test/unit/select_filters_datetime_test.lua @@ -0,0 +1,287 @@ +local _, datetime = pcall(require, 'datetime') + +local compare_conditions = require('crud.compare.conditions') +local cond_funcs = compare_conditions.funcs +local select_filters = require('crud.compare.filters') +local collations = require('crud.common.collations') +local select_plan = require('crud.compare.plan') + +local t = require('luatest') +local g = t.group('select_filters_datetime') + +local helpers = require('test.helper') + +g.before_all = function() + helpers.skip_datetime_unsupported() + + helpers.box_cfg() + + local customers_space = box.schema.space.create('customers', { + format = { + {'datetime', 'datetime'}, + {'bucket_id', 'unsigned'}, + {'name', 'string'}, + {'second_datetime', 'datetime'}, + }, + if_not_exists = true, + }) + customers_space:create_index('datetime', { -- id: 0 + parts = {'datetime'}, + if_not_exists = true, + }) + customers_space:create_index('second_datetime', { -- id: 1 + parts = { + { field = 'second_datetime', is_nullable = true }, + }, + if_not_exists = true, + }) +end + +g.after_all(function() + box.space.customers:drop() +end) + +g.test_parse = function() + -- select by indexed field with conditions by index and field + local dt1 = datetime.new{year = 2000, month = 1, day = 1, tz = 'Europe/Moscow'} + local dt2 = datetime.new{year = 2012, month = 1, day = 1, tzoffset = -180} + local dt3 = datetime.new{year = 1980, month = 1, day = 1} + + local conditions = { + cond_funcs.gt('datetime', dt1), + cond_funcs.lt('datetime', dt2), + cond_funcs.eq('name', 'Charlie'), + cond_funcs.eq('second_datetime', dt3) + } + + local plan, err = select_plan.new(box.space.customers, conditions) + t.assert_equals(err, nil) + + local space = box.space.customers + local scan_index = space.index[plan.index_id] + + local filter_conditions, err = select_filters.internal.parse(space, scan_index, conditions, { + scan_condition_num = plan.scan_condition_num, + tarantool_iter = plan.tarantool_iter, + }) + t.assert_equals(err, nil) + + -- datetime filter (early exit is possible) + local datetime_filter_condition = filter_conditions[1] + t.assert_type(datetime_filter_condition, 'table') + t.assert_equals(datetime_filter_condition.fields, {1}) + t.assert_equals(datetime_filter_condition.operator, compare_conditions.operators.LT) + t.assert_equals(datetime_filter_condition.values, {dt2}) + t.assert_equals(datetime_filter_condition.types, {'datetime'}) + t.assert_equals(datetime_filter_condition.early_exit_is_possible, true) + + -- name filter + local name_filter_condition = filter_conditions[2] + t.assert_type(name_filter_condition, 'table') + t.assert_equals(name_filter_condition.fields, {3}) + t.assert_equals(name_filter_condition.operator, compare_conditions.operators.EQ) + t.assert_equals(name_filter_condition.values, {'Charlie'}) + t.assert_equals(name_filter_condition.types, {'string'}) + t.assert_equals(name_filter_condition.early_exit_is_possible, false) + + -- second_datetime filter + local second_datetime_filter_condition = filter_conditions[3] + t.assert_type(second_datetime_filter_condition, 'table') + t.assert_equals(second_datetime_filter_condition.fields, {4}) + t.assert_equals(second_datetime_filter_condition.operator, compare_conditions.operators.EQ) + t.assert_equals(second_datetime_filter_condition.values, {dt3}) + t.assert_equals(second_datetime_filter_condition.types, {'datetime'}) + t.assert_equals(second_datetime_filter_condition.early_exit_is_possible, false) + + t.assert_equals(#second_datetime_filter_condition.values_opts, 1) + local second_datetime_opts = second_datetime_filter_condition.values_opts[1] + t.assert_equals(second_datetime_opts.is_nullable, true) + t.assert_equals(second_datetime_opts.collation, collations.NONE) +end + +g.test_one_condition_datetime = function() + local dt1 = datetime.new{year = 2000, month = 1, day = 1, tz = 'Europe/Moscow'} + local dt2 = datetime.new{year = 2012, month = 1, day = 1, tzoffset = -180} + + local filter_conditions = { + { + fields = {1}, + operator = compare_conditions.operators.EQ, + values = {dt1}, + types = {'datetime'}, + early_exit_is_possible = true, + } + } + + local expected_code = [[local tuple = ... + +local field_1 = tuple[1] + +if not eq_1(field_1) then return false, true end + +return true, false]] + + local expected_library_code = [[local M = {} + +function M.eq_1(field_1) + return (eq_datetime(field_1, "2000-01-01T00:00:00 Europe/Moscow")) +end + +return M]] + + local filter_code = select_filters.internal.gen_filter_code(filter_conditions) + t.assert_equals(filter_code.code, expected_code) + t.assert_equals(filter_code.library, expected_library_code) + + local filter_func = select_filters.internal.compile(filter_code) + t.assert_equals({ filter_func({dt1, dt1:format(), 1}) }, {true, false}) + t.assert_equals({ filter_func({dt2, dt1:format(), 1}) }, {false, true}) + t.assert_equals({ filter_func({nil, dt1:format(), 1}) }, {false, true}) +end + +g.test_one_condition_datetime_gt = function() + local dt1 = datetime.new{year = 2000, month = 1, day = 1, tz = 'Europe/Moscow'} + local dt2 = datetime.new{year = 2012, month = 1, day = 1, tzoffset = -180} + + local filter_conditions = { + { + fields = {1}, + operator = compare_conditions.operators.GT, + values = {dt1}, + types = {'datetime'}, + early_exit_is_possible = true, + } + } + + local expected_code = [[local tuple = ... + +local field_1 = tuple[1] + +if not cmp_1(field_1) then return false, true end + +return true, false]] + + local expected_library_code = [[local M = {} + +function M.cmp_1(field_1) + if lt_datetime(field_1, "2000-01-01T00:00:00 Europe/Moscow") then return false end + if not eq_datetime(field_1, "2000-01-01T00:00:00 Europe/Moscow") then return true end + + return false +end + +return M]] + + local filter_code = select_filters.internal.gen_filter_code(filter_conditions) + t.assert_equals(filter_code.code, expected_code) + t.assert_equals(filter_code.library, expected_library_code) + + local filter_func = select_filters.internal.compile(filter_code) + t.assert_equals({ filter_func({dt2, dt1:format(), 1}) }, {true, false}) + t.assert_equals({ filter_func({dt1, dt2:format(), 1}) }, {false, true}) + t.assert_equals({ filter_func({nil, dt1:format(), 1}) }, {false, true}) +end + +g.test_one_condition_datetime_with_nil_value = function() + local dt1 = datetime.new{year = 2000, month = 1, day = 1, tz = 'Europe/Moscow'} + local dt2 = datetime.new{year = 2012, month = 1, day = 1, tzoffset = -180} + + local filter_conditions = { + { + fields = {1, 3}, + operator = compare_conditions.operators.GE, + values = {dt1}, + types = {'datetime', 'string'}, + early_exit_is_possible = false, + values_opts = { + {is_nullable = false}, + {is_nullable = true}, + }, + }, + } + + local expected_code = [[local tuple = ... + +local field_1 = tuple[1] + +if not cmp_1(field_1) then return false, false end + +return true, false]] + + local expected_library_code = [[local M = {} + +function M.cmp_1(field_1) + if lt_datetime_strict(field_1, "2000-01-01T00:00:00 Europe/Moscow") then return false end + if not eq_datetime(field_1, "2000-01-01T00:00:00 Europe/Moscow") then return true end + + return true +end + +return M]] + + local filter_code = select_filters.internal.gen_filter_code(filter_conditions) + t.assert_equals(filter_code.code, expected_code) + t.assert_equals(filter_code.library, expected_library_code) + + local filter_func = select_filters.internal.compile(filter_code) + t.assert_equals(filter_func({dt1, 'test', 3}), true) + t.assert_equals(filter_func({dt2, 'xxx', 1}), true) +end + +g.test_two_conditions_datetime = function() + local dt1 = datetime.new{year = 2000, month = 1, day = 1, tz = 'Europe/Moscow'} + local dt2 = datetime.new{year = 2012, month = 1, day = 1, tzoffset = -180} + + local filter_conditions = { + { + fields = {2}, + operator = compare_conditions.operators.EQ, + values = {'Charlie'}, + types = {'string'}, + early_exit_is_possible = true, + }, + { + fields = {3}, + operator = compare_conditions.operators.GE, + values = {dt2:format()}, + types = {'datetime'}, + early_exit_is_possible = false, + } + } + + local expected_code = [[local tuple = ... + +local field_2 = tuple[2] +local field_3 = tuple[3] + +if not eq_1(field_2) then return false, true end +if not cmp_2(field_3) then return false, false end + +return true, false]] + + local expected_library_code = [[local M = {} + +function M.eq_1(field_2) + return (eq(field_2, "Charlie")) +end + +function M.cmp_2(field_3) + if lt_datetime(field_3, "2012-01-01T00:00:00-0300") then return false end + if not eq_datetime(field_3, "2012-01-01T00:00:00-0300") then return true end + + return true +end + +return M]] + + local filter_code = select_filters.internal.gen_filter_code(filter_conditions) + t.assert_equals(filter_code.code, expected_code) + t.assert_equals(filter_code.library, expected_library_code) + + local filter_func = select_filters.internal.compile(filter_code) + t.assert_equals({ filter_func({4, 'xxx', dt1}) }, {false, true}) + t.assert_equals({ filter_func({5, 'Charlie', dt1}) }, {false, false}) + t.assert_equals({ filter_func({5, 'xxx', dt2}) }, {false, true}) + t.assert_equals({ filter_func({6, 'Charlie', dt2}) }, {true, false}) + t.assert_equals({ filter_func({6, 'Charlie', nil}) }, {false, false}) +end From 9c7ceb28ac70bf5dc346576c6051be5b3396590b Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Tue, 12 Mar 2024 11:26:28 +0300 Subject: [PATCH 3/6] scan: fix decimal filters precision loss Before this patch, decimals were cast to numbers in filters through `tostring` in codegen. This patch fixes this precision loss. Part of #373 --- CHANGELOG.md | 2 + crud/compare/filters.lua | 4 + test/entrypoint/srv_select/storage_init.lua | 97 +++++++++ test/helper.lua | 23 ++ test/integration/count_test.lua | 7 +- test/integration/pairs_readview_test.lua | 7 +- test/integration/pairs_test.lua | 7 +- test/integration/read_scenario.lua | 223 ++++++++++++++++++++ test/integration/select_readview_test.lua | 7 +- test/integration/select_test.lua | 7 +- 10 files changed, 379 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 338830d1..bc81dec2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed * Working with datetime conditions in case of non-indexed fields or non-iterating indexes (#373). +* Precision loss for decimal conditions in case of non-indexed fields or + non-iterating indexes (#373). ## [1.4.3] - 05-02-24 diff --git a/crud/compare/filters.lua b/crud/compare/filters.lua index 87ec0b9a..fd9d5ef7 100644 --- a/crud/compare/filters.lua +++ b/crud/compare/filters.lua @@ -1,4 +1,5 @@ local datetime_supported, datetime = pcall(require, 'datetime') +local decimal_supported, decimal = pcall(require, 'decimal') local errors = require('errors') @@ -159,6 +160,9 @@ 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 + -- 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 diff --git a/test/entrypoint/srv_select/storage_init.lua b/test/entrypoint/srv_select/storage_init.lua index ebdf1de9..55c60494 100644 --- a/test/entrypoint/srv_select/storage_init.lua +++ b/test/entrypoint/srv_select/storage_init.lua @@ -1,4 +1,5 @@ local datetime_supported, _ = pcall(require, 'datetime') +local decimal_supported, _ = pcall(require, 'decimal') local crud_utils = require('crud.common.utils') @@ -230,6 +231,102 @@ return function() if_not_exists = true, }) + if decimal_supported then + local decimal_format = { + {name = 'id', type = 'unsigned'}, + {name = 'bucket_id', type = 'unsigned'}, + {name = 'decimal_field', type = 'decimal'}, + } + + + local decimal_nonindexed_space = box.schema.space.create('decimal_nonindexed', { + if_not_exists = true, + engine = engine, + }) + + decimal_nonindexed_space:format(decimal_format) + + decimal_nonindexed_space:create_index('id_index', { + parts = { 'id' }, + if_not_exists = true, + }) + + decimal_nonindexed_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + + local decimal_indexed_space = box.schema.space.create('decimal_indexed', { + if_not_exists = true, + engine = engine, + }) + + decimal_indexed_space:format(decimal_format) + + decimal_indexed_space:create_index('id_index', { + parts = { 'id' }, + if_not_exists = true, + }) + + decimal_indexed_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + decimal_indexed_space:create_index('decimal_index', { + parts = { 'decimal_field' }, + unique = false, + if_not_exists = true, + }) + + + local decimal_pk_space = box.schema.space.create('decimal_pk', { + if_not_exists = true, + engine = engine, + }) + + decimal_pk_space:format(decimal_format) + + decimal_pk_space:create_index('decimal_index', { + parts = { 'decimal_field' }, + if_not_exists = true, + }) + + decimal_pk_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + + local decimal_multipart_index_space = box.schema.space.create('decimal_multipart_index', { + if_not_exists = true, + engine = engine, + }) + + decimal_multipart_index_space:format(decimal_format) + + decimal_multipart_index_space:create_index('id_index', { + parts = { 'id' }, + if_not_exists = true, + }) + + decimal_multipart_index_space:create_index('bucket_id', { + parts = { 'bucket_id' }, + unique = false, + if_not_exists = true, + }) + + decimal_multipart_index_space:create_index('decimal_index', { + parts = { 'id', 'decimal_field' }, + unique = false, + if_not_exists = true, + }) + end + if datetime_supported then local datetime_format = { {name = 'id', type = 'unsigned'}, diff --git a/test/helper.lua b/test/helper.lua index 67bd9b94..9bbf47e8 100644 --- a/test/helper.lua +++ b/test/helper.lua @@ -958,9 +958,32 @@ function helpers.prepare_ordered_data(g, space, expected_objects, bucket_id, ord t.assert_equals(objects, expected_objects) end +function helpers.skip_decimal_unsupported() + local module_available, _ = pcall(require, 'decimal') + t.skip_if(not module_available, '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') end +function helpers.merge_tables(t1, t2, ...) + if t2 == nil then + return t1 + end + + local res = {} + + for k, v in pairs(t1) do + res[k] = v + end + + for k, v in pairs(t2) do + res[k] = v + end + + return helpers.merge_tables(res, ...) +end + return helpers diff --git a/test/integration/count_test.lua b/test/integration/count_test.lua index 575298ae..1a8ae46c 100644 --- a/test/integration/count_test.lua +++ b/test/integration/count_test.lua @@ -883,7 +883,12 @@ pgroup.test_gh_418_count_with_secondary_noneq_index_condition = function(g) read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) end -for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_cases) do +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 +) + +for case_name_template, case in pairs(gh_373_types_cases) do local case_name = 'test_' .. case_name_template:format('count') pgroup[case_name] = function(g) diff --git a/test/integration/pairs_readview_test.lua b/test/integration/pairs_readview_test.lua index b35581c3..cc1fe856 100644 --- a/test/integration/pairs_readview_test.lua +++ b/test/integration/pairs_readview_test.lua @@ -907,7 +907,12 @@ pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g) read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) end -for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_cases) do +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 +) + +for case_name_template, case in pairs(gh_373_types_cases) do local case_name = 'test_' .. case_name_template:format('pairs') pgroup[case_name] = function(g) diff --git a/test/integration/pairs_test.lua b/test/integration/pairs_test.lua index a05464f2..efdbc719 100644 --- a/test/integration/pairs_test.lua +++ b/test/integration/pairs_test.lua @@ -914,7 +914,12 @@ pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g) read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) end -for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_cases) do +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 +) + +for case_name_template, case in pairs(gh_373_types_cases) do local case_name = 'test_' .. case_name_template:format('pairs') pgroup[case_name] = function(g) diff --git a/test/integration/read_scenario.lua b/test/integration/read_scenario.lua index a08816fe..216e8fb9 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -6,6 +6,7 @@ local t = require('luatest') local datetime_supported, datetime = pcall(require, 'datetime') +local decimal_supported, decimal = pcall(require, 'decimal') local helpers = require('test.helper') @@ -93,6 +94,227 @@ local function build_condition_case( end end + +local decimal_vals = {} + +if decimal_supported then + decimal_vals = { + smallest_negative = decimal.new('-123456789012345678.987431234678392'), + bigger_negative = decimal.new('-123456789012345678.987431234678391'), + bigger_positive = decimal.new('123456789012345678.987431234678391'), + } + + assert(decimal_vals.smallest_negative < decimal_vals.bigger_negative) + assert(decimal_vals.bigger_negative < decimal_vals.bigger_positive) +end + +local decimal_data = { + { + id = 1, + decimal_field = decimal_vals.smallest_negative, + }, + { + id = 2, + decimal_field = decimal_vals.bigger_negative, + }, + { + id = 3, + decimal_field = decimal_vals.bigger_positive, + }, +} + +local function bigger_negative_condition(operator, operand, is_multipart) + if is_multipart then + return {operator, operand, {2, decimal_vals.bigger_negative}} + else + return {operator, operand, decimal_vals.bigger_negative} + end +end + +local decimal_condition_operator_options = { + single_lt = function(operand, is_multipart) + return { + conditions = {bigger_negative_condition('<', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + decimal_field = decimal_vals.smallest_negative, + }, + }, + } + end, + single_le = function(operand, is_multipart) + return { + conditions = {bigger_negative_condition('<=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + decimal_field = decimal_vals.smallest_negative, + }, + { + id = 2, + decimal_field = decimal_vals.bigger_negative, + }, + }, + } + end, + single_eq = function(operand, is_multipart) + return { + conditions = {bigger_negative_condition('==', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + decimal_field = decimal_vals.bigger_negative, + }, + }, + } + end, + single_ge = function(operand, is_multipart) + return { + conditions = {bigger_negative_condition('>=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + decimal_field = decimal_vals.bigger_negative, + }, + { + id = 3, + decimal_field = decimal_vals.bigger_positive, + }, + }, + } + end, + single_gt = function(operand, is_multipart) + return { + conditions = {bigger_negative_condition('>', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 3, + decimal_field = decimal_vals.bigger_positive, + }, + }, + } + end, + second_lt = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, bigger_negative_condition('<', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + decimal_field = decimal_vals.smallest_negative, + }, + }, + } + end, + second_le = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, bigger_negative_condition('<=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 1, + decimal_field = decimal_vals.smallest_negative, + }, + { + id = 2, + decimal_field = decimal_vals.bigger_negative, + }, + }, + } + end, + second_eq = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, bigger_negative_condition('==', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + decimal_field = decimal_vals.bigger_negative, + }, + }, + } + end, + second_ge = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, bigger_negative_condition('>=', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 2, + decimal_field = decimal_vals.bigger_negative, + }, + { + id = 3, + decimal_field = decimal_vals.bigger_positive, + }, + }, + } + end, + second_gt = function(operand, is_multipart) + return { + conditions = {{'>=', 'id', 1}, bigger_negative_condition('>', operand, is_multipart)}, + expected_objects_without_bucket_id = { + { + id = 3, + decimal_field = decimal_vals.bigger_positive, + }, + }, + } + end, +} + +local decimal_condition_space_options = { + nonindexed = { + space_name = 'decimal_nonindexed', + index_kind = nil, + }, + indexed = { + space_name = 'decimal_indexed', + index_kind = 'secondary', + }, + pk = { + space_name = 'decimal_pk', + index_kind = 'primary', + }, + multipart_indexed = { + space_name = 'decimal_multipart_index', + index_kind = 'multipart', + is_multipart = true, + }, +} + +local gh_373_read_with_decimal_condition_cases = {} + +for space_kind, space_option in pairs(decimal_condition_space_options) do + for operator_kind, operator_options_builder in pairs(decimal_condition_operator_options) do + local field_case_name_template = ('gh_373_%%s_with_decimal_%s_field_%s_condition'):format( + space_kind, operator_kind) + + local field_operator_options = operator_options_builder('decimal_field', false) + + gh_373_read_with_decimal_condition_cases[field_case_name_template] = build_condition_case( + helpers.skip_decimal_unsupported, + space_option.space_name, + decimal_data, + field_operator_options.conditions, + field_operator_options.expected_objects_without_bucket_id + ) + + if space_option.index_kind ~= nil then + local index_case_name_template = ('gh_373_%%s_with_decimal_%s_index_%s_condition'):format( + space_option.index_kind, operator_kind) + + local index_operator_options = operator_options_builder('decimal_index', space_option.is_multipart) + + gh_373_read_with_decimal_condition_cases[index_case_name_template] = build_condition_case( + helpers.skip_decimal_unsupported, + space_option.space_name, + decimal_data, + index_operator_options.conditions, + index_operator_options.expected_objects_without_bucket_id + ) + end + end +end + + local datetime_vals = {} if datetime_supported then @@ -326,5 +548,6 @@ 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, gh_373_read_with_datetime_condition_cases = gh_373_read_with_datetime_condition_cases, } diff --git a/test/integration/select_readview_test.lua b/test/integration/select_readview_test.lua index c7a5cc25..ef1fbe9b 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2505,7 +2505,12 @@ pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g) read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) end -for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_cases) do +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 +) + +for case_name_template, case in pairs(gh_373_types_cases) do local case_name = 'test_' .. case_name_template:format('select') pgroup[case_name] = function(g) diff --git a/test/integration/select_test.lua b/test/integration/select_test.lua index 166698a9..de0be3a0 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -2282,7 +2282,12 @@ pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g) read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read_impl) end -for case_name_template, case in pairs(read_scenario.gh_373_read_with_datetime_condition_cases) do +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 +) + +for case_name_template, case in pairs(gh_373_types_cases) do local case_name = 'test_' .. case_name_template:format('select') pgroup[case_name] = function(g) From c64642051d91767ef3d4db52ec0baa28ea01faa5 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Tue, 12 Mar 2024 17:47:17 +0300 Subject: [PATCH 4/6] ci: drop some EOLed 2.x Tarantool Versions 2.2--2.7 have reached their end of life in 2020-2021 [1]. 1. https://www.tarantool.io/ru/doc/latest/release/eol_versions/ --- .github/workflows/test_on_push.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_on_push.yaml b/.github/workflows/test_on_push.yaml index 02ea45cb..21ec9e1b 100644 --- a/.github/workflows/test_on_push.yaml +++ b/.github/workflows/test_on_push.yaml @@ -17,7 +17,7 @@ jobs: # it uses its own metrics package. # We test old metrics with Cartridge 2.7.9 because since 2.8.0 it # requires metrics 1.0.0. - tarantool-version: ["1.10.6", "1.10", "2.2", "2.3", "2.4", "2.5", "2.6", "2.7", "2.8", "2.10", "2.11"] + tarantool-version: ["1.10.6", "1.10", "2.8", "2.10", "2.11"] metrics-version: [""] cartridge-version: ["2.8.0"] external-tuple-merger-version: [""] From 7449adeee0b165e86d4f3d0a18aca1d33d009128 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Tue, 12 Mar 2024 18:36:49 +0300 Subject: [PATCH 5/6] crud: fix passing errors from storages for merger Before this patch, returning any errors from storages for merger operations (`crud.select`, `crud.pairs`, `readview:select`, `readview:pairs`) had resulted in non-informative "Invalid merge source 0x556bb6885a00" errors. This patch fixes the issue by replacing error return with error throw. Part of #373 --- CHANGELOG.md | 2 + crud/readview.lua | 10 +-- crud/select.lua | 8 +-- test/integration/pairs_readview_test.lua | 22 +++++- test/integration/pairs_test.lua | 21 +++++- test/integration/read_scenario.lua | 86 ++++++++++++++++++++++- test/integration/select_readview_test.lua | 22 +++++- test/integration/select_test.lua | 21 +++++- 8 files changed, 172 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc81dec2..5bad9343 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. non-iterating indexes (#373). * Precision loss for decimal conditions in case of non-indexed fields or non-iterating indexes (#373). +* Passing errors from storages for merger operations (`crud.select`, + `crud.pairs`, `readview:select`, `readview:pairs`) (#423). ## [1.4.3] - 05-02-24 diff --git a/crud/readview.lua b/crud/readview.lua index fcd0a832..c129d5b9 100644 --- a/crud/readview.lua +++ b/crud/readview.lua @@ -133,22 +133,22 @@ local function select_readview_on_storage(space_name, index_id, conditions, opts end if space_readview == nil then - return cursor, ReadviewError:new("Space %q doesn't exist", space_name) + return ReadviewError:assert(false, "Space %q doesn't exist", space_name) end local space = box.space[space_name] if space == nil then - return cursor, ReadviewError:new("Space %q doesn't exist", space_name) + return ReadviewError:assert(false, "Space %q doesn't exist", space_name) end space_readview.format = space:format() local index_readview = space_readview.index[index_id] if index_readview == nil then - return cursor, ReadviewError:new("Index with ID %s doesn't exist", index_id) + return ReadviewError:assert(false, "Index with ID %s doesn't exist", index_id) end local index = space.index[index_id] if index == nil then - return cursor, ReadviewError:new("Index with ID %s doesn't exist", index_id) + return ReadviewError:assert(false, "Index with ID %s doesn't exist", index_id) end local _, err = sharding.check_sharding_hash(space_name, @@ -179,7 +179,7 @@ local function select_readview_on_storage(space_name, index_id, conditions, opts readview_index = index_readview, }) if err ~= nil then - return cursor, ReadviewError:new("Failed to execute select: %s", err) + return ReadviewError:assert(false, "Failed to execute select: %s", err) end if resp.tuples_fetched < opts.limit or opts.limit == 0 then diff --git a/crud/select.lua b/crud/select.lua index dc53062a..8c609dac 100644 --- a/crud/select.lua +++ b/crud/select.lua @@ -61,12 +61,12 @@ local function select_on_storage(space_name, index_id, conditions, opts) local space = box.space[space_name] if space == nil then - return cursor, SelectError:new("Space %q doesn't exist", space_name) + SelectError:assert(false, "Space %q doesn't exist", space_name) end local index = space.index[index_id] if index == nil then - return cursor, SelectError:new("Index with ID %s doesn't exist", index_id) + SelectError:assert(false, "Index with ID %s doesn't exist", index_id) end local _, err = sharding.check_sharding_hash(space_name, @@ -83,7 +83,7 @@ local function select_on_storage(space_name, index_id, conditions, opts) scan_condition_num = opts.scan_condition_num, }) if err ~= nil then - return cursor, SelectError:new("Failed to generate tuples filter: %s", err) + return SelectError:assert(false, "Failed to generate tuples filter: %s", err) end -- execute select @@ -95,7 +95,7 @@ local function select_on_storage(space_name, index_id, conditions, opts) yield_every = opts.yield_every, }) if err ~= nil then - return cursor, SelectError:new("Failed to execute select: %s", err) + return SelectError:assert(false, "Failed to execute select: %s", err) end if resp.tuples_fetched < opts.limit or opts.limit == 0 then diff --git a/test/integration/pairs_readview_test.lua b/test/integration/pairs_readview_test.lua index cc1fe856..2231e553 100644 --- a/test/integration/pairs_readview_test.lua +++ b/test/integration/pairs_readview_test.lua @@ -895,11 +895,13 @@ local function read_impl(cg, space, conditions, opts) local status, resp = pcall(function() return rv:pairs(space, conditions, opts):totable() end) - t.assert(status, resp) - rv:close() - return resp, nil + if status then + return resp, nil + else + return nil, resp + end end, {space, conditions, opts}) end @@ -919,3 +921,17 @@ for case_name_template, case in pairs(gh_373_types_cases) do case(g, read_impl) end end + +pgroup.before_test( + 'test_pairs_merger_process_storage_error', + read_scenario.before_merger_process_storage_error +) + +pgroup.test_pairs_merger_process_storage_error = function(g) + read_scenario.merger_process_storage_error(g, read_impl) +end + +pgroup.after_test( + 'test_pairs_merger_process_storage_error', + read_scenario.after_merger_process_storage_error +) diff --git a/test/integration/pairs_test.lua b/test/integration/pairs_test.lua index efdbc719..a83d8bef 100644 --- a/test/integration/pairs_test.lua +++ b/test/integration/pairs_test.lua @@ -904,9 +904,12 @@ local function read_impl(cg, space, conditions, opts) local status, resp = pcall(function() return crud.pairs(space, conditions, opts):totable() end) - t.assert(status, resp) - return resp, nil + if status then + return resp, nil + else + return nil, resp + end end, {space, conditions, opts}) end @@ -926,3 +929,17 @@ for case_name_template, case in pairs(gh_373_types_cases) do case(g, read_impl) end end + +pgroup.before_test( + 'test_pairs_merger_process_storage_error', + read_scenario.before_merger_process_storage_error +) + +pgroup.test_pairs_merger_process_storage_error = function(g) + read_scenario.merger_process_storage_error(g, read_impl) +end + +pgroup.after_test( + 'test_pairs_merger_process_storage_error', + read_scenario.after_merger_process_storage_error +) diff --git a/test/integration/read_scenario.lua b/test/integration/read_scenario.lua index 216e8fb9..7d1030cb 100644 --- a/test/integration/read_scenario.lua +++ b/test/integration/read_scenario.lua @@ -50,11 +50,12 @@ local function gh_418_read_with_secondary_noneq_index_condition(cg, read) -- iterator had erroneously expected tuples to be sorted by `last_login` -- index while iterating on `city` index. Before the issue had beed fixed, -- user had received only one record instead of two. - local result = read(cg, + local result, err = read(cg, 'logins', {{'=', 'city', 'Tatsumi Port Island'}, {'<=', 'last_login', 42}}, {bucket_id = PINNED_BUCKET_NO} ) + t.assert_equals(err, nil) if type(result) == 'number' then -- crud.count t.assert_equals(result, 2) @@ -546,8 +547,91 @@ for space_kind, space_option in pairs(datetime_condition_space_options) do end end + +local function before_merger_process_storage_error(cg) + helpers.call_on_storages(cg.cluster, function(server) + server:exec(function() + local space + if box.info.ro == false then + space = box.schema.space.create('speedy_gonzales', {if_not_exists = true}) + + space:format({ + {name = 'id', type = 'unsigned'}, + {name = 'bucket_id', type = 'unsigned'}, + }) + + space:create_index('pk', { + parts = {'id'}, + if_not_exists = true, + }) + + space:create_index('bucket_id', { + parts = {'bucket_id'}, + unique = false, + if_not_exists = true, + }) + end + + local real_select_impl = rawget(_G, '_crud').select_on_storage + rawset(_G, '_real_select_impl', real_select_impl) + + local real_select_readview_impl = rawget(_G, '_crud').select_readview_on_storage + rawset(_G, '_real_select_readview_impl', real_select_readview_impl) + + -- Drop right before select to cause storage-side error. + -- Work guaranteed only with mode = 'write'. + local function erroneous_select_impl(...) + if box.info.ro == false then + space:drop() + end + + return real_select_impl(...) + end + rawget(_G, '_crud').select_on_storage = erroneous_select_impl + + -- Close right before select to cause storage-side error. + -- Work guaranteed only with mode = 'write'. + local function erroneous_select_readview_impl(space_name, index_id, conditions, opts) + local list = box.read_view.list() + + for k,v in pairs(list) do + if v.id == opts.readview_id then + list[k]:close() + end + end + + return real_select_readview_impl(space_name, index_id, conditions, opts) + end + rawget(_G, '_crud').select_readview_on_storage = erroneous_select_readview_impl + end) + end) +end + +local function merger_process_storage_error(cg, read) + local _, err = read(cg, 'speedy_gonzales', {{'==', 'id', 1}}) + t.assert_not_equals(err, nil) + + local err_msg = err.err or tostring(err) + t.assert_str_contains(err_msg, "Space \"speedy_gonzales\" doesn't exist") +end + +local function after_merger_process_storage_error(cg) + helpers.call_on_storages(cg.cluster, function(server) + server:exec(function() + local real_select_impl = rawget(_G, '_real_select_impl') + rawget(_G, '_crud').select_on_storage = real_select_impl + + local real_select_readview_impl = rawget(_G, '_real_select_readview_impl') + rawget(_G, '_crud').select_readview_on_storage = real_select_readview_impl + end) + 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, gh_373_read_with_datetime_condition_cases = gh_373_read_with_datetime_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 ef1fbe9b..24d0e4c0 100644 --- a/test/integration/select_readview_test.lua +++ b/test/integration/select_readview_test.lua @@ -2493,11 +2493,13 @@ local function read_impl(cg, space, conditions, opts) t.assert_equals(err, nil) local resp, err = rv:select(space, conditions, opts) - t.assert_equals(err, nil) - rv:close() - return crud.unflatten_rows(resp.rows, resp.metadata) + if err ~= nil then + return nil, err + end + + return crud.unflatten_rows(resp.rows, resp.metadata), nil end, {space, conditions, opts}) end @@ -2517,3 +2519,17 @@ for case_name_template, case in pairs(gh_373_types_cases) do case(g, read_impl) end end + +pgroup.before_test( + 'test_select_merger_process_storage_error', + read_scenario.before_merger_process_storage_error +) + +pgroup.test_select_merger_process_storage_error = function(g) + read_scenario.merger_process_storage_error(g, read_impl) +end + +pgroup.after_test( + 'test_select_merger_process_storage_error', + read_scenario.after_merger_process_storage_error +) diff --git a/test/integration/select_test.lua b/test/integration/select_test.lua index de0be3a0..b494de5d 100644 --- a/test/integration/select_test.lua +++ b/test/integration/select_test.lua @@ -2273,9 +2273,12 @@ local function read_impl(cg, space, conditions, opts) opts.mode = 'write' local resp, err = cg.cluster.main_server:call('crud.select', {space, conditions, opts}) - t.assert_equals(err, nil) - return crud.unflatten_rows(resp.rows, resp.metadata) + if err ~= nil then + return nil, err + end + + return crud.unflatten_rows(resp.rows, resp.metadata), nil end pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g) @@ -2294,3 +2297,17 @@ for case_name_template, case in pairs(gh_373_types_cases) do case(g, read_impl) end end + +pgroup.before_test( + 'test_select_merger_process_storage_error', + read_scenario.before_merger_process_storage_error +) + +pgroup.test_select_merger_process_storage_error = function(g) + read_scenario.merger_process_storage_error(g, read_impl) +end + +pgroup.after_test( + 'test_select_merger_process_storage_error', + read_scenario.after_merger_process_storage_error +) From 771904f1f060a3a33b334f222e0bf0251b361b1b Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Tue, 12 Mar 2024 19:17:47 +0300 Subject: [PATCH 6/6] 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