You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ra...@apache.org on 2019/06/18 06:03:12 UTC
[arrow] branch master updated: ARROW-5626: [C++] Fix caching of
expressions with decimals
This is an automated email from the ASF dual-hosted git repository.
ravindra pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new a30bb7b ARROW-5626: [C++] Fix caching of expressions with decimals
a30bb7b is described below
commit a30bb7b96a7e04c88378f380b9228bb14b92902c
Author: Pindikura Ravindra <ra...@dremio.com>
AuthorDate: Tue Jun 18 11:32:34 2019 +0530
ARROW-5626: [C++] Fix caching of expressions with decimals
- use full DataType instead of just name
- added test for caching decimal expr
Author: Pindikura Ravindra <ra...@dremio.com>
Closes #4592 from pravindra/cast2 and squashes the following commits:
7506c00e5 <Pindikura Ravindra> ARROW-5626: fix caching of expressions with decimals
---
cpp/src/gandiva/expr_validator.cc | 4 +--
cpp/src/gandiva/like_holder_test.cc | 4 +--
cpp/src/gandiva/node.h | 4 +--
cpp/src/gandiva/tests/projector_test.cc | 45 +++++++++++++++++++++++++++++++++
4 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/cpp/src/gandiva/expr_validator.cc b/cpp/src/gandiva/expr_validator.cc
index bc532bd..923841c 100644
--- a/cpp/src/gandiva/expr_validator.cc
+++ b/cpp/src/gandiva/expr_validator.cc
@@ -34,9 +34,9 @@ Status ExprValidator::Validate(const ExpressionPtr& expr) {
// support validation is not required because root type is already supported.
ARROW_RETURN_IF(!root.return_type()->Equals(*expr->result()->type()),
Status::ExpressionValidationError("Return type of root node ",
- root.return_type()->name(),
+ root.return_type()->ToString(),
" does not match that of expression ",
- expr->result()->type()->name()));
+ expr->result()->type()->ToString()));
return Status::OK();
}
diff --git a/cpp/src/gandiva/like_holder_test.cc b/cpp/src/gandiva/like_holder_test.cc
index 087fcba..817473d 100644
--- a/cpp/src/gandiva/like_holder_test.cc
+++ b/cpp/src/gandiva/like_holder_test.cc
@@ -98,12 +98,12 @@ TEST_F(TestLikeHolder, TestOptimise) {
// optimise for 'starts_with'
auto fnode = LikeHolder::TryOptimize(BuildLike("xy 123z%"));
EXPECT_EQ(fnode.descriptor()->name(), "starts_with");
- EXPECT_EQ(fnode.ToString(), "bool starts_with((utf8) in, (const string) xy 123z)");
+ EXPECT_EQ(fnode.ToString(), "bool starts_with((string) in, (const string) xy 123z)");
// optimise for 'ends_with'
fnode = LikeHolder::TryOptimize(BuildLike("%xyz"));
EXPECT_EQ(fnode.descriptor()->name(), "ends_with");
- EXPECT_EQ(fnode.ToString(), "bool ends_with((utf8) in, (const string) xyz)");
+ EXPECT_EQ(fnode.ToString(), "bool ends_with((string) in, (const string) xyz)");
// no optimisation for others.
fnode = LikeHolder::TryOptimize(BuildLike("xyz_"));
diff --git a/cpp/src/gandiva/node.h b/cpp/src/gandiva/node.h
index 87f87e9..55782a6 100644
--- a/cpp/src/gandiva/node.h
+++ b/cpp/src/gandiva/node.h
@@ -105,7 +105,7 @@ class GANDIVA_EXPORT FieldNode : public Node {
const FieldPtr& field() const { return field_; }
std::string ToString() const override {
- return "(" + field()->type()->name() + ") " + field()->name();
+ return "(" + field()->type()->ToString() + ") " + field()->name();
}
private:
@@ -124,7 +124,7 @@ class GANDIVA_EXPORT FunctionNode : public Node {
std::string ToString() const override {
std::stringstream ss;
- ss << descriptor()->return_type()->name() << " " << descriptor()->name() << "(";
+ ss << descriptor()->return_type()->ToString() << " " << descriptor()->name() << "(";
bool skip_comma = true;
for (auto& child : children()) {
if (skip_comma) {
diff --git a/cpp/src/gandiva/tests/projector_test.cc b/cpp/src/gandiva/tests/projector_test.cc
index 238fbe2..01e9640 100644
--- a/cpp/src/gandiva/tests/projector_test.cc
+++ b/cpp/src/gandiva/tests/projector_test.cc
@@ -162,6 +162,51 @@ TEST_F(TestProjector, TestProjectCacheFloat) {
EXPECT_TRUE(projector0.get() != projector1.get());
}
+TEST_F(TestProjector, TestProjectCacheLiteral) {
+ auto schema = arrow::schema({});
+ auto res = field("result", arrow::decimal(38, 5));
+
+ DecimalScalar128 d0("12345678", 38, 5);
+ DecimalScalar128 d1("98756432", 38, 5);
+
+ auto literal0 = TreeExprBuilder::MakeDecimalLiteral(d0);
+ auto expr0 = TreeExprBuilder::MakeExpression(literal0, res);
+ std::shared_ptr<Projector> projector0;
+ ASSERT_OK(Projector::Make(schema, {expr0}, TestConfiguration(), &projector0));
+
+ auto literal1 = TreeExprBuilder::MakeDecimalLiteral(d1);
+ auto expr1 = TreeExprBuilder::MakeExpression(literal1, res);
+ std::shared_ptr<Projector> projector1;
+ ASSERT_OK(Projector::Make(schema, {expr1}, TestConfiguration(), &projector1));
+
+ EXPECT_NE(projector0.get(), projector1.get());
+}
+
+TEST_F(TestProjector, TestProjectCacheDecimalCast) {
+ auto field_float64 = field("float64", arrow::float64());
+ auto schema = arrow::schema({field_float64});
+
+ auto res_31_13 = field("result", arrow::decimal(31, 13));
+ auto expr0 = TreeExprBuilder::MakeExpression("castDECIMAL", {field_float64}, res_31_13);
+ std::shared_ptr<Projector> projector0;
+ ASSERT_OK(Projector::Make(schema, {expr0}, TestConfiguration(), &projector0));
+
+ // if the output scale is different, the cache can't be used.
+ auto res_31_14 = field("result", arrow::decimal(31, 14));
+ auto expr1 = TreeExprBuilder::MakeExpression("castDECIMAL", {field_float64}, res_31_14);
+ std::shared_ptr<Projector> projector1;
+ ASSERT_OK(Projector::Make(schema, {expr1}, TestConfiguration(), &projector1));
+ EXPECT_NE(projector0.get(), projector1.get());
+
+ // if the output scale/precision are same, should get a cache hit.
+ auto res_31_13_alt = field("result", arrow::decimal(31, 13));
+ auto expr2 =
+ TreeExprBuilder::MakeExpression("castDECIMAL", {field_float64}, res_31_13_alt);
+ std::shared_ptr<Projector> projector2;
+ ASSERT_OK(Projector::Make(schema, {expr2}, TestConfiguration(), &projector2));
+ EXPECT_EQ(projector0.get(), projector2.get());
+}
+
TEST_F(TestProjector, TestIntSumSub) {
// schema for input fields
auto field0 = field("f0", int32());