GDV-31:[C++]Fixed Literal ToString. (apache#85)

Literal string coversion was ignoring types, leading to mismatch in hashing of expressions.
praveenbingo · Aug 24, 2018 · 3c1156e · 3c1156e
1 parent cb74b04
commit 3c1156e
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 9 deletions.
diff --git a/cpp/src/gandiva/codegen/node.h b/cpp/src/gandiva/codegen/node.h
@@ -59,11 +59,12 @@ class LiteralNode : public Node {
  bool is_null() const { return is_null_; }
 
  std::string ToString() override {
+ std::stringstream ss;
+ ss << "(" << return_type()->ToString() << ") ";
  if (is_null()) {
- return std::string("null");
+ ss << std::string("null");
+ return ss.str();
  }
-
- std::stringstream ss;
  ss << holder();
  return ss.str();
  }

diff --git a/cpp/src/gandiva/codegen/projector.cc b/cpp/src/gandiva/codegen/projector.cc
@@ -212,7 +212,7 @@ Status Projector::ValidateArrayDataCapacity(const arrow::ArrayData &array_data,
  int64_t data_len = array_data.buffers[1]->capacity();
  if (data_len < min_data_len) {
  std::stringstream ss;
- ss << "data buffer for output field " << field.name() << "has size " << data_len
+ ss << "data buffer for output field " << field.name() << " has size " << data_len
  << ", must have minimum size " << min_data_len;
  return Status::Invalid(ss.str());
  }

diff --git a/cpp/src/gandiva/integ/literal_test.cc b/cpp/src/gandiva/integ/literal_test.cc
@@ -120,6 +120,29 @@ TEST_F(TestLiteral, TestSimpleArithmetic) {
  EXPECT_ARROW_ARRAY_EQUALS(exp_e, outputs.at(4));
 }
 
+TEST_F(TestLiteral, TestLiteralHash) {
+ auto schema = arrow::schema({});
+ // output fields
+ auto res = field("a", int32());
+ auto int_literal = TreeExprBuilder::MakeLiteral((int32_t)2);
+ auto expr = TreeExprBuilder::MakeExpression(int_literal, res);
+
+ // Build a projector for the expressions.
+ std::shared_ptr<Projector> projector;
+ Status status = Projector::Make(schema, {expr}, &projector);
+ EXPECT_TRUE(status.ok()) << status.message();
+
+ auto res1 = field("a", int64());
+ auto int_literal1 = TreeExprBuilder::MakeLiteral((int64_t)2);
+ auto expr1 = TreeExprBuilder::MakeExpression(int_literal1, res1);
+
+ // Build a projector for the expressions.
+ std::shared_ptr<Projector> projector1;
+ status = Projector::Make(schema, {expr1}, &projector1);
+ EXPECT_TRUE(status.ok()) << status.message();
+ EXPECT_TRUE(projector.get() != projector1.get());
+}
+
 TEST_F(TestLiteral, TestNullLiteral) {
  // schema for input fields
  auto field_a = field("a", int32());

diff --git a/cpp/src/gandiva/integ/to_string_test.cc b/cpp/src/gandiva/integ/to_string_test.cc
@@ -41,7 +41,7 @@ TEST_F(TestToString, TestAll) {
  auto literal_node = TreeExprBuilder::MakeLiteral((uint64_t)100);
  auto literal_expr =
  TreeExprBuilder::MakeExpression(literal_node, arrow::field("r", int64()));
- CHECK_EXPR_TO_STRING(literal_expr, "100");
+ CHECK_EXPR_TO_STRING(literal_expr, "(uint64) 100");
 
  auto f0 = arrow::field("f0", float64());
  auto f0_node = TreeExprBuilder::MakeField(f0);
@@ -63,8 +63,8 @@ TEST_F(TestToString, TestAll) {
 
  auto if_node = TreeExprBuilder::MakeIf(cond_node, then_node, else_node, int64());
  auto if_expr = TreeExprBuilder::MakeExpression(if_node, f1);
- CHECK_EXPR_TO_STRING(if_expr,
-  "if (bool lesser_than(double, 0)) { int64 } else { int64 }");
+ CHECK_EXPR_TO_STRING(
+ if_expr, "if (bool lesser_than(double, (float) 0)) { int64 } else { int64 }");
 
  auto f1_gt_100 =
  TreeExprBuilder::MakeFunction("greater_than", {f1_node, literal_node}, boolean());
@@ -73,8 +73,9 @@ TEST_F(TestToString, TestAll) {
  auto and_node = TreeExprBuilder::MakeAnd({f1_gt_100, f2_equals_100});
  auto and_expr =
  TreeExprBuilder::MakeExpression(and_node, arrow::field("f0", boolean()));
- CHECK_EXPR_TO_STRING(and_expr,
- "bool greater_than(int64, 100) && bool equals(int64, 100)");
+ CHECK_EXPR_TO_STRING(
+ and_expr,
+ "bool greater_than(int64, (uint64) 100) && bool equals(int64, (uint64) 100)");
 }
 
 } // namespace gandiva