diff --git a/dbcon/joblist/tupleunion.cpp b/dbcon/joblist/tupleunion.cpp index 62216f4e6..f6a64ef0e 100644 --- a/dbcon/joblist/tupleunion.cpp +++ b/dbcon/joblist/tupleunion.cpp @@ -504,34 +504,29 @@ void TupleUnion::normalize(const Row& in, Row* out) { dec1: int diff = out->getScale(i) - in.getScale(i); + + idbassert(diff >= 0); /* Signed INT to XDecimal - TODO: there are a few problems here: - - This code is not wide decimal friendly. - `uint64_t val` can overflow when applying a positive scale - It's not possible to make a reproducible bug report - at the moment: MCOL-4612 and MCOL-4613 should be fixed first. - - Using uint64_t is wrong here. The data type of "in" field is - signed. In case of a negative diff, the result will be wrong, - because division (unlike multiplication) is sensitive to - the signess of the operands. - Perhaps diff cannot be negative and we can put an assert for it. - Anyway, it's safer to change `uint64_t val` to `int64_t val`. + TODO: - This code does not handle overflow that may happen on - scale multiplication (MCOL-4613). Instead of returning a garbage + scale multiplication. Instead of returning a garbage value we should probably apply saturation here. In long terms we should implement DECIMAL(65,x) to avoid overflow completely (so the UNION between DECIMAL and integer can choose a proper DECIMAL(M,N) result data type to guarantee that any incoming integer value can fit into it). */ - // TODO: isn't overflow possible below? - uint64_t val = datatypes::applySignedScale(in.getIntField(i), diff); - if (out->getColumnWidth(i) == datatypes::MAXDECIMALWIDTH) + { + int128_t val = datatypes::applySignedScale(in.getIntField(i), diff); out->setInt128Field(val, i); + } else + { + int64_t val = datatypes::applySignedScale(in.getIntField(i), diff); out->setIntField(val, i); + } break; } @@ -625,20 +620,24 @@ dec1: { dec2: int diff = out->getScale(i) - in.getScale(i); + + idbassert(diff >= 0); /* Unsigned INT to XDecimal - TODO: There are a few problems here: - - It should use in.getUintField() instead of in.getIntField() - - All problems mentioned in the code under label "dec1:" are + TODO: + - The overflow problem mentioned in the code under label "dec1:" is also applicable here. */ - // TODO: isn't overflow possible below? - uint64_t val = datatypes::applySignedScale(in.getIntField(i), diff); - if (out->getColumnWidth(i) == datatypes::MAXDECIMALWIDTH) + { + int128_t val = datatypes::applySignedScale(in.getUintField(i), diff); out->setInt128Field(val, i); + } else - out->setIntField(val, i); + { + uint64_t val = datatypes::applySignedScale(in.getUintField(i), diff); + out->setUintField(val, i); + } break; } @@ -1109,9 +1108,8 @@ dec4: /* have to pick a scale to use for the double. using 5.. out->setInt128Field(isInputWide ? val128 : val, i); else if (out->getScale(i) > scale) { - int128_t divisor = 1; - datatypes::getScaleDivisor(divisor, out->getScale(i) - scale); - int128_t temp = isInputWide ? divisor*val128 : divisor*val; + int128_t temp = datatypes::applySignedScale( + isInputWide ? val128 : val, out->getScale(i) - scale); out->setInt128Field(temp, i); } else // should not happen, the output's scale is the largest @@ -1124,7 +1122,11 @@ dec4: /* have to pick a scale to use for the double. using 5.. if (out->getScale(i) == scale) out->setIntField(val, i); else if (out->getScale(i) > scale) - out->setIntField(IDB_pow[out->getScale(i) - scale]*val, i); + { + int64_t temp = datatypes::applySignedScale( + val, out->getScale(i) - scale); + out->setIntField(temp, i); + } else // should not happen, the output's scale is the largest throw logic_error("TupleUnion::normalize(): incorrect scale setting"); } diff --git a/mysql-test/columnstore/csinternal/devregression/r/mcs7147_regression_bug4388.result b/mysql-test/columnstore/csinternal/devregression/r/mcs7147_regression_bug4388.result index 9d81c6f74..ec0f8809b 100644 --- a/mysql-test/columnstore/csinternal/devregression/r/mcs7147_regression_bug4388.result +++ b/mysql-test/columnstore/csinternal/devregression/r/mcs7147_regression_bug4388.result @@ -1,9 +1,11 @@ USE tpch1; -create table if not exists bug4388( +set default_storage_engine=columnstore; +drop table if exists bug4388; +create table bug4388( `venta_clave` int(10) DEFAULT NULL, `cantidad` decimal(10,3) DEFAULT NULL, `changev` decimal(18,4) DEFAULT NULL -) engine=columnstore; +); insert into bug4388 values (null,null,6.8000); select coalesce(sum(changev),0) as col1 from bug4388; col1 @@ -38,4 +40,76 @@ select sum(co) from ( select sum(col1) as co from (select sum(changev) as col1 from bug4388 ) t ) res; sum(co) 6.8000 +select sum(col1) as co from ( select 984467440737095516 +as col1 union all select coalesce(sum(changev),0) as col1 from +bug4388 ) t; +co +984467440737095522.8000 +select sum(col1) as co from ( select 18446744073709551612 +as col1 union all select coalesce(sum(changev),0) as col1 from +bug4388 ) t; +co +18446744073709551618.8000 drop table bug4388; +# +# MCOL-4613 Garbage result of a union between huge narrow DECIMAL and BIGINT +# +drop table if exists t1; +drop table if exists t2; +drop table if exists t3; +create table t1 (a decimal(17,1), b bigint); +insert into t1 values (9999999999999999.9, 999999999999999999); +select * from (select a from t1 union select b from t1) tu order by a; +a +9999999999999999.9 +999999999999999999.0 +drop table t1; +# +# MCOL-4612 A subquery with a union for DECIMAL and BIGINT returns zeros +# +create table t1 (a decimal(17,1), b bigint); +insert into t1 values (1, 1); +insert into t1 values (9999999999999999, 99999999999999999); +select * from (select a from t1 union select b from t1) tu order by a; +a +1.0 +9999999999999999.0 +99999999999999999.0 +select * from (select a from t1 union all select b from t1) tu order by a; +a +1.0 +1.0 +9999999999999999.0 +99999999999999999.0 +drop table t1; +create table t1 (a decimal(18,5), b decimal(18,5) unsigned); +create table t2 (a bigint, b bigint unsigned); +create table t3 (a decimal(38,10), b decimal(38,10) unsigned); +insert into t1 values +(-1234567890123.12345, 1234567890123.12345), +(-1234567890123.1234, 1234567890123.1234), +(-9999999999999.99999, 9999999999999.99999), +(-999999999999.99999, 999999999999.99999), +(-99999999999.99999, 99999999999.99999); +insert into t2 values +(-123456789012345, 123456789012345), +(9223372036854775807, 18446744073709551613), +(-9223372036854775806, 0); +insert into t3 values +(-9999999999999999999999999999.9999999999, 9999999999999999999999999999.9999999999), +(-1234567890123456789012345678.9012345678, 1234567890123456789012345678.9012345678); +select * from (select a,b from t1 union select a,b from t2 union select a,b from t3) tu order by a,b; +a b +-9999999999999999999999999999.9999999999 9999999999999999999999999999.9999999999 +-1234567890123456789012345678.9012345678 1234567890123456789012345678.9012345678 +-9223372036854775806.0000000000 0.0000000000 +-123456789012345.0000000000 123456789012345.0000000000 +-9999999999999.9999900000 9999999999999.9999900000 +-1234567890123.1234500000 1234567890123.1234500000 +-1234567890123.1234000000 1234567890123.1234000000 +-999999999999.9999900000 999999999999.9999900000 +-99999999999.9999900000 99999999999.9999900000 +9223372036854775807.0000000000 18446744073709551613.0000000000 +drop table t1; +drop table t2; +drop table t3; diff --git a/mysql-test/columnstore/csinternal/devregression/t/mcs7147_regression_bug4388.test b/mysql-test/columnstore/csinternal/devregression/t/mcs7147_regression_bug4388.test index 785c87f56..3aa62f0ed 100644 --- a/mysql-test/columnstore/csinternal/devregression/t/mcs7147_regression_bug4388.test +++ b/mysql-test/columnstore/csinternal/devregression/t/mcs7147_regression_bug4388.test @@ -8,11 +8,17 @@ # USE tpch1; # -create table if not exists bug4388( + +set default_storage_engine=columnstore; + +--disable_warnings +drop table if exists bug4388; +--enable_warnings +create table bug4388( `venta_clave` int(10) DEFAULT NULL, `cantidad` decimal(10,3) DEFAULT NULL, `changev` decimal(18,4) DEFAULT NULL -) engine=columnstore; +); insert into bug4388 values (null,null,6.8000); select coalesce(sum(changev),0) as col1 from bug4388; select sum(co) + 0 from ( select sum(col1) as co from ( select 0 @@ -31,6 +37,68 @@ select sum(col1) from select sum(co) from (select sum(changev) as co from bug4388 ) t; select sum(co) from ( select sum(col1) as co from (select sum(changev) as col1 from bug4388 ) t ) res; -drop table bug4388; -# +select sum(col1) as co from ( select 984467440737095516 +as col1 union all select coalesce(sum(changev),0) as col1 from +bug4388 ) t; + +select sum(col1) as co from ( select 18446744073709551612 +as col1 union all select coalesce(sum(changev),0) as col1 from +bug4388 ) t; + +drop table bug4388; + +--echo # +--echo # MCOL-4613 Garbage result of a union between huge narrow DECIMAL and BIGINT +--echo # + +--disable_warnings +drop table if exists t1; +drop table if exists t2; +drop table if exists t3; +--enable_warnings + +# Original test case from the MCOL-4613 issue description +create table t1 (a decimal(17,1), b bigint); +insert into t1 values (9999999999999999.9, 999999999999999999); +select * from (select a from t1 union select b from t1) tu order by a; +drop table t1; + +--echo # +--echo # MCOL-4612 A subquery with a union for DECIMAL and BIGINT returns zeros +--echo # + +# Original test case from the MCOL-4612 issue description +create table t1 (a decimal(17,1), b bigint); +insert into t1 values (1, 1); +insert into t1 values (9999999999999999, 99999999999999999); +select * from (select a from t1 union select b from t1) tu order by a; +select * from (select a from t1 union all select b from t1) tu order by a; +drop table t1; + +# Additional test cases for MCOL-4613 and MCOL-4612 +create table t1 (a decimal(18,5), b decimal(18,5) unsigned); +create table t2 (a bigint, b bigint unsigned); +create table t3 (a decimal(38,10), b decimal(38,10) unsigned); + +insert into t1 values +(-1234567890123.12345, 1234567890123.12345), +(-1234567890123.1234, 1234567890123.1234), +(-9999999999999.99999, 9999999999999.99999), +(-999999999999.99999, 999999999999.99999), +(-99999999999.99999, 99999999999.99999); + +insert into t2 values +(-123456789012345, 123456789012345), +(9223372036854775807, 18446744073709551613), +(-9223372036854775806, 0); + +insert into t3 values +(-9999999999999999999999999999.9999999999, 9999999999999999999999999999.9999999999), +(-1234567890123456789012345678.9012345678, 1234567890123456789012345678.9012345678); + +select * from (select a,b from t1 union select a,b from t2 union select a,b from t3) tu order by a,b; + +drop table t1; +drop table t2; +drop table t3; diff --git a/utils/dataconvert/dataconvert.cpp b/utils/dataconvert/dataconvert.cpp index e2027ef60..1d4c2f1d9 100644 --- a/utils/dataconvert/dataconvert.cpp +++ b/utils/dataconvert/dataconvert.cpp @@ -3037,6 +3037,17 @@ DataConvert::joinColTypeForUnion(datatypes::SystemCatalog::TypeHolderStd &unione case datatypes::SystemCatalog::UINT: case datatypes::SystemCatalog::UBIGINT: case datatypes::SystemCatalog::UDECIMAL: + + unionedType.precision = std::max(type.precision, unionedType.precision); + unionedType.scale = std::max(type.scale, unionedType.scale); + + if (datatypes::Decimal::isWideDecimalTypeByPrecision(unionedType.precision)) + { + unionedType.colDataType = datatypes::SystemCatalog::DECIMAL; + unionedType.colWidth = datatypes::MAXDECIMALWIDTH; + break; + } + if (type.colWidth > unionedType.colWidth) { unionedType.colDataType = type.colDataType; @@ -3054,10 +3065,6 @@ DataConvert::joinColTypeForUnion(datatypes::SystemCatalog::TypeHolderStd &unione unionedType.colDataType = datatypes::SystemCatalog::DECIMAL; } - if (type.precision > unionedType.precision) - unionedType.precision = type.precision; - - unionedType.scale = (type.scale > unionedType.scale) ? type.scale : unionedType.scale; break; case datatypes::SystemCatalog::DATE: