[arrow] branch master updated: ARROW-4275: [C++] [Gandiva] Fix slow decimal test

[ap...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

apitrou 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 8540035  ARROW-4275: [C++] [Gandiva] Fix slow decimal test
8540035 is described below

commit 854003548e48507e07c6b0c6f7b351a5a0ca3ef8
Author: Pindikura Ravindra <ra...@dremio.com>
AuthorDate: Tue Jan 22 14:47:20 2019 +0100

    ARROW-4275: [C++] [Gandiva] Fix slow decimal test
    
    - removed static variant of test
    - fixed a bug in cache lookup
    
    Author: Pindikura Ravindra <ra...@dremio.com>
    
    Closes #3426 from pravindra/slowtest and squashes the following commits:
    
    ffce286e <Pindikura Ravindra> ARROW-4275: add test for cache with configuration
    bed49e8b <Pindikura Ravindra> ARROW-4275: fix slow decimal test
---
 cpp/src/gandiva/projector_cache_key.h   |  4 ++--
 cpp/src/gandiva/tests/CMakeLists.txt    |  4 ----
 cpp/src/gandiva/tests/projector_test.cc | 31 ++++++++++++++++++++++++-------
 3 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/cpp/src/gandiva/projector_cache_key.h b/cpp/src/gandiva/projector_cache_key.h
index e583916..26da528 100644
--- a/cpp/src/gandiva/projector_cache_key.h
+++ b/cpp/src/gandiva/projector_cache_key.h
@@ -41,7 +41,7 @@ class ProjectorCacheKey {
       boost::hash_combine(result, expr_as_string);
       UpdateUniqifier(expr_as_string);
     }
-    boost::hash_combine(result, configuration);
+    boost::hash_combine(result, configuration->Hash());
     boost::hash_combine(result, schema_->ToString());
     boost::hash_combine(result, uniqifier_);
     hash_code_ = result;
@@ -55,7 +55,7 @@ class ProjectorCacheKey {
       return false;
     }
 
-    if (configuration_ != other.configuration_) {
+    if (*configuration_ != *other.configuration_) {
       return false;
     }
 
diff --git a/cpp/src/gandiva/tests/CMakeLists.txt b/cpp/src/gandiva/tests/CMakeLists.txt
index b47e5fd..c81618e 100644
--- a/cpp/src/gandiva/tests/CMakeLists.txt
+++ b/cpp/src/gandiva/tests/CMakeLists.txt
@@ -34,10 +34,6 @@ ADD_GANDIVA_TEST(projector_test_static
   SOURCES projector_test.cc
   USE_STATIC_LINKING)
 
-ADD_GANDIVA_TEST(decimal_single_test_static
-  SOURCES decimal_single_test.cc
-  USE_STATIC_LINKING)
-
 ADD_ARROW_BENCHMARK(micro_benchmarks
   PREFIX "gandiva"
   EXTRA_LINK_LIBS gandiva_static)
diff --git a/cpp/src/gandiva/tests/projector_test.cc b/cpp/src/gandiva/tests/projector_test.cc
index 33cdce0..5c32f50 100644
--- a/cpp/src/gandiva/tests/projector_test.cc
+++ b/cpp/src/gandiva/tests/projector_test.cc
@@ -54,15 +54,15 @@ TEST_F(TestProjector, TestProjectCache) {
 
   std::shared_ptr<Projector> projector;
   auto status = Projector::Make(schema, {sum_expr, sub_expr}, configuration, &projector);
-  EXPECT_TRUE(status.ok());
+  ASSERT_OK(status);
 
   // everything is same, should return the same projector.
   auto schema_same = arrow::schema({field0, field1});
   std::shared_ptr<Projector> cached_projector;
   status = Projector::Make(schema_same, {sum_expr, sub_expr}, configuration,
                            &cached_projector);
-  EXPECT_TRUE(status.ok());
-  EXPECT_TRUE(cached_projector.get() == projector.get());
+  ASSERT_OK(status);
+  EXPECT_EQ(cached_projector, projector);
 
   // schema is different should return a new projector.
   auto field2 = field("f2", int32());
@@ -70,14 +70,31 @@ TEST_F(TestProjector, TestProjectCache) {
   std::shared_ptr<Projector> should_be_new_projector;
   status = Projector::Make(different_schema, {sum_expr, sub_expr}, configuration,
                            &should_be_new_projector);
-  EXPECT_TRUE(status.ok());
-  EXPECT_TRUE(cached_projector.get() != should_be_new_projector.get());
+  ASSERT_OK(status);
+  EXPECT_NE(cached_projector, should_be_new_projector);
 
   // expression list is different should return a new projector.
   std::shared_ptr<Projector> should_be_new_projector1;
   status = Projector::Make(schema, {sum_expr}, configuration, &should_be_new_projector1);
-  EXPECT_TRUE(status.ok());
-  EXPECT_TRUE(cached_projector.get() != should_be_new_projector1.get());
+  ASSERT_OK(status);
+  EXPECT_NE(cached_projector, should_be_new_projector1);
+
+  // another instance of the same configuration, should return the same projector.
+  status = Projector::Make(schema, {sum_expr, sub_expr}, TestConfiguration(),
+                           &cached_projector);
+  ASSERT_OK(status);
+  EXPECT_EQ(cached_projector, projector);
+
+  // if configuration is different, should return a new projector.
+  auto other_configuration =
+      ConfigurationBuilder()
+          .set_byte_code_file_path("/" + std::string(GANDIVA_BYTE_COMPILE_FILE_PATH))
+          .build();
+  std::shared_ptr<Projector> should_be_new_projector2;
+  status = Projector::Make(schema, {sum_expr, sub_expr}, other_configuration,
+                           &should_be_new_projector2);
+  ASSERT_OK(status);
+  EXPECT_NE(projector, should_be_new_projector2);
 }
 
 TEST_F(TestProjector, TestProjectCacheFieldNames) {