Skip to content

Commit

Permalink
scan: fix nil filters
Browse files Browse the repository at this point in the history
Before this patch, some nil conditions failed with internal filter
library build error. This patch fixes this internal error, as well as
normalize filters to behave similar to core indexes.

The logic in core Tarantool select is as follows. `nil` condition in
index select is an absence of condition, thus all data is returned
disregarding the condition (condition may affect the order). `box.NULL`
condition is a condition for the null value -- in case of EQ,
only records with null index value are returned, in case of GT,
all non-null values are returned since nulls are in the beginning of an
index and so on. `nil`s and `box.NULL`s in tuple are both satisfy
`box.NULL` equity.

After this patch, `nil` filter condition is treated as no condition.
This is a breaking change since conditions for `'>'` and `'<'`
operator with `nil` operand had resulted with empty response
before this patch. But since it was inconsistent with scanning index
conditions and wasn't intentional, we change it here.

Closes #422
  • Loading branch information
DifferentialOrange committed Mar 20, 2024
1 parent 864daf3 commit 5b05d38
Show file tree
Hide file tree
Showing 9 changed files with 396 additions and 13 deletions.
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
36 changes: 23 additions & 13 deletions crud/compare/filters.lua
Original file line number Diff line number Diff line change
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
4 changes: 4 additions & 0 deletions test/helper.lua
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,10 @@ function helpers.is_decimal_supported()
return crud_utils.tarantool_supports_decimals()
end

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

function helpers.is_datetime_supported()
return crud_utils.tarantool_supports_datetimes()
end
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

0 comments on commit 5b05d38

Please sign in to comment.