From 1f4a7817045677a666ace83fb73539fa1bb825a2 Mon Sep 17 00:00:00 2001 From: Gagan Goel Date: Thu, 22 Oct 2020 12:16:48 -0400 Subject: [PATCH] MCOL-641 Fixes for arithmetic operations. 1. Perform type promotion to wide decimal if the result of an arithmetic operation has a precision > 18. 2. Only set the decimal width of an arithmetic operation to wide if both the LHS and RHS of the operation are decimal types. --- datatypes/mcs_decimal.h | 15 ++++++ dbcon/joblist/tupleaggregatestep.cpp | 51 ++++++++++++-------- dbcon/mysql/ha_mcs_execplan.cpp | 71 +++++++++++----------------- 3 files changed, 72 insertions(+), 65 deletions(-) diff --git a/datatypes/mcs_decimal.h b/datatypes/mcs_decimal.h index 329487743..178dce1ba 100644 --- a/datatypes/mcs_decimal.h +++ b/datatypes/mcs_decimal.h @@ -470,6 +470,21 @@ class Decimal scale += (scaleAvailable >= MAXSCALEINC4AVG) ? MAXSCALEINC4AVG : scaleAvailable; precision += (precisionAvailable >= MAXSCALEINC4AVG) ? MAXSCALEINC4AVG : precisionAvailable; } + + /** + @brief Returns true if all arguments have a DECIMAL/UDECIMAL type + */ + static inline bool isDecimalOperands(const ColDataTypeAlias resultDataType, + const ColDataTypeAlias leftColDataType, + const ColDataTypeAlias rightColDataType) + { + return ((resultDataType == execplan::CalpontSystemCatalog::DECIMAL || + resultDataType == execplan::CalpontSystemCatalog::UDECIMAL) && + (leftColDataType == execplan::CalpontSystemCatalog::DECIMAL || + leftColDataType == execplan::CalpontSystemCatalog::UDECIMAL) && + (rightColDataType == execplan::CalpontSystemCatalog::DECIMAL || + rightColDataType == execplan::CalpontSystemCatalog::UDECIMAL)); + } }; /** diff --git a/dbcon/joblist/tupleaggregatestep.cpp b/dbcon/joblist/tupleaggregatestep.cpp index 50767ab36..90d9f9148 100644 --- a/dbcon/joblist/tupleaggregatestep.cpp +++ b/dbcon/joblist/tupleaggregatestep.cpp @@ -343,12 +343,8 @@ namespace joblist void wideDecimalOrLongDouble(const uint64_t colProj, const CalpontSystemCatalog::ColDataType type, const vector& precisionProj, - const vector& oidsProj, - const uint32_t aggKey, const vector& scaleProj, const vector& width, - vector& oidsAgg, - vector& keysAgg, vector& typeAgg, vector& scaleAgg, vector& precisionAgg, @@ -358,8 +354,6 @@ void wideDecimalOrLongDouble(const uint64_t colProj, || type == CalpontSystemCatalog::UDECIMAL) && datatypes::Decimal::isWideDecimalType(precisionProj[colProj])) { - oidsAgg.push_back(oidsProj[colProj]); - keysAgg.push_back(aggKey); typeAgg.push_back(type); scaleAgg.push_back(scaleProj[colProj]); precisionAgg.push_back(precisionProj[colProj]); @@ -367,8 +361,6 @@ void wideDecimalOrLongDouble(const uint64_t colProj, } else { - oidsAgg.push_back(oidsProj[colProj]); - keysAgg.push_back(aggKey); typeAgg.push_back(CalpontSystemCatalog::LONGDOUBLE); scaleAgg.push_back(0); precisionAgg.push_back(-1); @@ -758,29 +750,38 @@ void TupleAggregateStep::configDeliveredRowGroup(const JobInfo& jobInfo) if (jobInfo.havingStep) { retColCount = jobInfo.returnedColVec.size(); + idbassert(jobInfo.returnedColVec.size() == jobInfo.nonConstCols.size()); - for (auto& rc : jobInfo.nonConstCols) + + for (size_t i = 0; i < jobInfo.nonConstCols.size() && + scaleIter != scale.end(); i++) { - auto& colType = rc->resultType(); + const auto& colType = jobInfo.nonConstCols[i]->resultType(); + if (datatypes::Decimal::isWideDecimalType(colType)) { *scaleIter = colType.scale; *precisionIter = colType.precision; } + scaleIter++; precisionIter++; } } else { retColCount = jobInfo.nonConstDelCols.size(); - for (auto& rc : jobInfo.nonConstDelCols) + + for (size_t i = 0; i < jobInfo.nonConstDelCols.size() && + scaleIter != scale.end(); i++) { - auto& colType = rc->resultType(); + const auto& colType = jobInfo.nonConstDelCols[i]->resultType(); + if (datatypes::Decimal::isWideDecimalType(colType)) { *scaleIter = colType.scale; *precisionIter = colType.precision; } + scaleIter++; precisionIter++; } } @@ -1356,10 +1357,13 @@ void TupleAggregateStep::prep1PhaseAggregate( cerr << "prep1PhaseAggregate: " << emsg << endl; throw IDBExcept(emsg, ERR_AGGREGATE_TYPE_NOT_SUPPORT); } + wideDecimalOrLongDouble(colProj, typeProj[colProj], - precisionProj, oidsProj, key, scaleProj, width, - oidsAgg, keysAgg, typeAgg, scaleAgg, - precisionAgg, widthAgg); + precisionProj, scaleProj, width, + typeAgg, scaleAgg, precisionAgg, widthAgg); + + oidsAgg.push_back(oidsProj[colProj]); + keysAgg.push_back(key); csNumAgg.push_back(csNumProj[colProj]); } break; @@ -3181,10 +3185,13 @@ void TupleAggregateStep::prep2PhasesAggregate( cerr << "prep2PhasesAggregate: " << emsg << endl; throw IDBExcept(emsg, ERR_AGGREGATE_TYPE_NOT_SUPPORT); } + wideDecimalOrLongDouble(colProj, typeProj[colProj], - precisionProj, oidsProj, aggKey, scaleProj, width, - oidsAggPm, keysAggPm, typeAggPm, scaleAggPm, - precisionAggPm, widthAggPm); + precisionProj, scaleProj, width, + typeAggPm, scaleAggPm, precisionAggPm, widthAggPm); + + oidsAggPm.push_back(oidsProj[colProj]); + keysAggPm.push_back(aggKey); csNumAggPm.push_back(8); colAggPm++; } @@ -3469,9 +3476,11 @@ void TupleAggregateStep::prep2PhasesAggregate( if (aggOp == ROWAGG_SUM) { wideDecimalOrLongDouble(colPm, typeProj[colPm], - precisionProj, oidsProj, retKey, scaleProj, widthAggPm, - oidsAggUm, keysAggUm, typeAggUm, scaleAggUm, - precisionAggUm, widthAggUm); + precisionProj, scaleProj, widthAggPm, + typeAggUm, scaleAggUm, precisionAggUm, widthAggUm); + + oidsAggUm.push_back(oidsProj[colPm]); + keysAggUm.push_back(retKey); csNumAggUm.push_back(8); } else diff --git a/dbcon/mysql/ha_mcs_execplan.cpp b/dbcon/mysql/ha_mcs_execplan.cpp index aa5dd5184..666bb467a 100755 --- a/dbcon/mysql/ha_mcs_execplan.cpp +++ b/dbcon/mysql/ha_mcs_execplan.cpp @@ -3602,68 +3602,51 @@ ArithmeticColumn* buildArithmeticColumn( // @bug5715. Use InfiniDB adjusted coltype for result type. // decimal arithmetic operation gives double result when the session variable is set. - CalpontSystemCatalog::ColType mysql_type = colType_MysqlToIDB(item); + CalpontSystemCatalog::ColType mysqlType = colType_MysqlToIDB(item); - if (mysql_type.colDataType == CalpontSystemCatalog::DECIMAL || - mysql_type.colDataType == CalpontSystemCatalog::UDECIMAL) + const CalpontSystemCatalog::ColType& leftColType = pt->left()->data()->resultType(); + const CalpontSystemCatalog::ColType& rightColType = pt->right()->data()->resultType(); + + // Only tinker with the type if all columns involved are decimal + if (datatypes::Decimal::isDecimalOperands(mysqlType.colDataType, + leftColType.colDataType, rightColType.colDataType)) { - int32_t leftColWidth = pt->left()->data()->resultType().colWidth; - int32_t rightColWidth = pt->right()->data()->resultType().colWidth; + int32_t leftColWidth = leftColType.colWidth; + int32_t rightColWidth = rightColType.colWidth; - // Revert back to legacy values of scale and precision - // if the 2 columns involved in the expression are not wide - if (leftColWidth <= utils::MAXLEGACYWIDTH && - rightColWidth <= utils::MAXLEGACYWIDTH) + if (leftColWidth == datatypes::MAXDECIMALWIDTH || + rightColWidth == datatypes::MAXDECIMALWIDTH) { - Item_decimal* idp = (Item_decimal*)item; + mysqlType.colWidth = datatypes::MAXDECIMALWIDTH; - mysql_type.colWidth = 8; + string funcName = item->func_name(); - unsigned int precision = idp->max_length; - unsigned int scale = idp->decimals; + int32_t scale1 = leftColType.scale; + int32_t scale2 = rightColType.scale; - datatypes::Decimal::setDecimalScalePrecisionLegacy(mysql_type, - precision, scale); - } - else - { - - if (leftColWidth == datatypes::MAXDECIMALWIDTH || - rightColWidth == datatypes::MAXDECIMALWIDTH) - mysql_type.colWidth = datatypes::MAXDECIMALWIDTH; - - if (mysql_type.colWidth == datatypes::MAXDECIMALWIDTH) + if (funcName == "/" && + (mysqlType.scale - (scale1 - scale2)) > datatypes::INT128MAXPRECISION) { - string funcName = item->func_name(); + Item_decimal* idp = (Item_decimal*)item; - int32_t scale1 = pt->left()->data()->resultType().scale; - int32_t scale2 = pt->right()->data()->resultType().scale; + unsigned int precision = idp->decimal_precision(); + unsigned int scale = idp->decimal_scale(); - if (funcName == "/" && - (mysql_type.scale - (scale1 - scale2)) > - datatypes::INT128MAXPRECISION) - { - Item_decimal* idp = (Item_decimal*)item; + datatypes::Decimal::setDecimalScalePrecisionHeuristic(mysqlType, precision, scale); - unsigned int precision = idp->decimal_precision(); - unsigned int scale = idp->decimal_scale(); + if (mysqlType.scale < scale1) + mysqlType.scale = scale1; - datatypes::Decimal::setDecimalScalePrecisionHeuristic(mysql_type, precision, scale); - - if (mysql_type.scale < scale1) - mysql_type.scale = scale1; - - if (mysql_type.precision < mysql_type.scale) - mysql_type.precision = mysql_type.scale; - } + if (mysqlType.precision < mysqlType.scale) + mysqlType.precision = mysqlType.scale; } } } if (get_double_for_decimal_math(current_thd) == true) - aop->adjustResultType(mysql_type); + aop->adjustResultType(mysqlType); else - aop->resultType(mysql_type); + aop->resultType(mysqlType); // adjust decimal result type according to internalDecimalScale if (gwi.internalDecimalScale >= 0 && aop->resultType().colDataType == CalpontSystemCatalog::DECIMAL)