Skip to content

Commit 7214fa1

Browse files
committed
Throw on reversed range in AddRange for consistency
A reversed range (start > end) indicates a bug in the caller and should fail fast with std::invalid_argument rather than silently succeed. An empty range (start == end) remains a valid no-op. This makes AddRange behavior consistent across the Java, C++, and Rust Iceberg implementations.
1 parent ab7e189 commit 7214fa1

File tree

3 files changed

+16
-4
lines changed

3 files changed

+16
-4
lines changed

src/iceberg/deletes/roaring_position_bitmap.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <cstring>
2323
#include <exception>
2424
#include <limits>
25+
#include <stdexcept>
2526
#include <utility>
2627
#include <vector>
2728

@@ -128,7 +129,12 @@ void RoaringPositionBitmap::Add(int64_t pos) {
128129
}
129130

130131
void RoaringPositionBitmap::AddRange(int64_t pos_start, int64_t pos_end) {
131-
if (pos_start >= pos_end) {
132+
if (pos_start > pos_end) {
133+
throw std::invalid_argument(
134+
"AddRange requires pos_start <= pos_end, got [" + std::to_string(pos_start) +
135+
", " + std::to_string(pos_end) + ")");
136+
}
137+
if (pos_start == pos_end) {
132138
return;
133139
}
134140
pos_start = std::max(pos_start, int64_t{0});

src/iceberg/deletes/roaring_position_bitmap.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ class ICEBERG_EXPORT RoaringPositionBitmap {
6767
/// \brief Sets a range of positions [pos_start, pos_end).
6868
/// \param pos_start the start of the range (inclusive), clamped to 0
6969
/// \param pos_end the end of the range (exclusive), clamped to kMaxPosition + 1
70-
/// \note If pos_start >= pos_end after clamping, this method does nothing.
70+
/// \throws std::invalid_argument if pos_start > pos_end
71+
/// \note If pos_start == pos_end, this method does nothing.
7172
/// Positions outside [0, kMaxPosition] are silently ignored.
7273
void AddRange(int64_t pos_start, int64_t pos_end);
7374

src/iceberg/test/roaring_position_bitmap_test.cc

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <fstream>
2626
#include <random>
2727
#include <set>
28+
#include <stdexcept>
2829
#include <string>
2930
#include <vector>
3031

@@ -203,8 +204,7 @@ TEST_P(RoaringPositionBitmapAddRangeNoOpTest, IsNoOp) {
203204

204205
INSTANTIATE_TEST_SUITE_P(
205206
AddRangeNoOpScenarios, RoaringPositionBitmapAddRangeNoOpTest,
206-
::testing::Values(AddRangeNoOpParams{.name = "reversed", .start = 100, .end = 50},
207-
AddRangeNoOpParams{.name = "equal", .start = 100, .end = 100},
207+
::testing::Values(AddRangeNoOpParams{.name = "equal", .start = 100, .end = 100},
208208
AddRangeNoOpParams{.name = "zero_length_at_zero",
209209
.start = 0,
210210
.end = 0},
@@ -215,6 +215,11 @@ INSTANTIATE_TEST_SUITE_P(
215215
return info.param.name;
216216
});
217217

218+
TEST(RoaringPositionBitmapTest, TestAddRangeReversedThrows) {
219+
RoaringPositionBitmap bitmap;
220+
ASSERT_THROW(bitmap.AddRange(100, 50), std::invalid_argument);
221+
}
222+
218223
struct OrParams {
219224
const char* name;
220225
std::set<int64_t> lhs_input;

0 commit comments

Comments
 (0)