Move ArrowResultSetConverter to ResultSet library.

Signed-off-by: ienkovich <[email protected]>
intel · Jun 28, 2023 · 4793f42 · 4793f42
1 parent 84d38f1
commit 4793f42
Show file tree

Hide file tree

Showing 18 changed files with 39 additions and 71 deletions.
diff --git a/omniscidb/QueryEngine/CMakeLists.txt b/omniscidb/QueryEngine/CMakeLists.txt
@@ -12,9 +12,6 @@ set(query_engine_source_files
     ArithmeticIR.cpp
     ArrayIR.cpp
     ArrayOps.cpp
-    ArrowResultSetConverter.cpp
-    ArrowResultSet.cpp
-    BitmapGenerators.cpp
     CalciteDeserializerUtils.cpp
     CardinalityEstimator.cpp
     CaseIR.cpp
@@ -318,7 +315,6 @@ set(QUERY_ENGINE_LIBS
         SchemaMgr
         SqliteConnector
         SQLite::SQLite3
-        ${Arrow_LIBRARIES}
         )
 
 list(APPEND QUERY_ENGINE_LIBS ${llvm_libs} ${ZLIB_LIBRARIES})

diff --git a/omniscidb/QueryEngine/ArrowResultSet.cpp → omniscidb/ResultSet/ArrowResultSet.cpp b/omniscidb/QueryEngine/ArrowResultSet.cpp → omniscidb/ResultSet/ArrowResultSet.cpp
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#include "QueryEngine/ArrowResultSet.h"
+#include "ArrowResultSet.h"
 
 #include <arrow/api.h>
 #include <arrow/io/memory.h>
@@ -331,43 +331,15 @@ void ArrowResultSet::resultSetArrowLoopback(
 }
 
 std::unique_ptr<ArrowResultSet> result_set_arrow_loopback(
-    const ExecutionResult& results) {
-  // NOTE(wesm): About memory ownership
-
-  // After calling ReadRecordBatch, the buffers inside arrow::RecordBatch now
-  // share ownership of the memory in serialized_arrow_output.records (zero
-  // copy). Not necessary to retain these buffers. Same is true of any
-  // dictionaries contained in serialized_arrow_output.schema; the arrays
-  // reference that memory (zero copy).
-  return std::make_unique<ArrowResultSet>(results.getRows(), results.getTargetsMeta());
-}
-
-std::unique_ptr<ArrowResultSet> result_set_arrow_loopback(
-    const ExecutionResult* results,
-    const std::shared_ptr<ResultSet>& rows,
-    const ExecutorDeviceType device_type) {
-  return results ? std::make_unique<ArrowResultSet>(
-                       rows, results->getTargetsMeta(), device_type)
-                 : std::make_unique<ArrowResultSet>(rows, device_type);
-}
-
-std::unique_ptr<ArrowResultSet> result_set_arrow_loopback(
-    const ExecutionResult* results,
     const std::shared_ptr<ResultSet>& rows,
     const ExecutorDeviceType device_type,
     const size_t min_result_size_for_bulk_dictionary_fetch,
     const double max_dictionary_to_result_size_ratio_for_bulk_dictionary_fetch) {
   std::vector<hdk::ir::TargetMetaInfo> dummy_targets_meta;
-  return results ? std::make_unique<ArrowResultSet>(
-                       rows,
-                       results->getTargetsMeta(),
-                       device_type,
-                       min_result_size_for_bulk_dictionary_fetch,
-                       max_dictionary_to_result_size_ratio_for_bulk_dictionary_fetch)
-                 : std::make_unique<ArrowResultSet>(
-                       rows,
-                       dummy_targets_meta,
-                       device_type,
-                       min_result_size_for_bulk_dictionary_fetch,
-                       max_dictionary_to_result_size_ratio_for_bulk_dictionary_fetch);
+  return std::make_unique<ArrowResultSet>(
+      rows,
+      dummy_targets_meta,
+      device_type,
+      min_result_size_for_bulk_dictionary_fetch,
+      max_dictionary_to_result_size_ratio_for_bulk_dictionary_fetch);
 }
diff --git a/omniscidb/QueryEngine/ArrowResultSet.h → omniscidb/ResultSet/ArrowResultSet.h b/omniscidb/QueryEngine/ArrowResultSet.h → omniscidb/ResultSet/ArrowResultSet.h
@@ -16,9 +16,6 @@
 
 #pragma once
 
-#include "CompilationOptions.h"
-#include "Descriptors/RelAlgExecutionDescriptor.h"
-
 #include "DataMgr/DataMgr.h"
 #include "IR/TargetMetaInfo.h"
 #include "ResultSet/ResultSet.h"
@@ -183,26 +180,11 @@ ArrowResultSetRowIterator::value_type ArrowResultSetRowIterator::operator*() con
   return result_set_->getRowAt(crt_row_idx_);
 }
 
-class ExecutionResult;
-
-// The following result_set_arrow_loopback methods are used by our test
-// framework (ExecuteTest specifically) to take results from the executor,
-// serialize them to Arrow and then deserialize them to an ArrowResultSet,
-// which can then be used by the test framework.
-
-std::unique_ptr<ArrowResultSet> result_set_arrow_loopback(const ExecutionResult& results);
-
-std::unique_ptr<ArrowResultSet> result_set_arrow_loopback(
-    const ExecutionResult* results,
-    const std::shared_ptr<ResultSet>& rows,
-    const ExecutorDeviceType device_type = ExecutorDeviceType::CPU);
-
 // This version of result_set_arrow_loopback allows setting the parameters that
 // drive the choice between dense and sparse dictionary conversion, used for
 // Select.ArrowDictionaries tests in ExecuteTest
 
 std::unique_ptr<ArrowResultSet> result_set_arrow_loopback(
-    const ExecutionResult* results,
     const std::shared_ptr<ResultSet>& rows,
     const ExecutorDeviceType device_type,
     const size_t min_result_size_for_bulk_dictionary_fetch,

diff --git a/...b/QueryEngine/ArrowResultSetConverter.cpp → ...idb/ResultSet/ArrowResultSetConverter.cpp b/...b/QueryEngine/ArrowResultSetConverter.cpp → ...idb/ResultSet/ArrowResultSetConverter.cpp
@@ -17,10 +17,12 @@
 //  project headers
 #include "ArrowResultSet.h"
 #include "BitmapGenerators.h"
-#include "Execute.h"
+
 #include "Shared/ArrowUtil.h"
 #include "Shared/DateConverters.h"
+#include "Shared/thread_count.h"
 #include "Shared/toString.h"
+#include "StringDictionary/StringDictionaryProxy.h"
 
 //  arrow headers
 #include "arrow/api.h"

diff --git a/omniscidb/QueryEngine/BitmapGenerators.cpp → omniscidb/ResultSet/BitmapGenerators.cpp b/omniscidb/QueryEngine/BitmapGenerators.cpp → omniscidb/ResultSet/BitmapGenerators.cpp
@@ -1,3 +1,10 @@
+/*
+ * Copyright (C) 2023 Intel Corporation
+ * Copyright 2021 OmniSci, Inc.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
 #include "BitmapGenerators.h"
 #include <immintrin.h>
 

diff --git a/omniscidb/QueryEngine/BitmapGenerators.h → omniscidb/ResultSet/BitmapGenerators.h b/omniscidb/QueryEngine/BitmapGenerators.h → omniscidb/ResultSet/BitmapGenerators.h
@@ -1,3 +1,10 @@
+/*
+ * Copyright (C) 2023 Intel Corporation
+ * Copyright 2021 OmniSci, Inc.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
 #ifndef BITMAP_GENERATORS_h
 #define BITMAP_GENERATORS_h
 
@@ -23,4 +30,4 @@ size_t gen_null_bitmap_64(uint8_t* bitmap,
                           const uint64_t* data,
                           size_t size,
                           const uint64_t null_val);
-#endif
+#endif
diff --git a/omniscidb/ResultSet/CMakeLists.txt b/omniscidb/ResultSet/CMakeLists.txt
@@ -1,4 +1,7 @@
 set(result_set_source_files
+    ArrowResultSet.cpp
+    ArrowResultSetConverter.cpp
+    BitmapGenerators.cpp
     ColSlotContext.cpp
     QueryMemoryDescriptor.cpp
     ResultSet.cpp
@@ -11,4 +14,4 @@ set(result_set_source_files
 
 add_library(ResultSet ${result_set_source_files})
 
-target_link_libraries(ResultSet DataMgr IR Logger Utils StringDictionary ${Folly_LIBRARIES})
+target_link_libraries(ResultSet DataMgr IR Logger Utils StringDictionary ${Folly_LIBRARIES} ${Arrow_LIBRARIES})
diff --git a/omniscidb/Tests/ArrowBasedExecuteTest.cpp b/omniscidb/Tests/ArrowBasedExecuteTest.cpp
@@ -18,9 +18,9 @@
 #include "ArrowSQLRunner/SQLiteComparator.h"
 #include "TestHelpers.h"
 
-#include "QueryEngine/ArrowResultSet.h"
 #include "QueryEngine/Execute.h"
 #include "QueryEngine/ResultSetReductionJIT.h"
+#include "ResultSet/ArrowResultSet.h"
 #include "Shared/scope.h"
 
 #include <gtest/gtest.h>

diff --git a/omniscidb/Tests/ArrowSQLRunner/ArrowSQLRunner.cpp b/omniscidb/Tests/ArrowSQLRunner/ArrowSQLRunner.cpp
@@ -263,7 +263,6 @@ class ArrowSQLRunnerImpl {
                double max_dictionary_to_result_size_ratio_for_bulk_dictionary_fetch) {
     auto results = run_multiple_agg(query_string, device_type);
     auto arrow_omnisci_results = result_set_arrow_loopback(
-        nullptr,
         results,
         device_type,
         min_result_size_for_bulk_dictionary_fetch,

diff --git a/omniscidb/Tests/ArrowSQLRunner/ArrowSQLRunner.h b/omniscidb/Tests/ArrowSQLRunner/ArrowSQLRunner.h
@@ -18,9 +18,9 @@
 
 #include "ArrowStorage/ArrowStorage.h"
 #include "IR/Context.h"
-#include "QueryEngine/ArrowResultSet.h"
 #include "QueryEngine/CompilationOptions.h"
 #include "QueryEngine/Descriptors/RelAlgExecutionDescriptor.h"
+#include "ResultSet/ArrowResultSet.h"
 #include "ResultSetRegistry/ResultSetRegistry.h"
 #include "Shared/Config.h"
 

diff --git a/omniscidb/Tests/ArrowSQLRunner/SQLiteComparator.h b/omniscidb/Tests/ArrowSQLRunner/SQLiteComparator.h
@@ -16,7 +16,7 @@
 
 #pragma once
 
-#include "QueryEngine/ArrowResultSet.h"
+#include "ResultSet/ArrowResultSet.h"
 #include "Shared/sqltypes.h"
 #include "SqliteConnector/SqliteConnector.h"
 

diff --git a/omniscidb/Tests/ArrowStorageSqlTest.cpp b/omniscidb/Tests/ArrowStorageSqlTest.cpp
@@ -16,8 +16,8 @@
 #include "Calcite/CalciteJNI.h"
 #include "DataMgr/DataMgrBufferProvider.h"
 #include "DataMgr/DataMgrDataProvider.h"
-#include "QueryEngine/ArrowResultSet.h"
 #include "QueryEngine/RelAlgExecutor.h"
+#include "ResultSet/ArrowResultSet.h"
 
 #include "ArrowSQLRunner/ArrowSQLRunner.h"
 

diff --git a/omniscidb/Tests/CorrelatedSubqueryTest.cpp b/omniscidb/Tests/CorrelatedSubqueryTest.cpp
@@ -19,9 +19,9 @@
 #include "TestHelpers.h"
 
 #include "ConfigBuilder/ConfigBuilder.h"
-#include "QueryEngine/ArrowResultSet.h"
 #include "QueryEngine/Execute.h"
 #include "QueryEngine/RelAlgExecutor.h"
+#include "ResultSet/ArrowResultSet.h"
 #include "Shared/file_delete.h"
 #include "Shared/scope.h"
 

diff --git a/omniscidb/Tests/FromTableReorderingTest.cpp b/omniscidb/Tests/FromTableReorderingTest.cpp
@@ -16,10 +16,10 @@
 
 #include "TestHelpers.h"
 
-#include "../QueryEngine/ArrowResultSet.h"
 #include "../QueryEngine/Descriptors/RelAlgExecutionDescriptor.h"
 #include "../QueryEngine/Execute.h"
 #include "../QueryEngine/FromTableReordering.h"
+#include "../ResultSet/ArrowResultSet.h"
 #include "../Shared/scope.h"
 #include "../SqliteConnector/SqliteConnector.h"
 

diff --git a/omniscidb/Tests/NoCatalogSqlTest.cpp b/omniscidb/Tests/NoCatalogSqlTest.cpp
@@ -15,8 +15,8 @@
 #include "Calcite/CalciteJNI.h"
 #include "DataMgr/DataMgrBufferProvider.h"
 #include "DataMgr/DataMgrDataProvider.h"
-#include "QueryEngine/ArrowResultSet.h"
 #include "QueryEngine/RelAlgExecutor.h"
+#include "ResultSet/ArrowResultSet.h"
 #include "SchemaMgr/SimpleSchemaProvider.h"
 #include "Shared/scope.h"
 

diff --git a/omniscidb/Tests/ResultSetArrowConversion.cpp b/omniscidb/Tests/ResultSetArrowConversion.cpp
@@ -17,7 +17,7 @@
 #include "ArrowSQLRunner/ArrowSQLRunner.h"
 #include "ArrowTestHelpers.h"
 
-#include "QueryEngine/ArrowResultSet.h"
+#include "ResultSet/ArrowResultSet.h"
 #include "Shared/ArrowUtil.h"
 #include "Shared/InlineNullValues.h"
 #include "Shared/scope.h"

diff --git a/omniscidb/Tests/TestHelpers.h b/omniscidb/Tests/TestHelpers.h
@@ -18,8 +18,8 @@
 #define TEST_HELPERS_H_
 
 #include "Logger/Logger.h"
-#include "QueryEngine/ArrowResultSet.h"
 #include "QueryEngine/Descriptors/RelAlgExecutionDescriptor.h"
+#include "ResultSet/ArrowResultSet.h"
 
 #include <gtest/gtest.h>
 #include <boost/algorithm/string.hpp>

diff --git a/python/pyhdk/_execute.pxd b/python/pyhdk/_execute.pxd
@@ -137,7 +137,7 @@ cdef extern from "omniscidb/IR/TargetMetaInfo.h":
     const string& get_resname()
     const CType* type()
 
-cdef extern from "omniscidb/QueryEngine/ArrowResultSet.h":
+cdef extern from "omniscidb/ResultSet/ArrowResultSet.h":
   cdef cppclass CArrowResultSetConverter "ArrowResultSetConverter":
     CArrowResultSetConverter(const CResultSetPtr&, const vector[string]&, int)