kou commented on code in PR #49951:
URL: https://github.com/apache/arrow/pull/49951#discussion_r3424981456
##########
cpp/src/gandiva/tests/in_expr_test.cc:
##########
@@ -221,6 +221,62 @@ TEST_F(TestIn, TestInDecimal) {
EXPECT_ARROW_ARRAY_EQUALS(exp, selection_vector->ToArray());
}
+TEST_F(TestIn, TestInDecimalCached) {
+ int32_t precision = 38;
+ int32_t scale = 5;
+ auto decimal_type = std::make_shared<arrow::Decimal128Type>(precision,
scale);
+
+ auto field0 = field("f0", arrow::decimal128(precision, scale));
+ auto schema = arrow::schema({field0});
+
+ auto node_f0 = TreeExprBuilder::MakeField(field0);
+
+ gandiva::DecimalScalar128 d0("6", precision, scale);
+ gandiva::DecimalScalar128 d1("12", precision, scale);
+ gandiva::DecimalScalar128 d2("11", precision, scale);
+
+ std::unordered_set<gandiva::DecimalScalar128> in_constants({d0, d1, d2});
+
+ auto in_expr = TreeExprBuilder::MakeInExpressionDecimal(node_f0,
in_constants);
+ auto condition = TreeExprBuilder::MakeCondition(in_expr);
+
+ int num_records = 5;
+
+ auto values0 = MakeDecimalVector({"1", "2", "0", "-6", "6"});
+
+ auto array0 =
+ MakeArrowArrayDecimal(decimal_type, values0, {true, true, true, false,
true});
+
+ auto exp = MakeArrowArrayUint16({4});
+
+ auto in_batch = arrow::RecordBatch::Make(schema, num_records, {array0});
+
+ // Run twice to force cache hit on second build.
+ for (int i = 0; i < 2; ++i) {
+ std::shared_ptr<Filter> filter;
+
+ auto status = Filter::Make(schema, condition, TestConfiguration(),
&filter);
+
+ EXPECT_TRUE(status.ok());
+
+ if (i == 1) {
+ EXPECT_TRUE(filter->GetBuiltFromCache());
+ }
+
+ std::shared_ptr<SelectionVector> selection_vector;
+
+ status = SelectionVector::MakeInt16(num_records, pool_, &selection_vector);
+
+ EXPECT_TRUE(status.ok());
Review Comment:
```suggestion
ASSERT_OK(SelectionVector::MakeInt16(num_records, pool_,
&selection_vector));
```
##########
cpp/src/gandiva/tests/in_expr_test.cc:
##########
@@ -221,6 +221,62 @@ TEST_F(TestIn, TestInDecimal) {
EXPECT_ARROW_ARRAY_EQUALS(exp, selection_vector->ToArray());
}
+TEST_F(TestIn, TestInDecimalCached) {
+ int32_t precision = 38;
+ int32_t scale = 5;
+ auto decimal_type = std::make_shared<arrow::Decimal128Type>(precision,
scale);
+
+ auto field0 = field("f0", arrow::decimal128(precision, scale));
+ auto schema = arrow::schema({field0});
+
+ auto node_f0 = TreeExprBuilder::MakeField(field0);
+
+ gandiva::DecimalScalar128 d0("6", precision, scale);
+ gandiva::DecimalScalar128 d1("12", precision, scale);
+ gandiva::DecimalScalar128 d2("11", precision, scale);
+
+ std::unordered_set<gandiva::DecimalScalar128> in_constants({d0, d1, d2});
+
+ auto in_expr = TreeExprBuilder::MakeInExpressionDecimal(node_f0,
in_constants);
+ auto condition = TreeExprBuilder::MakeCondition(in_expr);
+
+ int num_records = 5;
+
+ auto values0 = MakeDecimalVector({"1", "2", "0", "-6", "6"});
+
+ auto array0 =
+ MakeArrowArrayDecimal(decimal_type, values0, {true, true, true, false,
true});
+
+ auto exp = MakeArrowArrayUint16({4});
+
+ auto in_batch = arrow::RecordBatch::Make(schema, num_records, {array0});
+
+ // Run twice to force cache hit on second build.
+ for (int i = 0; i < 2; ++i) {
+ std::shared_ptr<Filter> filter;
+
+ auto status = Filter::Make(schema, condition, TestConfiguration(),
&filter);
+
+ EXPECT_TRUE(status.ok());
Review Comment:
```suggestion
ASSERT_OK(Filter::Make(schema, condition, TestConfiguration(), &filter));
```
##########
cpp/src/gandiva/tests/in_expr_test.cc:
##########
@@ -221,6 +221,62 @@ TEST_F(TestIn, TestInDecimal) {
EXPECT_ARROW_ARRAY_EQUALS(exp, selection_vector->ToArray());
}
+TEST_F(TestIn, TestInDecimalCached) {
Review Comment:
Does this test cover the existing `TestInDecimal` case when `i == 0`?
If so, how about adding `for (i = 0; i < 2; ++1)` to the existing
`TestInDecimal` not adding a new test?
##########
cpp/src/gandiva/tests/in_expr_test.cc:
##########
@@ -221,6 +221,62 @@ TEST_F(TestIn, TestInDecimal) {
EXPECT_ARROW_ARRAY_EQUALS(exp, selection_vector->ToArray());
}
+TEST_F(TestIn, TestInDecimalCached) {
+ int32_t precision = 38;
+ int32_t scale = 5;
+ auto decimal_type = std::make_shared<arrow::Decimal128Type>(precision,
scale);
+
+ auto field0 = field("f0", arrow::decimal128(precision, scale));
+ auto schema = arrow::schema({field0});
+
+ auto node_f0 = TreeExprBuilder::MakeField(field0);
+
+ gandiva::DecimalScalar128 d0("6", precision, scale);
+ gandiva::DecimalScalar128 d1("12", precision, scale);
+ gandiva::DecimalScalar128 d2("11", precision, scale);
+
+ std::unordered_set<gandiva::DecimalScalar128> in_constants({d0, d1, d2});
+
+ auto in_expr = TreeExprBuilder::MakeInExpressionDecimal(node_f0,
in_constants);
+ auto condition = TreeExprBuilder::MakeCondition(in_expr);
+
+ int num_records = 5;
+
+ auto values0 = MakeDecimalVector({"1", "2", "0", "-6", "6"});
+
+ auto array0 =
+ MakeArrowArrayDecimal(decimal_type, values0, {true, true, true, false,
true});
+
+ auto exp = MakeArrowArrayUint16({4});
+
+ auto in_batch = arrow::RecordBatch::Make(schema, num_records, {array0});
+
+ // Run twice to force cache hit on second build.
+ for (int i = 0; i < 2; ++i) {
+ std::shared_ptr<Filter> filter;
+
+ auto status = Filter::Make(schema, condition, TestConfiguration(),
&filter);
+
+ EXPECT_TRUE(status.ok());
+
+ if (i == 1) {
+ EXPECT_TRUE(filter->GetBuiltFromCache());
+ }
+
+ std::shared_ptr<SelectionVector> selection_vector;
+
+ status = SelectionVector::MakeInt16(num_records, pool_, &selection_vector);
+
+ EXPECT_TRUE(status.ok());
+
+ status = filter->Evaluate(*in_batch, selection_vector);
+
+ EXPECT_TRUE(status.ok());
Review Comment:
```suggestion
ASSERT_OK(filter->Evaluate(*in_batch, selection_vector));
```
--
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]
