Fix incorrect result in case of decimal precision loss in IN operator

This commit is contained in:
vdimir 2022-09-09 13:52:02 +00:00
parent c2634c9fae
commit 5c9bfbc116
No known key found for this signature in database
GPG Key ID: 6EE4CE2BEDC51862
4 changed files with 70 additions and 11 deletions

View File

@ -1,8 +1,9 @@
#include <memory>
#include <Common/quoteString.h>
#include <Common/typeid_cast.h>
#include <Columns/ColumnArray.h>
#include <Columns/ColumnFixedString.h>
#include <Common/FieldVisitorsAccurateComparison.h>
#include <Core/ColumnNumbers.h>
#include <Core/ColumnWithTypeAndName.h>
@ -22,9 +23,12 @@
#include <DataTypes/DataTypeLowCardinality.h>
#include <DataTypes/DataTypesNumber.h>
#include <DataTypes/FieldToDataType.h>
#include <DataTypes/DataTypesDecimal.h>
#include <Columns/ColumnSet.h>
#include <Columns/ColumnArray.h>
#include <Columns/ColumnConst.h>
#include <Columns/ColumnFixedString.h>
#include <Columns/ColumnSet.h>
#include <Storages/StorageSet.h>
@ -87,6 +91,22 @@ static size_t getTypeDepth(const DataTypePtr & type)
return 0;
}
/// Applies stricter rules than convertFieldToType:
/// Doesn't allow :
/// - loss of precision with `Decimals`
static bool convertFieldToTypeStrict(const Field & from_value, const IDataType & to_type, Field & result_value)
{
result_value = convertFieldToType(from_value, to_type);
if (Field::isDecimal(from_value.getType()) && Field::isDecimal(result_value.getType()))
return applyVisitor(FieldVisitorAccurateEquals{}, from_value, result_value);
return true;
}
/// The `convertFieldToTypeStrict` is used to prevent unexpected results in case of conversion with loss of precision.
/// Example: `SELECT 33.3 :: Decimal(9, 1) AS a WHERE a IN (33.33 :: Decimal(9, 2))`
/// 33.33 in the set is converted to 33.3, but it is not equal to 33.3 in the column, so the result should still be empty.
/// We can not include values that don't represent any possible value from the type of filtered column to the set.
template<typename Collection>
static Block createBlockFromCollection(const Collection & collection, const DataTypes & types, bool transform_null_in)
{
@ -103,9 +123,10 @@ static Block createBlockFromCollection(const Collection & collection, const Data
{
if (columns_num == 1)
{
auto field = convertFieldToType(value, *types[0]);
Field field;
bool is_conversion_ok = convertFieldToTypeStrict(value, *types[0], field);
bool need_insert_null = transform_null_in && types[0]->isNullable();
if (!field.isNull() || need_insert_null)
if (is_conversion_ok && (!field.isNull() || need_insert_null))
columns[0]->insert(std::move(field));
}
else
@ -127,7 +148,10 @@ static Block createBlockFromCollection(const Collection & collection, const Data
size_t i = 0;
for (; i < tuple_size; ++i)
{
tuple_values[i] = convertFieldToType(tuple[i], *types[i]);
bool is_conversion_ok = convertFieldToTypeStrict(tuple[i], *types[i], tuple_values[i]);
if (!is_conversion_ok)
break;
bool need_insert_null = transform_null_in && types[i]->isNullable();
if (tuple_values[i].isNull() && !need_insert_null)
break;

View File

@ -1,18 +1,17 @@
DROP TABLE IF EXISTS temp;
CREATE TABLE temp
(
x Decimal(38,2),
y Nullable(Decimal(38,2))
x Decimal(38, 2),
y Nullable(Decimal(38, 2))
) ENGINE = Memory;
INSERT INTO temp VALUES (32, 32), (64, 64), (128, 128);
SELECT * FROM temp WHERE x IN (toDecimal128(128, 2));
SELECT * FROM temp WHERE y IN (toDecimal128(128, 2));
SELECT * FROM temp WHERE x IN (toDecimal128(128, 1));
SELECT * FROM temp WHERE x IN (toDecimal128(128, 2));
SELECT * FROM temp WHERE x IN (toDecimal128(128, 3));
SELECT * FROM temp WHERE y IN (toDecimal128(128, 1));
SELECT * FROM temp WHERE y IN (toDecimal128(128, 2));
SELECT * FROM temp WHERE y IN (toDecimal128(128, 3));
SELECT * FROM temp WHERE x IN (toDecimal32(32, 1));
@ -29,4 +28,7 @@ SELECT * FROM temp WHERE y IN (toDecimal64(64, 1));
SELECT * FROM temp WHERE y IN (toDecimal64(64, 2));
SELECT * FROM temp WHERE y IN (toDecimal64(64, 3));
SELECT * FROM temp WHERE x IN (toDecimal256(256, 1)); -- { serverError 53 }
SELECT * FROM temp WHERE y IN (toDecimal256(256, 1)); -- { serverError 53 }
DROP TABLE IF EXISTS temp;

View File

@ -0,0 +1,10 @@
1
1
1
1
1
1
1
1
1
1

View File

@ -0,0 +1,23 @@
DROP TABLE IF EXISTS dtest;
SELECT count() == 0 FROM (SELECT '33.3' :: Decimal(9, 1) AS a WHERE a IN ('33.33' :: Decimal(9, 2)));
CREATE TABLE dtest ( `a` Decimal(18, 0), `b` Decimal(18, 1), `c` Decimal(36, 0) ) ENGINE = Memory;
INSERT INTO dtest VALUES ('33', '44.4', '35');
SELECT count() == 0 FROM dtest WHERE a IN toDecimal32('33.3000', 4);
SELECT count() == 0 FROM dtest WHERE a IN toDecimal64('33.3000', 4);
SELECT count() == 0 FROM dtest WHERE a IN toDecimal128('33.3000', 4);
SELECT count() == 0 FROM dtest WHERE a IN toDecimal256('33.3000', 4); -- { serverError 53 }
SELECT count() == 0 FROM dtest WHERE b IN toDecimal32('44.4000', 0);
SELECT count() == 0 FROM dtest WHERE b IN toDecimal64('44.4000', 0);
SELECT count() == 0 FROM dtest WHERE b IN toDecimal128('44.4000', 0);
SELECT count() == 0 FROM dtest WHERE b IN toDecimal256('44.4000', 0); -- { serverError 53 }
SELECT count() == 1 FROM dtest WHERE b IN toDecimal32('44.4000', 4);
SELECT count() == 1 FROM dtest WHERE b IN toDecimal64('44.4000', 4);
SELECT count() == 1 FROM dtest WHERE b IN toDecimal128('44.4000', 4);
SELECT count() == 1 FROM dtest WHERE b IN toDecimal256('44.4000', 4); -- { serverError 53 }
DROP TABLE IF EXISTS dtest;