Skip to content

Commit 146a86e

Browse files
committed
Add basic tests, and fix float compare bug(qNan != sNan)
1 parent 07dd257 commit 146a86e

File tree

3 files changed

+422
-4
lines changed

3 files changed

+422
-4
lines changed

src/iceberg/literal.cc

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
#include "iceberg/literal.h"
2121

22+
#include <cmath>
23+
#include <concepts>
2224
#include <sstream>
2325

2426
#include "iceberg/exception.h"
@@ -60,11 +62,11 @@ PrimitiveLiteral PrimitiveLiteral::Binary(std::vector<uint8_t> value) {
6062
}
6163

6264
PrimitiveLiteral PrimitiveLiteral::BelowMinLiteral(std::shared_ptr<PrimitiveType> type) {
63-
return PrimitiveLiteral(PrimitiveLiteralValue{BelowMin{}}, std::move(type));
65+
return {PrimitiveLiteralValue{BelowMin{}}, std::move(type)};
6466
}
6567

6668
PrimitiveLiteral PrimitiveLiteral::AboveMaxLiteral(std::shared_ptr<PrimitiveType> type) {
67-
return PrimitiveLiteral(PrimitiveLiteralValue{AboveMax{}}, std::move(type));
69+
return {PrimitiveLiteralValue{AboveMax{}}, std::move(type)};
6870
}
6971

7072
Result<PrimitiveLiteral> PrimitiveLiteral::Deserialize(std::span<const uint8_t> data) {
@@ -171,6 +173,30 @@ Result<PrimitiveLiteral> PrimitiveLiteral::CastFromFloat(TypeId target_type_id)
171173
}
172174
}
173175

176+
// Template function for floating point comparison following Iceberg rules:
177+
// -NaN < NaN, but all NaN values (qNaN, sNaN) are treated as equivalent within their sign
178+
template <std::floating_point T>
179+
std::partial_ordering iceberg_float_compare(T lhs, T rhs) {
180+
bool lhs_is_nan = std::isnan(lhs);
181+
bool rhs_is_nan = std::isnan(rhs);
182+
183+
// If both are NaN, check their signs
184+
if (lhs_is_nan && rhs_is_nan) {
185+
bool lhs_is_negative = std::signbit(lhs);
186+
bool rhs_is_negative = std::signbit(rhs);
187+
188+
if (lhs_is_negative == rhs_is_negative) {
189+
// Same sign NaN values are equivalent (no qNaN vs sNaN distinction)
190+
return std::partial_ordering::equivalent;
191+
}
192+
// -NaN < NaN
193+
return lhs_is_negative ? std::partial_ordering::less : std::partial_ordering::greater;
194+
}
195+
196+
// For non-NaN values, use standard strong ordering
197+
return std::strong_order(lhs, rhs);
198+
}
199+
174200
// Three-way comparison operator
175201
std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& other) const {
176202
// If types are different, comparison is unordered
@@ -208,14 +234,14 @@ std::partial_ordering PrimitiveLiteral::operator<=>(const PrimitiveLiteral& othe
208234
auto this_val = std::get<float>(value_);
209235
auto other_val = std::get<float>(other.value_);
210236
// Use strong_ordering for floating point as spec requests
211-
return std::strong_order(this_val, other_val);
237+
return iceberg_float_compare(this_val, other_val);
212238
}
213239

214240
case TypeId::kDouble: {
215241
auto this_val = std::get<double>(value_);
216242
auto other_val = std::get<double>(other.value_);
217243
// Use strong_ordering for floating point as spec requests
218-
return std::strong_order(this_val, other_val);
244+
return iceberg_float_compare(this_val, other_val);
219245
}
220246

221247
case TypeId::kString: {

test/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ target_sources(util_test PRIVATE expected_test.cc formatter_test.cc config_test.
6969
target_link_libraries(util_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock)
7070
add_test(NAME util_test COMMAND util_test)
7171

72+
add_executable(literal_test)
73+
target_sources(literal_test PRIVATE literal_test.cc)
74+
target_link_libraries(literal_test PRIVATE iceberg_static GTest::gtest_main GTest::gmock)
75+
add_test(NAME literal_test COMMAND literal_test)
76+
7277
if(ICEBERG_BUILD_BUNDLE)
7378
add_executable(avro_test)
7479
target_sources(avro_test PRIVATE avro_test.cc avro_schema_test.cc avro_stream_test.cc)

0 commit comments

Comments
 (0)