Skip to content

Commit 67b4637

Browse files
peterenescufacebook-github-bot
authored andcommitted
fix: Integer overflow in range filter boundary conversion (prestodb#27600)
Summary: Guard against integer overflow when converting exclusive bounds to inclusive bounds for BigintRange, HugeintRange, and TimestampRange filters in the OSS Hive connector. When the boundary value is at the type limit, return AlwaysFalse or IsNull instead of overflowing. This mirrors the fix applied to the Prism connector and Velox expression filters. Differential Revision: D101072577
1 parent 94a11f7 commit 67b4637

File tree

1 file changed

+56
-4
lines changed

1 file changed

+56
-4
lines changed

presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.cpp

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ bool toBoolean(
8787
return variant.value<bool>();
8888
}
8989

90-
std::unique_ptr<common::BigintRange> bigintRangeToFilter(
90+
std::unique_ptr<common::Filter> bigintRangeToFilter(
9191
const protocol::Range& range,
9292
bool nullAllowed,
9393
const VeloxExprConverter& exprConverter,
@@ -96,6 +96,12 @@ std::unique_ptr<common::BigintRange> bigintRangeToFilter(
9696
auto low = lowUnbounded ? std::numeric_limits<int64_t>::min()
9797
: toInt64(range.low.valueBlock, exprConverter, type);
9898
if (!lowUnbounded && range.low.bound == protocol::Bound::ABOVE) {
99+
if (low == std::numeric_limits<int64_t>::max()) {
100+
if (nullAllowed) {
101+
return std::make_unique<common::IsNull>();
102+
}
103+
return std::make_unique<common::AlwaysFalse>();
104+
}
99105
low++;
100106
}
101107

@@ -104,12 +110,18 @@ std::unique_ptr<common::BigintRange> bigintRangeToFilter(
104110
? std::numeric_limits<int64_t>::max()
105111
: toInt64(range.high.valueBlock, exprConverter, type);
106112
if (!highUnbounded && range.high.bound == protocol::Bound::BELOW) {
113+
if (high == std::numeric_limits<int64_t>::min()) {
114+
if (nullAllowed) {
115+
return std::make_unique<common::IsNull>();
116+
}
117+
return std::make_unique<common::AlwaysFalse>();
118+
}
107119
high--;
108120
}
109121
return std::make_unique<common::BigintRange>(low, high, nullAllowed);
110122
}
111123

112-
std::unique_ptr<common::HugeintRange> hugeintRangeToFilter(
124+
std::unique_ptr<common::Filter> hugeintRangeToFilter(
113125
const protocol::Range& range,
114126
bool nullAllowed,
115127
const VeloxExprConverter& exprConverter,
@@ -118,6 +130,12 @@ std::unique_ptr<common::HugeintRange> hugeintRangeToFilter(
118130
auto low = lowUnbounded ? std::numeric_limits<int128_t>::min()
119131
: toInt128(range.low.valueBlock, exprConverter, type);
120132
if (!lowUnbounded && range.low.bound == protocol::Bound::ABOVE) {
133+
if (low == std::numeric_limits<int128_t>::max()) {
134+
if (nullAllowed) {
135+
return std::make_unique<common::IsNull>();
136+
}
137+
return std::make_unique<common::AlwaysFalse>();
138+
}
121139
low++;
122140
}
123141

@@ -126,12 +144,18 @@ std::unique_ptr<common::HugeintRange> hugeintRangeToFilter(
126144
? std::numeric_limits<int128_t>::max()
127145
: toInt128(range.high.valueBlock, exprConverter, type);
128146
if (!highUnbounded && range.high.bound == protocol::Bound::BELOW) {
147+
if (high == std::numeric_limits<int128_t>::min()) {
148+
if (nullAllowed) {
149+
return std::make_unique<common::IsNull>();
150+
}
151+
return std::make_unique<common::AlwaysFalse>();
152+
}
129153
high--;
130154
}
131155
return std::make_unique<common::HugeintRange>(low, high, nullAllowed);
132156
}
133157

134-
std::unique_ptr<common::TimestampRange> timestampRangeToFilter(
158+
std::unique_ptr<common::Filter> timestampRangeToFilter(
135159
const protocol::Range& range,
136160
bool nullAllowed,
137161
const VeloxExprConverter& exprConverter,
@@ -141,6 +165,12 @@ std::unique_ptr<common::TimestampRange> timestampRangeToFilter(
141165
? std::numeric_limits<Timestamp>::min()
142166
: toTimestamp(range.low.valueBlock, exprConverter, type);
143167
if (!lowUnbounded && range.low.bound == protocol::Bound::ABOVE) {
168+
if (low == std::numeric_limits<Timestamp>::max()) {
169+
if (nullAllowed) {
170+
return std::make_unique<common::IsNull>();
171+
}
172+
return std::make_unique<common::AlwaysFalse>();
173+
}
144174
++low;
145175
}
146176

@@ -149,6 +179,12 @@ std::unique_ptr<common::TimestampRange> timestampRangeToFilter(
149179
? std::numeric_limits<Timestamp>::max()
150180
: toTimestamp(range.high.valueBlock, exprConverter, type);
151181
if (!highUnbounded && range.high.bound == protocol::Bound::BELOW) {
182+
if (high == std::numeric_limits<Timestamp>::min()) {
183+
if (nullAllowed) {
184+
return std::make_unique<common::IsNull>();
185+
}
186+
return std::make_unique<common::AlwaysFalse>();
187+
}
152188
--high;
153189
}
154190
return std::make_unique<common::TimestampRange>(low, high, nullAllowed);
@@ -635,8 +671,24 @@ std::unique_ptr<common::Filter> toFilter(
635671
std::vector<std::unique_ptr<common::BigintRange>> bigintFilters;
636672
bigintFilters.reserve(ranges.size());
637673
for (const auto& range : ranges) {
674+
std::unique_ptr<common::Filter> filter =
675+
bigintRangeToFilter(range, nullAllowed, exprConverter, type);
676+
VELOX_CHECK_NOT_NULL(filter);
677+
if (filter->kind() == common::FilterKind::kAlwaysFalse ||
678+
filter->kind() == common::FilterKind::kIsNull) {
679+
continue;
680+
}
638681
bigintFilters.emplace_back(
639-
bigintRangeToFilter(range, nullAllowed, exprConverter, type));
682+
dynamic_cast<common::BigintRange*>(filter.release()));
683+
}
684+
if (bigintFilters.empty()) {
685+
if (nullAllowed) {
686+
return std::make_unique<common::IsNull>();
687+
}
688+
return std::make_unique<common::AlwaysFalse>();
689+
}
690+
if (bigintFilters.size() == 1) {
691+
return std::move(bigintFilters[0]);
640692
}
641693
return combineIntegerRanges(bigintFilters, nullAllowed);
642694
}

0 commit comments

Comments
 (0)