1
0
mirror of https://github.com/mariadb-corporation/mariadb-columnstore-engine.git synced 2025-07-30 19:23:07 +03:00

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.
This commit is contained in:
Gagan Goel
2020-11-24 20:15:33 -05:00
parent c707f72f0c
commit c5d4a918ee
12 changed files with 55 additions and 71 deletions

View File

@ -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<VDecimal&>(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;

View File

@ -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)) ?

View File

@ -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())
{

View File

@ -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);

View File

@ -1871,7 +1871,8 @@ boost::shared_ptr<ParsedColumnFilter> 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<ParsedColumnFilter> 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());

View File

@ -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);

View File

@ -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<double>(decimal);
}

View File

@ -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)
{

View File

@ -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);

View File

@ -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);

View File

@ -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;

View File

@ -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<int128_t>(i));
datatypes::TSInt128::assignPtrPtr(&t, fRow.getBinaryField<int128_t>(i));
}
break;
}