Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scan: fix nil filters #425

Merged
merged 8 commits into from
Mar 20, 2024
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
non-iterating indexes (#373).
* Passing errors from storages for merger operations (`crud.select`,
`crud.pairs`, `readview:select`, `readview:pairs`) (#423).
* Working with `nil` operand conditions in case of non-indexed fields or
non-iterating indexes (#422).

## [1.4.3] - 05-02-24

Expand Down
83 changes: 35 additions & 48 deletions crud/common/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ local fun = require('fun')
local vshard = require('vshard')
local log = require('log')

local datetime_supported, datetime = pcall(require, 'datetime')

local is_cartridge, cartridge = pcall(require, 'cartridge')
local is_cartridge_hotreload, cartridge_hotreload = pcall(require, 'cartridge.hotreload')

Expand Down Expand Up @@ -605,10 +603,34 @@ local function determine_enabled_features()
enabled_tarantool_features.fieldpaths = is_version_ge(major, minor, patch, suffix,
2, 3, 1, nil)

-- since Tarantool 2.4.1
-- Full support (Lua type, space format type and indexes) for decimal type
-- is since Tarantool 2.3.1 [1]
--
-- [1] https://github.com/tarantool/tarantool/commit/485439e33196e26d120e622175f88b4edc7a5aa1
enabled_tarantool_features.decimals = is_version_ge(major, minor, patch, suffix,
2, 3, 1, nil)

-- Full support (Lua type, space format type and indexes) for uuid type
-- is since Tarantool 2.4.1 [1]
--
-- [1] https://github.com/tarantool/tarantool/commit/b238def8065d20070dcdc50b54c2536f1de4c7c7
enabled_tarantool_features.uuids = is_version_ge(major, minor, patch, suffix,
2, 4, 1, nil)

-- Full support (Lua type, space format type and indexes) for datetime type
-- is since Tarantool 2.10.0-beta2 [1]
--
-- [1] https://github.com/tarantool/tarantool/commit/3bd870261c462416c29226414fe0a2d79aba0c74
enabled_tarantool_features.datetimes = is_version_ge(major, minor, patch, suffix,
2, 10, 0, 'beta2')

-- Full support (Lua type, space format type and indexes) for datetime type
-- is since Tarantool 2.10.0-rc1 [1]
--
-- [1] https://github.com/tarantool/tarantool/commit/38f0c904af4882756c6dc802f1895117d3deae6a
enabled_tarantool_features.intervals = is_version_ge(major, minor, patch, suffix,
2, 10, 0, 'rc1')

-- since Tarantool 2.6.3 / 2.7.2 / 2.8.1
enabled_tarantool_features.jsonpath_indexes = is_version_ge(major, minor, patch, suffix,
2, 8, 1, nil)
Expand Down Expand Up @@ -664,52 +686,19 @@ local function determine_enabled_features()
2, 2, 0, nil)
end

function utils.tarantool_supports_fieldpaths()
if enabled_tarantool_features.fieldpaths == nil then
determine_enabled_features()
end

return enabled_tarantool_features.fieldpaths
end

function utils.tarantool_supports_uuids()
if enabled_tarantool_features.uuids == nil then
determine_enabled_features()
end

return enabled_tarantool_features.uuids
end

function utils.tarantool_supports_jsonpath_indexes()
if enabled_tarantool_features.jsonpath_indexes == nil then
determine_enabled_features()
end

return enabled_tarantool_features.jsonpath_indexes
end

function utils.tarantool_has_builtin_merger()
if enabled_tarantool_features.builtin_merger == nil then
determine_enabled_features()
end

return enabled_tarantool_features.builtin_merger
end
determine_enabled_features()

function utils.tarantool_supports_external_merger()
if enabled_tarantool_features.external_merger == nil then
determine_enabled_features()
for feature_name, feature_enabled in pairs(enabled_tarantool_features) do
local util_name
if feature_name == 'builtin_merger' then
util_name = ('tarantool_has_%s'):format(feature_name)
else
util_name = ('tarantool_supports_%s'):format(feature_name)
end

return enabled_tarantool_features.external_merger
end
local util_func = function() return feature_enabled end

function utils.tarantool_supports_netbox_skip_header_option()
if enabled_tarantool_features.netbox_skip_header_option == nil then
determine_enabled_features()
end

return enabled_tarantool_features.netbox_skip_header_option
utils[util_name] = util_func
end

local function add_nullable_fields_recursive(operations, operations_map, space_format, tuple, id)
Expand Down Expand Up @@ -1337,9 +1326,7 @@ function utils.is_cartridge_hotreload_supported()
return true, cartridge_hotreload
end

local interval_supported = datetime_supported and (datetime.interval ~= nil)

if interval_supported then
if utils.tarantool_supports_intervals() then
-- https://github.com/tarantool/tarantool/blob/0510ffa07afd84a70c9c6f1a4c28aacd73a393d6/src/lua/datetime.lua#L175-179
local interval_t = ffi.typeof('struct interval')

Expand Down
46 changes: 28 additions & 18 deletions crud/compare/filters.lua
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
local datetime_supported, datetime = pcall(require, 'datetime')
local decimal_supported, decimal = pcall(require, 'decimal')

local errors = require('errors')

local _, datetime = pcall(require, 'datetime')
local _, decimal = pcall(require, 'decimal')

local utils = require('crud.common.utils')
local dev_checks = require('crud.common.dev_checks')
local collations = require('crud.common.collations')
Expand Down Expand Up @@ -160,12 +160,12 @@ local function format_value(value)
return tostring(value)
elseif type(value) == 'boolean' then
return tostring(value)
elseif decimal_supported and decimal.is_decimal(value) then
elseif utils.tarantool_supports_decimals() and decimal.is_decimal(value) then
-- decimal supports comparison with string.
return ("%q"):format(tostring(value))
elseif utils.is_uuid(value) then
return ("%q"):format(value)
elseif datetime_supported and datetime.is_datetime(value) then
elseif utils.tarantool_supports_datetimes() and datetime.is_datetime(value) then
return ("%q"):format(value:format())
elseif utils.is_interval(value) then
-- As for Tarantool 3.0 and older, datetime intervals
Expand Down Expand Up @@ -345,9 +345,14 @@ local function gen_eq_func_code(func_name, cond, func_args_code)
local header = LIB_FUNC_HEADER_TEMPLATE:format(func_name, func_args_code)
table.insert(func_code_lines, header)

local return_line = string.format(
' return %s', concat_conditions(eq_conds, 'and')
)
local return_line
if #eq_conds > 0 then
return_line = string.format(
' return %s', concat_conditions(eq_conds, 'and')
)
else -- nil condition is treated as no condition
return_line = ' return true'
end
table.insert(func_code_lines, return_line)

table.insert(func_code_lines, 'end')
Expand Down Expand Up @@ -398,18 +403,23 @@ local function gen_cmp_array_func_code(operator, func_name, cond, func_args_code
local results = results_by_operators[operator]
assert(results ~= nil)

for i = 1, #eq_conds do
local comp_value_code = table.concat({
string.format(' if %s then return %s end', lt_conds[i], results.le),
string.format(' if not %s then return %s end', eq_conds[i], results.not_eq),
'',
}, '\n')
if #eq_conds > 0 then
for i = 1, #eq_conds do
local comp_value_code = table.concat({
string.format(' if %s then return %s end', lt_conds[i], results.le),
string.format(' if not %s then return %s end', eq_conds[i], results.not_eq),
'',
}, '\n')

table.insert(func_code_lines, comp_value_code)
end
table.insert(func_code_lines, comp_value_code)
end

local return_code = (' return %s'):format(results.default)
table.insert(func_code_lines, return_code)
local return_code = (' return %s'):format(results.default)
table.insert(func_code_lines, return_code)
else -- nil condition is treated as no condition
local return_line = ' return true'
table.insert(func_code_lines, return_line)
end

table.insert(func_code_lines, 'end')

Expand Down
10 changes: 3 additions & 7 deletions test/entrypoint/srv_select/storage_init.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
local datetime_supported, datetime = pcall(require, 'datetime')
local decimal_supported, _ = pcall(require, 'decimal')

local crud_utils = require('crud.common.utils')

return function()
Expand Down Expand Up @@ -231,7 +228,7 @@ return function()
if_not_exists = true,
})

if decimal_supported then
if crud_utils.tarantool_supports_decimals() then
local decimal_format = {
{name = 'id', type = 'unsigned'},
{name = 'bucket_id', type = 'unsigned'},
Expand Down Expand Up @@ -327,7 +324,7 @@ return function()
})
end

if datetime_supported then
if crud_utils.tarantool_supports_datetimes() then
local datetime_format = {
{name = 'id', type = 'unsigned'},
{name = 'bucket_id', type = 'unsigned'},
Expand Down Expand Up @@ -423,8 +420,7 @@ return function()
})
end

local interval_supported = datetime_supported and (datetime.interval ~= nil)
if interval_supported then
if crud_utils.tarantool_supports_intervals() then
-- Interval is non-indexable.
local interval_space = box.schema.space.create('interval', {
if_not_exists = true,
Expand Down
26 changes: 19 additions & 7 deletions test/helper.lua
Original file line number Diff line number Diff line change
Expand Up @@ -958,20 +958,32 @@ function helpers.prepare_ordered_data(g, space, expected_objects, bucket_id, ord
t.assert_equals(objects, expected_objects)
end

function helpers.is_decimal_supported()
return crud_utils.tarantool_supports_decimals()
end

function helpers.is_uuid_supported()
return crud_utils.tarantool_supports_uuids()
end

function helpers.is_datetime_supported()
return crud_utils.tarantool_supports_datetimes()
end

function helpers.is_interval_supported()
return crud_utils.tarantool_supports_intervals()
end

function helpers.skip_decimal_unsupported()
local module_available, _ = pcall(require, 'decimal')
t.skip_if(not module_available, 'decimal is not supported')
t.skip_if(not helpers.is_decimal_supported(), 'decimal is not supported')
end

function helpers.skip_datetime_unsupported()
local module_available, _ = pcall(require, 'datetime')
t.skip_if(not module_available, 'datetime is not supported')
t.skip_if(not helpers.is_datetime_supported(), 'datetime is not supported')
end

function helpers.skip_interval_unsupported()
local datetime_supported, datetime = pcall(require, 'datetime')
local interval_supported = datetime_supported and (datetime.interval ~= nil)
t.skip_if(not interval_supported, 'interval is not supported')
t.skip_if(not helpers.is_interval_supported(), 'interval is not supported')
end

function helpers.merge_tables(t1, t2, ...)
Expand Down
8 changes: 8 additions & 0 deletions test/integration/count_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -893,3 +893,11 @@ for case_name_template, case in pairs(gh_373_types_cases) do
case(g, read_impl)
end
end

for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do
local case_name = 'test_' .. case_name_template:format('count')

pgroup[case_name] = function(g)
case(g, read_impl)
end
end
8 changes: 8 additions & 0 deletions test/integration/pairs_readview_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -936,3 +936,11 @@ pgroup.after_test(
'test_pairs_merger_process_storage_error',
read_scenario.after_merger_process_storage_error
)

for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do
local case_name = 'test_' .. case_name_template:format('pairs')

pgroup[case_name] = function(g)
case(g, read_impl)
end
end
8 changes: 8 additions & 0 deletions test/integration/pairs_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -944,3 +944,11 @@ pgroup.after_test(
'test_pairs_merger_process_storage_error',
read_scenario.after_merger_process_storage_error
)

for case_name_template, case in pairs(read_scenario.gh_422_nullability_cases) do
local case_name = 'test_' .. case_name_template:format('pairs')

pgroup[case_name] = function(g)
case(g, read_impl)
end
end
Loading
Loading