PR fixes:

* Fixed precision calculation in DataTypeDecimalBase c-tor
* Fixed max precision calculation in getLeastSupertype
* Fixed reading past end of vector in FunctionsConversion with extractToDecimalScale
* More verbose comments on FunctionArgumentTypeValidator and validateFunctionArgumentTypes
* style and other minor fixes.
This commit is contained in:
Vasily Nemkov 2019-12-11 17:06:34 +03:00
parent c781908e6d
commit 514847609d
8 changed files with 40 additions and 22 deletions

View File

@ -24,7 +24,7 @@ namespace DB
{
DataTypeDateTime64::DataTypeDateTime64(UInt32 scale_, const std::string & time_zone_name)
: DataTypeDecimalBase<DateTime64>(DecimalUtils::maxPrecision<DateTime64>() - scale_, scale_),
: DataTypeDecimalBase<DateTime64>(DecimalUtils::maxPrecision<DateTime64>(), scale_),
TimezoneMixin(time_zone_name)
{
}

View File

@ -230,20 +230,28 @@ DataTypePtr getLeastSupertype(const DataTypes & types)
// big enough to hold max whole-value of any type from `types`.
// That would sacrifice scale when comparing DateTime64 of different scales.
UInt32 max_datetime64_precision = 0;
UInt32 max_datetime64_whole_precision = 0;
for (const auto & t : types)
{
if (auto dt64 = typeid_cast<const DataTypeDateTime64 *>(t.get()))
{
const auto precision = dt64->getPrecision();
max_datetime64_precision = std::max(precision, max_datetime64_precision);
const auto whole_precision = dt64->getPrecision() - dt64->getScale();
max_datetime64_whole_precision = std::max(whole_precision, max_datetime64_whole_precision);
}
}
const UInt32 least_decimal_precision = have_datetime ? leastDecimalPrecisionFor(TypeIndex::UInt32) : have_date ? leastDecimalPrecisionFor(TypeIndex::UInt16) : 0;
max_datetime64_precision = std::max(least_decimal_precision, max_datetime64_precision);
UInt32 least_decimal_precision = 0;
if (have_datetime)
{
least_decimal_precision = leastDecimalPrecisionFor(TypeIndex::UInt32);
}
else if (have_date)
{
least_decimal_precision = leastDecimalPrecisionFor(TypeIndex::UInt16);
}
max_datetime64_whole_precision = std::max(least_decimal_precision, max_datetime64_whole_precision);
const UInt32 scale = DataTypeDateTime64::maxPrecision() - max_datetime64_precision;
const UInt32 scale = DataTypeDateTime64::maxPrecision() - max_datetime64_whole_precision;
return std::make_shared<DataTypeDateTime64>(scale);
}
}

View File

@ -387,8 +387,6 @@ public:
return resolveReturnType<DataTypeDateTime64>(arguments);
default:
{
// TODO (vnemkov): quick and dirty way to check, remove before merging.
assert(false);
throw Exception("Invalid type of 1st argument of function " + getName() + ": "
+ arguments[0].type->getName() + ", expected: Date, DateTime or DateTime64.",
ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT);

View File

@ -90,6 +90,7 @@ void validateArgumentType(const IFunction & func, const DataTypes & arguments,
size_t argument_index, bool (* validator_func)(const IDataType &),
const char * expected_type_description);
// Simple validator that is used in conjunction with validateFunctionArgumentTypes() to check if function arguments are as expected.
struct FunctionArgumentTypeValidator
{
bool (* validator_func)(const IDataType &);
@ -99,8 +100,20 @@ struct FunctionArgumentTypeValidator
using FunctionArgumentTypeValidators = std::vector<FunctionArgumentTypeValidator>;
/** Validate that function arguments match specification.
* first, check that mandatory args present and have valid type.
* second, check optional arguents types, skipping ones that are missing.
*
* Designed to simplify argument validation
* for functions with variable arguments (e.g. depending on result type or other trait).
* first, checks that mandatory args present and have valid type.
* second, checks optional arguents types, skipping ones that are missing.
*
* Please note that if you have several optional arguments, like f([a, b, c]),
* only these calls are considered valid:
* f(a)
* f(a, b)
* f(a, b, c)
*
* But NOT these: f(a, c), f(b, c)
* In other words you can't skip
*
* If any mandatory arg is missing, throw an exception, with explicit description of expected arguments.
*/

View File

@ -11,7 +11,6 @@
#include <DataTypes/DataTypeArray.h>
#include <DataTypes/DataTypeDate.h>
#include <DataTypes/DataTypeDateTime.h>
#include <DataTypes/DataTypeDateTime64.h>
#include <DataTypes/DataTypeUUID.h>
#include <DataTypes/DataTypeTuple.h>
#include <Columns/ColumnsNumber.h>

View File

@ -1158,7 +1158,9 @@ public:
}
else if constexpr (std::is_same_v<ToDataType, DataTypeDateTime64>)
{
UInt64 scale = extractToDecimalScale(arguments[1]);
UInt64 scale = DataTypeDateTime64::default_scale;
if (arguments.size() > 0)
scale = extractToDecimalScale(arguments[1]);
const auto timezone = extractTimeZoneNameFromFunctionArguments(arguments, 2, 0);
res = std::make_shared<DataTypeDateTime64>(scale, timezone);
}

View File

@ -6,7 +6,6 @@
#include <DataTypes/DataTypeString.h>
#include <DataTypes/DataTypeDate.h>
#include <DataTypes/DataTypeDateTime.h>
#include <DataTypes/DataTypeDateTime64.h>
#include <DataTypes/DataTypeTuple.h>
#include <DataTypes/DataTypeUUID.h>

View File

@ -26,7 +26,6 @@
#include <DataTypes/DataTypeString.h>
#include <DataTypes/DataTypeDate.h>
#include <DataTypes/DataTypeDateTime.h>
#include <DataTypes/DataTypeDateTime64.h>
#include <DataTypes/DataTypeArray.h>
#include <DataTypes/DataTypeFixedString.h>
#include <DataTypes/DataTypeEnum.h>