From c5d4a918ee8e8fa77ad1bf8e16822204e1e29513 Mon Sep 17 00:00:00 2001 From: Gagan Goel Date: Tue, 24 Nov 2020 20:15:33 -0500 Subject: [PATCH] MCOL-4188 Regression fixes for MCOL-641. 1. In TupleAggregateStep::configDeliveredRowGroup(), use jobInfo.projectionCols instead of jobInfo.nonConstCols for setting scale and precision if the source column is wide decimal. 2. Tighten rules for wide decimal processing. Specifically: a. Replace (precision > INT64MAXPRECISION) checks with (precision > INT64MAXPRECISION && precision <= INT128MAXPRECISION) b. At places where (colWidth == MAXDECIMALWIDTH) is not enough to determine if a column is wide decimal or not, also add a check on type being DECIMAL/UDECIMAL. --- datatypes/mcs_decimal.h | 54 +++++++-------------- dbcon/execplan/functioncolumn.h | 2 +- dbcon/joblist/tupleaggregatestep.cpp | 6 +-- dbcon/joblist/tupleunion.cpp | 5 +- primitives/linux-port/column.cpp | 6 ++- utils/funcexp/func_abs.cpp | 2 +- utils/funcexp/func_cast.cpp | 38 +++++++-------- utils/funcexp/func_inet_aton.cpp | 2 +- utils/funcexp/func_mod.cpp | 4 +- utils/funcexp/func_monthname.cpp | 2 +- utils/funcexp/func_sec_to_time.cpp | 2 +- utils/windowfunction/windowfunctiontype.cpp | 3 +- 12 files changed, 55 insertions(+), 71 deletions(-) diff --git a/datatypes/mcs_decimal.h b/datatypes/mcs_decimal.h index b079bafde..c256500ab 100644 --- a/datatypes/mcs_decimal.h +++ b/datatypes/mcs_decimal.h @@ -598,16 +598,14 @@ class VDecimal: public TSInt128 bool operator==(const VDecimal& rhs) const { - if (precision > datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + if (isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return s128Value == rhs.s128Value; else return (datatypes::Decimal::compare(*this, rhs) == 0); } - else if (precision > datatypes::INT64MAXPRECISION && - rhs.precision <= datatypes::INT64MAXPRECISION) + else if (isTSInt128ByPrecision() && !rhs.isTSInt128ByPrecision()) { const_cast(rhs).s128Value = rhs.value; @@ -616,8 +614,7 @@ class VDecimal: public TSInt128 else return (datatypes::Decimal::compare(*this, rhs) == 0); } - else if (precision <= datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + else if (!isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return (int128_t) value == rhs.s128Value; @@ -635,16 +632,14 @@ class VDecimal: public TSInt128 bool operator>(const VDecimal& rhs) const { - if (precision > datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + if (isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return s128Value > rhs.s128Value; else return (datatypes::Decimal::compare(*this, rhs) > 0); } - else if (precision > datatypes::INT64MAXPRECISION && - rhs.precision <= datatypes::INT64MAXPRECISION) + else if (isTSInt128ByPrecision() && !rhs.isTSInt128ByPrecision()) { VDecimal rhstmp(0, rhs.scale, rhs.precision, (int128_t) rhs.value); @@ -653,8 +648,7 @@ class VDecimal: public TSInt128 else return (datatypes::Decimal::compare(*this, rhstmp) > 0); } - else if (precision <= datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + else if (!isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return (int128_t) value > rhs.s128Value; @@ -672,16 +666,14 @@ class VDecimal: public TSInt128 bool operator<(const VDecimal& rhs) const { - if (precision > datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + if (isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return s128Value < rhs.s128Value; else return (datatypes::Decimal::compare(*this, rhs) < 0); } - else if (precision > datatypes::INT64MAXPRECISION && - rhs.precision <= datatypes::INT64MAXPRECISION) + else if (isTSInt128ByPrecision() && !rhs.isTSInt128ByPrecision()) { VDecimal rhstmp(0, rhs.scale, rhs.precision, (int128_t) rhs.value); @@ -690,8 +682,7 @@ class VDecimal: public TSInt128 else return (datatypes::Decimal::compare(*this, rhstmp) < 0); } - else if (precision <= datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + else if (!isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return (int128_t) value < rhs.s128Value; @@ -709,16 +700,14 @@ class VDecimal: public TSInt128 bool operator>=(const VDecimal& rhs) const { - if (precision > datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + if (isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return s128Value >= rhs.s128Value; else return (datatypes::Decimal::compare(*this, rhs) >= 0); } - else if (precision > datatypes::INT64MAXPRECISION && - rhs.precision <= datatypes::INT64MAXPRECISION) + else if (isTSInt128ByPrecision() && !rhs.isTSInt128ByPrecision()) { VDecimal rhstmp(0, rhs.scale, rhs.precision, (int128_t) rhs.value); @@ -727,8 +716,7 @@ class VDecimal: public TSInt128 else return (datatypes::Decimal::compare(*this, rhstmp) >= 0); } - else if (precision <= datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + else if (!isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return (int128_t) value >= rhs.s128Value; @@ -746,16 +734,14 @@ class VDecimal: public TSInt128 bool operator<=(const VDecimal& rhs) const { - if (precision > datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + if (isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return s128Value <= rhs.s128Value; else return (datatypes::Decimal::compare(*this, rhs) <= 0); } - else if (precision > datatypes::INT64MAXPRECISION && - rhs.precision <= datatypes::INT64MAXPRECISION) + else if (isTSInt128ByPrecision() && !rhs.isTSInt128ByPrecision()) { VDecimal rhstmp(0, rhs.scale, rhs.precision, (int128_t) rhs.value); @@ -764,8 +750,7 @@ class VDecimal: public TSInt128 else return (datatypes::Decimal::compare(*this, rhstmp) <= 0); } - else if (precision <= datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + else if (!isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return (int128_t) value <= rhs.s128Value; @@ -783,16 +768,14 @@ class VDecimal: public TSInt128 bool operator!=(const VDecimal& rhs) const { - if (precision > datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + if (isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return s128Value != rhs.s128Value; else return (datatypes::Decimal::compare(*this, rhs) != 0); } - else if (precision > datatypes::INT64MAXPRECISION && - rhs.precision <= datatypes::INT64MAXPRECISION) + else if (isTSInt128ByPrecision() && !rhs.isTSInt128ByPrecision()) { VDecimal rhstmp(0, rhs.scale, rhs.precision, (int128_t) rhs.value); @@ -801,8 +784,7 @@ class VDecimal: public TSInt128 else return (datatypes::Decimal::compare(*this, rhstmp) != 0); } - else if (precision <= datatypes::INT64MAXPRECISION && - rhs.precision > datatypes::INT64MAXPRECISION) + else if (!isTSInt128ByPrecision() && rhs.isTSInt128ByPrecision()) { if (scale == rhs.scale) return (int128_t) value != rhs.s128Value; diff --git a/dbcon/execplan/functioncolumn.h b/dbcon/execplan/functioncolumn.h index cda618b85..2326840d5 100644 --- a/dbcon/execplan/functioncolumn.h +++ b/dbcon/execplan/functioncolumn.h @@ -250,7 +250,7 @@ public: && fResultType.scale == decimal.scale)) return decimal; - if (LIKELY(fResultType.colWidth == datatypes::MAXDECIMALWIDTH)) + if (LIKELY(fResultType.isWideDecimalType())) { decimal.s128Value = (datatypes::Decimal::isWideDecimalTypeByPrecision(decimal.precision)) ? diff --git a/dbcon/joblist/tupleaggregatestep.cpp b/dbcon/joblist/tupleaggregatestep.cpp index 4a6e39324..db0fc6c8a 100644 --- a/dbcon/joblist/tupleaggregatestep.cpp +++ b/dbcon/joblist/tupleaggregatestep.cpp @@ -751,12 +751,12 @@ void TupleAggregateStep::configDeliveredRowGroup(const JobInfo& jobInfo) { retColCount = jobInfo.returnedColVec.size(); - idbassert(jobInfo.returnedColVec.size() == jobInfo.nonConstCols.size()); + idbassert(jobInfo.returnedColVec.size() == jobInfo.projectionCols.size()); - for (size_t i = 0; i < jobInfo.nonConstCols.size() && + for (size_t i = 0; i < jobInfo.projectionCols.size() && scaleIter != scale.end(); i++) { - const auto& colType = jobInfo.nonConstCols[i]->resultType(); + const auto& colType = jobInfo.projectionCols[i]->resultType(); if (colType.isWideDecimalType()) { diff --git a/dbcon/joblist/tupleunion.cpp b/dbcon/joblist/tupleunion.cpp index 438dc8ef2..8bd445b1e 100644 --- a/dbcon/joblist/tupleunion.cpp +++ b/dbcon/joblist/tupleunion.cpp @@ -1068,7 +1068,7 @@ dec3: /* have to pick a scale to use for the double. using 5.. case CalpontSystemCatalog::DECIMAL: case CalpontSystemCatalog::UDECIMAL: { -dec4: /* have to pick a scale to use for the double. using 5... */ +dec4: /* have to pick a scale to use for the double. using 5... */ uint32_t scale = 5; uint64_t ival = (uint64_t) (double) (val * pow((double) 10, (double) scale)); int diff = out->getScale(i) - scale; @@ -1128,7 +1128,8 @@ dec4: /* have to pick a scale to use for the double. using 5... */ case CalpontSystemCatalog::DECIMAL: case CalpontSystemCatalog::UDECIMAL: { - if (out->getColumnWidth(i) == datatypes::MAXDECIMALWIDTH) + if (datatypes::isWideDecimalType(out->getColTypes()[i], + out->getColumnWidth(i))) { if (out->getScale(i) == scale) out->setInt128Field(isInputWide ? val128 : val, i); diff --git a/primitives/linux-port/column.cpp b/primitives/linux-port/column.cpp index 72a0d747f..bd7b0527e 100644 --- a/primitives/linux-port/column.cpp +++ b/primitives/linux-port/column.cpp @@ -1871,7 +1871,8 @@ boost::shared_ptr parseColumnFilter ret.reset(new ParsedColumnFilter()); ret->columnFilterMode = TWO_ARRAYS; - if (colWidth == datatypes::MAXDECIMALWIDTH) + if (datatypes::isWideDecimalType( + (CalpontSystemCatalog::ColDataType)colType, colWidth)) ret->prestored_argVals128.reset(new int128_t[filterCount]); else ret->prestored_argVals.reset(new int64_t[filterCount]); @@ -2002,7 +2003,8 @@ boost::shared_ptr parseColumnFilter if (convertToSet) { ret->columnFilterMode = UNORDERED_SET; - if (colWidth == datatypes::MAXDECIMALWIDTH) + if (datatypes::isWideDecimalType( + (CalpontSystemCatalog::ColDataType)colType, colWidth)) { ret->prestored_set_128.reset(new prestored_set_t_128()); diff --git a/utils/funcexp/func_abs.cpp b/utils/funcexp/func_abs.cpp index 830a48a6d..10227b4e9 100644 --- a/utils/funcexp/func_abs.cpp +++ b/utils/funcexp/func_abs.cpp @@ -67,7 +67,7 @@ IDB_Decimal Func_abs::getDecimalVal(Row& row, { IDB_Decimal d = parm[0]->data()->getDecimalVal(row, isNull); - if (parm[0]->data()->resultType().colWidth == datatypes::MAXDECIMALWIDTH) + if (parm[0]->data()->resultType().isWideDecimalType()) d.s128Value = (d.s128Value < 0) ? -d.s128Value : d.s128Value; else d.value = llabs(d.value); diff --git a/utils/funcexp/func_cast.cpp b/utils/funcexp/func_cast.cpp index eff0e69ae..928357469 100644 --- a/utils/funcexp/func_cast.cpp +++ b/utils/funcexp/func_cast.cpp @@ -636,7 +636,7 @@ IDB_Decimal Func_cast_date::getDecimalVal(Row& row, { IDB_Decimal decimal; - if (parm[0]->data()->resultType().colWidth == datatypes::MAXDECIMALWIDTH) + if (parm[0]->data()->resultType().isWideDecimalType()) decimal.s128Value = Func_cast_date::getDatetimeIntVal(row, parm, isNull, @@ -945,7 +945,7 @@ IDB_Decimal Func_cast_datetime::getDecimalVal(Row& row, { IDB_Decimal decimal; - if (parm[0]->data()->resultType().colWidth == datatypes::MAXDECIMALWIDTH) + if (parm[0]->data()->resultType().isWideDecimalType()) decimal.s128Value = Func_cast_datetime::getDatetimeIntVal(row, parm, isNull, @@ -1204,7 +1204,7 @@ int64_t Func_cast_decimal::getIntVal(Row& row, isNull, operationColType); - if (decimal.precision > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { int128_t scaleDivisor; @@ -1228,7 +1228,7 @@ string Func_cast_decimal::getStrVal(Row& row, parm, isNull, operationColType); - if (operationColType.colWidth == datatypes::MAXDECIMALWIDTH) + if (operationColType.isWideDecimalType()) return decimal.toString(true); else return decimal.toString(); @@ -1258,7 +1258,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, case execplan::CalpontSystemCatalog::TINYINT: case execplan::CalpontSystemCatalog::SMALLINT: { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { bool dummy = false; char *ep = NULL; @@ -1307,7 +1307,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, case execplan::CalpontSystemCatalog::UTINYINT: case execplan::CalpontSystemCatalog::USMALLINT: { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { bool dummy = false; char *ep = NULL; @@ -1361,7 +1361,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, case execplan::CalpontSystemCatalog::FLOAT: case execplan::CalpontSystemCatalog::UFLOAT: { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { bool dummy = false; char *ep = NULL; @@ -1411,7 +1411,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, case execplan::CalpontSystemCatalog::LONGDOUBLE: { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { bool dummy = false; char *ep = NULL; @@ -1462,7 +1462,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, case execplan::CalpontSystemCatalog::DECIMAL: case execplan::CalpontSystemCatalog::UDECIMAL: { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { bool dummy = false; char *ep = NULL; @@ -1473,7 +1473,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, int128_t scaleDivisor; datatypes::getScaleDivisor(scaleDivisor, abs(decimals - decimal.scale)); - if (decimal.precision <= datatypes::INT64MAXPRECISION) + if (!decimal.isTSInt128ByPrecision()) decimal.s128Value = decimal.value; decimal.precision = max_length; @@ -1501,7 +1501,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, decimal = parm[0]->data()->getDecimalVal(row, isNull); - if (decimal.precision > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { if ( decimal.s128Value > (int128_t) max_number_decimal ) decimal.value = max_number_decimal; @@ -1559,7 +1559,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, { if (*s == 'e' || *s == 'E') { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { bool dummy = false; char *ep = NULL; @@ -1656,7 +1656,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, } } - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) { bool dummy = false; char *ep = NULL; @@ -1781,7 +1781,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, if (!isNull) { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) decimal.s128Value = x; else decimal.value = x; @@ -1804,7 +1804,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, if (!isNull) { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) decimal.s128Value = x; else decimal.value = x; @@ -1827,7 +1827,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, if (!isNull) { - if (max_length > datatypes::INT64MAXPRECISION) + if (max_length > decimal.isTSInt128ByPrecision()) decimal.s128Value = x; else decimal.value = x; @@ -1850,7 +1850,7 @@ IDB_Decimal Func_cast_decimal::getDecimalVal(Row& row, if (!isNull) { - if (max_length > datatypes::INT64MAXPRECISION) + if (decimal.isTSInt128ByPrecision()) decimal.s128Value = x; else decimal.value = x; @@ -1882,8 +1882,8 @@ double Func_cast_decimal::getDoubleVal(Row& row, operationColType); // WIP MCOL-641 This could deliver wrong result b/c wide DECIMAL might have - // p <= INT64MAXPRECISION - if (decimal.precision > datatypes::INT64MAXPRECISION) + // p <= INT64MAXPRECISION || p > INT128MAXPRECISION + if (decimal.isTSInt128ByPrecision()) { return static_cast(decimal); } diff --git a/utils/funcexp/func_inet_aton.cpp b/utils/funcexp/func_inet_aton.cpp index 2f0e5295e..7ac21cd77 100644 --- a/utils/funcexp/func_inet_aton.cpp +++ b/utils/funcexp/func_inet_aton.cpp @@ -158,7 +158,7 @@ execplan::IDB_Decimal Func_inet_aton::getDecimalVal(rowgroup::Row& row, const std::string& sValue = fp[0]->data()->getStrVal(row, isNull); - if (colType.precision <= datatypes::INT64MAXPRECISION) + if (!datatypes::Decimal::isWideDecimalTypeByPrecision(colType.precision)) { if (!isNull) { diff --git a/utils/funcexp/func_mod.cpp b/utils/funcexp/func_mod.cpp index a2f6efcbe..f66bd42e7 100644 --- a/utils/funcexp/func_mod.cpp +++ b/utils/funcexp/func_mod.cpp @@ -66,8 +66,8 @@ IDB_Decimal Func_mod::getDecimalVal(Row& row, return retValue; } - if (parm[0]->data()->resultType().colWidth == datatypes::MAXDECIMALWIDTH || - parm[1]->data()->resultType().colWidth == datatypes::MAXDECIMALWIDTH) + if (parm[0]->data()->resultType().isWideDecimalType() || + parm[1]->data()->resultType().isWideDecimalType()) { IDB_Decimal div = parm[1]->data()->getDecimalVal(row, isNull); diff --git a/utils/funcexp/func_monthname.cpp b/utils/funcexp/func_monthname.cpp index 7d41e3ba4..8bc35a0c8 100644 --- a/utils/funcexp/func_monthname.cpp +++ b/utils/funcexp/func_monthname.cpp @@ -199,7 +199,7 @@ execplan::IDB_Decimal Func_monthname::getDecimalVal(rowgroup::Row& row, { IDB_Decimal d; - if (fp[0]->data()->resultType().colWidth == datatypes::MAXDECIMALWIDTH) + if (fp[0]->data()->resultType().isWideDecimalType()) d.s128Value = getIntVal(row, fp, isNull, op_ct); else d.value = getIntVal(row, fp, isNull, op_ct); diff --git a/utils/funcexp/func_sec_to_time.cpp b/utils/funcexp/func_sec_to_time.cpp index ff808a975..f4a9af8ad 100644 --- a/utils/funcexp/func_sec_to_time.cpp +++ b/utils/funcexp/func_sec_to_time.cpp @@ -284,7 +284,7 @@ execplan::IDB_Decimal Func_sec_to_time::getDecimalVal(rowgroup::Row& row, tmpVal = strtoll(str, &ep, 10); } - if (parm[0]->data()->resultType().colWidth == datatypes::MAXDECIMALWIDTH) + if (parm[0]->data()->resultType().isWideDecimalType()) d.s128Value = tmpVal; else d.value = tmpVal; diff --git a/utils/windowfunction/windowfunctiontype.cpp b/utils/windowfunction/windowfunctiontype.cpp index d5bb92047..03afa5729 100644 --- a/utils/windowfunction/windowfunctiontype.cpp +++ b/utils/windowfunction/windowfunctiontype.cpp @@ -522,8 +522,7 @@ void WindowFunctionType::implicit2T(uint64_t i, T& t, int s) } else if (width == datatypes::MAXDECIMALWIDTH) { - datatypes::TSInt128::assignPtrPtr(&t, - fRow.getBinaryField(i)); + datatypes::TSInt128::assignPtrPtr(&t, fRow.getBinaryField(i)); } break; }