Skip to content

Commit

Permalink
scan: fix decimal filters precision loss
Browse files Browse the repository at this point in the history
Before this patch, decimals were cast to numbers in filters through
`tostring` in codegen. This patch fixes this precision loss.

Part of #373
  • Loading branch information
DifferentialOrange committed Mar 12, 2024
1 parent 258bb09 commit 8cf201f
Show file tree
Hide file tree
Showing 10 changed files with 382 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

local errors = require('errors')

Expand Down Expand Up @@ -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
Expand Down
97 changes: 97 additions & 0 deletions test/entrypoint/srv_select/storage_init.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
local datetime_supported, _ = pcall(require, 'datetime')
local decimal_supported, _ = pcall(require, 'decimal')
local crud_utils = require('crud.common.utils')

return function()
Expand Down Expand Up @@ -229,6 +230,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'},
Expand Down
23 changes: 23 additions & 0 deletions test/helper.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 6 additions & 1 deletion test/integration/count_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion test/integration/pairs_readview_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion test/integration/pairs_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,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)
Expand Down
Loading

0 comments on commit 8cf201f

Please sign in to comment.