Skip to content

Commit

Permalink
Fix(UBSan): convertion NaN to unsigned int in Reservoir Sampler
Browse files Browse the repository at this point in the history
See #42978
  • Loading branch information
devcrafter committed Nov 9, 2022
1 parent dce0994 commit 31c3f87
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 24 deletions.
27 changes: 6 additions & 21 deletions src/AggregateFunctions/ReservoirSampler.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,6 @@ namespace ReservoirSamplerOnEmpty
};
}

template <typename ResultType, bool is_float>
struct NanLikeValueConstructor
{
static ResultType getValue()
{
return std::numeric_limits<ResultType>::quiet_NaN();
}
};
template <typename ResultType>
struct NanLikeValueConstructor<ResultType, false>
{
static ResultType getValue()
{
return ResultType();
}
};

template <typename T, ReservoirSamplerOnEmpty::Enum OnEmpty = ReservoirSamplerOnEmpty::THROW, typename Comparer = std::less<T>>
class ReservoirSampler
{
Expand Down Expand Up @@ -123,9 +106,11 @@ class ReservoirSampler
{
if (samples.empty())
{
if (DB::is_decimal<T>)
[[maybe_unused]] auto nan = onEmpty<T>();
if constexpr (DB::is_decimal<T>)
return 0;
return onEmpty<double>();
else
return nan;
}
sortIfNeeded();

Expand Down Expand Up @@ -262,9 +247,9 @@ class ReservoirSampler
template <typename ResultType>
ResultType onEmpty() const
{
if (OnEmpty == ReservoirSamplerOnEmpty::THROW)
if constexpr (OnEmpty == ReservoirSamplerOnEmpty::THROW)
throw DB::Exception(DB::ErrorCodes::LOGICAL_ERROR, "Quantile of empty ReservoirSampler");
else
return NanLikeValueConstructor<ResultType, std::is_floating_point_v<ResultType>>::getValue();
return std::numeric_limits<ResultType>::quiet_NaN();
}
};
12 changes: 9 additions & 3 deletions src/AggregateFunctions/ReservoirSamplerDeterministic.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,13 @@ class ReservoirSamplerDeterministic
double quantileInterpolated(double level)
{
if (samples.empty())
return onEmpty<double>();
{
[[maybe_unused]] auto nan = onEmpty<T>();
if constexpr (DB::is_decimal<T>)
return 0;
else
return nan;
}

sortIfNeeded();

Expand Down Expand Up @@ -259,9 +265,9 @@ class ReservoirSamplerDeterministic
template <typename ResultType>
ResultType onEmpty() const
{
if (OnEmpty == ReservoirSamplerDeterministicOnEmpty::THROW)
if constexpr (OnEmpty == ReservoirSamplerDeterministicOnEmpty::THROW)
throw DB::Exception(DB::ErrorCodes::LOGICAL_ERROR, "Quantile of empty ReservoirSamplerDeterministic");
else
return NanLikeValueConstructor<ResultType, std::is_floating_point_v<ResultType>>::getValue();
return std::numeric_limits<ResultType>::quiet_NaN();
}
};

0 comments on commit 31c3f87

Please sign in to comment.