lxy-9602 commented on code in PR #271:
URL: https://github.com/apache/paimon-cpp/pull/271#discussion_r3925210141
##########
src/paimon/common/file_index/bitmap/bitmap_file_index_test.cpp:
##########
@@ -620,6 +645,229 @@ TEST_F(BitmapIndexTest, TestTimestampType) {
}
}
+TEST_F(BitmapIndexTest, TestFloatAndDoubleTypes) {
+ const auto check_float = [&](int32_t version) {
+ const auto type = arrow::float32();
+ auto array =
+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({arrow::field("f0",
type)}),
+ R"([[1.25], [null],
[-2.5], [1.25], [3.75]])")
+ .ValueOrDie();
+ ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR<Bytes> index_bytes,
+ WriteIndex(type, version, array));
+ auto input_stream =
+ std::make_shared<ByteArrayInputStream>(index_bytes->data(),
index_bytes->size());
+ BitmapFileIndex file_index({});
+ ASSERT_OK_AND_ASSIGN(auto reader,
+
file_index.CreateReader(CreateArrowSchema(type).get(), 0,
+ index_bytes->size(),
input_stream, pool_));
+ CheckResult(reader->VisitEqual(Literal(1.25f)).value(), {0, 3});
+ CheckResult(reader->VisitNotEqual(Literal(1.25f)).value(), {2, 4});
+ CheckResult(reader->VisitIsNull().value(), {1});
+ };
+
+ const auto check_double = [&](int32_t version) {
+ const auto type = arrow::float64();
+ auto array =
+
arrow::ipc::internal::json::ArrayFromJSON(arrow::struct_({arrow::field("f0",
type)}),
+ R"([[1.25], [null],
[-2.5], [1.25], [3.75]])")
+ .ValueOrDie();
+ ASSERT_OK_AND_ASSIGN(PAIMON_UNIQUE_PTR<Bytes> index_bytes,
+ WriteIndex(type, version, array));
+ auto input_stream =
+ std::make_shared<ByteArrayInputStream>(index_bytes->data(),
index_bytes->size());
+ BitmapFileIndex file_index({});
+ ASSERT_OK_AND_ASSIGN(auto reader,
+
file_index.CreateReader(CreateArrowSchema(type).get(), 0,
+ index_bytes->size(),
input_stream, pool_));
+ CheckResult(reader->VisitEqual(Literal(1.25)).value(), {0, 3});
+ CheckResult(reader->VisitNotEqual(Literal(1.25)).value(), {2, 4});
+ CheckResult(reader->VisitIsNull().value(), {1});
+ };
+
+ for (int32_t version : {1, 2}) {
+ check_float(version);
+ check_double(version);
+ }
+}
+
+TEST_F(BitmapIndexTest, TestFloatingPointJavaCompatibility) {
+ // Generated by BitmapFloatingPointCompatibilityTest with Apache Paimon
Java at
+ // 0043a70fd88ac75dcb83a8f2da5e72ce91e22b1f. The Java writer receives
canonical,
Review Comment:
Is `BitmapFloatingPointCompatibilityTest` a test for the Java repo? I’m not
sure I understand the intent behind this test name. If it is not necessary,
maybe we should just remove it together with this commit.
##########
src/paimon/core/operation/internal_read_context.cpp:
##########
@@ -276,10 +276,10 @@ Result<std::unique_ptr<InternalReadContext>>
InternalReadContext::Create(
}
// validate predicate
if (context->GetPredicate()) {
- PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithSchema(
- *read_schema, context->GetPredicate(),
/*validate_field_idx=*/true));
PAIMON_RETURN_NOT_OK(
PredicateValidator::ValidatePredicateWithLiterals(context->GetPredicate()));
+ PAIMON_RETURN_NOT_OK(PredicateValidator::ValidatePredicateWithSchema(
+ *read_schema, context->GetPredicate(),
/*validate_field_idx=*/true));
}
Review Comment:
Is this assumption a bit too tightly coupled? It depends on the call order
always being `ValidatePredicateWithLiterals` first and then
`ValidatePredicateWithSchema`. If these two are always used together, maybe it
would make sense to wrap them into a single abstraction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
