Reorder SplitReader class fields

yingsu00 · Mar 29, 2024 · 4815c45 · 4815c45
1 parent 1fd3689
commit 4815c45
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 64 deletions.
diff --git a/velox/connectors/hive/HiveDataSource.cpp b/velox/connectors/hive/HiveDataSource.cpp
@@ -167,14 +167,14 @@ std::unique_ptr<SplitReader> HiveDataSource::createSplitReader() {
   return SplitReader::create(
       split_,
       hiveTableHandle_,
-      scanSpec_,
-      readerOutputType_,
       &partitionKeys_,
-      fileHandleFactory_,
-      executor_,
       connectorQueryCtx_,
       hiveConfig_,
-      ioStats_);
+      readerOutputType_,
+      ioStats_,
+      fileHandleFactory_,
+      executor_,
+      scanSpec_);
 }
 
 void HiveDataSource::addSplit(std::shared_ptr<ConnectorSplit> split) {

diff --git a/velox/connectors/hive/SplitReader.cpp b/velox/connectors/hive/SplitReader.cpp
@@ -62,69 +62,69 @@ VectorPtr newConstantFromString(
 std::unique_ptr<SplitReader> SplitReader::create(
     const std::shared_ptr<hive::HiveConnectorSplit>& hiveSplit,
     const std::shared_ptr<const HiveTableHandle>& hiveTableHandle,
-    const std::shared_ptr<common::ScanSpec>& scanSpec,
-    const RowTypePtr& readerOutputType,
     const std::unordered_map<
         std::string,
         std::shared_ptr<const HiveColumnHandle>>* partitionKeys,
-    FileHandleFactory* fileHandleFactory,
-    folly::Executor* executor,
     const ConnectorQueryCtx* connectorQueryCtx,
     const std::shared_ptr<const HiveConfig>& hiveConfig,
-    const std::shared_ptr<io::IoStatistics>& ioStats) {
+    const RowTypePtr& readerOutputType,
+    const std::shared_ptr<io::IoStatistics>& ioStats,
+    FileHandleFactory* fileHandleFactory,
+    folly::Executor* executor,
+    const std::shared_ptr<common::ScanSpec>& scanSpec) {
   //  Create the SplitReader based on hiveSplit->customSplitInfo["table_format"]
   if (hiveSplit->customSplitInfo.count("table_format") > 0 &&
       hiveSplit->customSplitInfo["table_format"] == "hive-iceberg") {
     return std::make_unique<iceberg::IcebergSplitReader>(
         hiveSplit,
         hiveTableHandle,
-        scanSpec,
-        readerOutputType,
         partitionKeys,
-        fileHandleFactory,
-        executor,
         connectorQueryCtx,
         hiveConfig,
-        ioStats);
+        readerOutputType,
+        ioStats,
+        fileHandleFactory,
+        executor,
+        scanSpec);
   } else {
     return std::make_unique<SplitReader>(
         hiveSplit,
         hiveTableHandle,
-        scanSpec,
-        readerOutputType,
         partitionKeys,
-        fileHandleFactory,
-        executor,
         connectorQueryCtx,
         hiveConfig,
-        ioStats);
+        readerOutputType,
+        ioStats,
+        fileHandleFactory,
+        executor,
+        scanSpec);
   }
 }
 
 SplitReader::SplitReader(
     const std::shared_ptr<const hive::HiveConnectorSplit>& hiveSplit,
     const std::shared_ptr<const HiveTableHandle>& hiveTableHandle,
-    const std::shared_ptr<common::ScanSpec>& scanSpec,
-    const RowTypePtr& readerOutputType,
     const std::unordered_map<
         std::string,
         std::shared_ptr<const HiveColumnHandle>>* partitionKeys,
-    FileHandleFactory* fileHandleFactory,
-    folly::Executor* executor,
     const ConnectorQueryCtx* connectorQueryCtx,
     const std::shared_ptr<const HiveConfig>& hiveConfig,
-    const std::shared_ptr<io::IoStatistics>& ioStats)
+    const RowTypePtr& readerOutputType,
+    const std::shared_ptr<io::IoStatistics>& ioStats,
+    FileHandleFactory* fileHandleFactory,
+    folly::Executor* executor,
+    const std::shared_ptr<common::ScanSpec>& scanSpec)
     : hiveSplit_(hiveSplit),
       hiveTableHandle_(hiveTableHandle),
-      scanSpec_(scanSpec),
-      readerOutputType_(readerOutputType),
       partitionKeys_(partitionKeys),
-      pool_(connectorQueryCtx->memoryPool()),
-      fileHandleFactory_(fileHandleFactory),
-      executor_(executor),
       connectorQueryCtx_(connectorQueryCtx),
       hiveConfig_(hiveConfig),
+      readerOutputType_(readerOutputType),
       ioStats_(ioStats),
+      fileHandleFactory_(fileHandleFactory),
+      executor_(executor),
+      pool_(connectorQueryCtx->memoryPool()),
+      scanSpec_(scanSpec),
       baseReaderOpts_(connectorQueryCtx->memoryPool()),
       emptySplit_(false) {}
 
@@ -180,11 +180,8 @@ int64_t SplitReader::estimatedRowSize() const {
     return DataSource::kUnknownRowSize;
   }
 
-  auto size = baseRowReader_->estimatedRowSize();
-  if (size.has_value()) {
-    return size.value();
-  }
-  return DataSource::kUnknownRowSize;
+  const auto size = baseRowReader_->estimatedRowSize();
+  return size.value_or(DataSource::kUnknownRowSize);
 }
 
 void SplitReader::updateRuntimeStats(

diff --git a/velox/connectors/hive/SplitReader.h b/velox/connectors/hive/SplitReader.h
@@ -57,30 +57,30 @@ class SplitReader {
   static std::unique_ptr<SplitReader> create(
       const std::shared_ptr<hive::HiveConnectorSplit>& hiveSplit,
       const std::shared_ptr<const HiveTableHandle>& hiveTableHandle,
-      const std::shared_ptr<common::ScanSpec>& scanSpec,
-      const RowTypePtr& readerOutputType,
       const std::unordered_map<
           std::string,
           std::shared_ptr<const HiveColumnHandle>>* partitionKeys,
-      FileHandleFactory* fileHandleFactory,
-      folly::Executor* executor,
       const ConnectorQueryCtx* connectorQueryCtx,
       const std::shared_ptr<const HiveConfig>& hiveConfig,
-      const std::shared_ptr<io::IoStatistics>& ioStats);
+      const RowTypePtr& readerOutputType,
+      const std::shared_ptr<io::IoStatistics>& ioStats,
+      FileHandleFactory* fileHandleFactory,
+      folly::Executor* executor,
+      const std::shared_ptr<common::ScanSpec>& scanSpec);
 
   SplitReader(
       const std::shared_ptr<const hive::HiveConnectorSplit>& hiveSplit,
       const std::shared_ptr<const HiveTableHandle>& hiveTableHandle,
-      const std::shared_ptr<common::ScanSpec>& scanSpec,
-      const RowTypePtr& readerOutputType,
       const std::unordered_map<
           std::string,
           std::shared_ptr<const HiveColumnHandle>>* partitionKeys,
-      FileHandleFactory* fileHandleFactory,
-      folly::Executor* executor,
       const ConnectorQueryCtx* connectorQueryCtx,
       const std::shared_ptr<const HiveConfig>& hiveConfig,
-      const std::shared_ptr<io::IoStatistics>& ioStats);
+      const RowTypePtr& readerOutputType,
+      const std::shared_ptr<io::IoStatistics>& ioStats,
+      FileHandleFactory* fileHandleFactory,
+      folly::Executor* executor,
+      const std::shared_ptr<common::ScanSpec>& scanSpec);
 
   virtual ~SplitReader() = default;
 
@@ -139,19 +139,19 @@ class SplitReader {
       const std::string& partitionKey,
       const std::optional<std::string>& value) const;
 
+  std::shared_ptr<const HiveConnectorSplit> hiveSplit_;
   const std::shared_ptr<const HiveTableHandle> hiveTableHandle_;
   const std::unordered_map<
       std::string,
       std::shared_ptr<const HiveColumnHandle>>* const partitionKeys_;
   const ConnectorQueryCtx* const connectorQueryCtx_;
   const std::shared_ptr<const HiveConfig> hiveConfig_;
 
-  std::shared_ptr<const HiveConnectorSplit> hiveSplit_;
   const RowTypePtr readerOutputType_;
   const std::shared_ptr<io::IoStatistics> ioStats_;
-  memory::MemoryPool* const pool_;
   FileHandleFactory* const fileHandleFactory_;
   folly::Executor* const executor_;
+  memory::MemoryPool* const pool_;
 
   std::shared_ptr<common::ScanSpec> scanSpec_;
   std::unique_ptr<dwio::common::Reader> baseReader_;

diff --git a/velox/connectors/hive/iceberg/IcebergSplitReader.cpp b/velox/connectors/hive/iceberg/IcebergSplitReader.cpp
@@ -25,29 +25,29 @@ using namespace facebook::velox::dwio::common;
 namespace facebook::velox::connector::hive::iceberg {
 
 IcebergSplitReader::IcebergSplitReader(
-    const std::shared_ptr<hive::HiveConnectorSplit>& hiveSplit,
+    const std::shared_ptr<const hive::HiveConnectorSplit>& hiveSplit,
     const std::shared_ptr<const HiveTableHandle>& hiveTableHandle,
-    const std::shared_ptr<common::ScanSpec>& scanSpec,
-    const RowTypePtr& readerOutputType,
     const std::unordered_map<
         std::string,
         std::shared_ptr<const HiveColumnHandle>>* partitionKeys,
-    FileHandleFactory* fileHandleFactory,
-    folly::Executor* executor,
     const ConnectorQueryCtx* connectorQueryCtx,
     const std::shared_ptr<const HiveConfig>& hiveConfig,
-    std::shared_ptr<io::IoStatistics> ioStats)
+    const RowTypePtr& readerOutputType,
+    const std::shared_ptr<io::IoStatistics>& ioStats,
+    FileHandleFactory* const fileHandleFactory,
+    folly::Executor* executor,
+    const std::shared_ptr<common::ScanSpec>& scanSpec)
     : SplitReader(
           hiveSplit,
           hiveTableHandle,
-          scanSpec,
-          readerOutputType,
           partitionKeys,
-          fileHandleFactory,
-          executor,
           connectorQueryCtx,
           hiveConfig,
-          ioStats),
+          readerOutputType,
+          ioStats,
+          fileHandleFactory,
+          executor,
+          scanSpec),
       baseReadOffset_(0),
       splitOffset_(0) {}
 

diff --git a/velox/connectors/hive/iceberg/IcebergSplitReader.h b/velox/connectors/hive/iceberg/IcebergSplitReader.h
@@ -27,18 +27,18 @@ struct IcebergDeleteFile;
 class IcebergSplitReader : public SplitReader {
  public:
   IcebergSplitReader(
-      const std::shared_ptr<hive::HiveConnectorSplit>& hiveSplit,
+      const std::shared_ptr<const hive::HiveConnectorSplit>& hiveSplit,
       const std::shared_ptr<const HiveTableHandle>& hiveTableHandle,
-      const std::shared_ptr<common::ScanSpec>& scanSpec,
-      const RowTypePtr& readerOutputType,
       const std::unordered_map<
           std::string,
           std::shared_ptr<const HiveColumnHandle>>* partitionKeys,
-      FileHandleFactory* fileHandleFactory,
-      folly::Executor* executor,
       const ConnectorQueryCtx* connectorQueryCtx,
       const std::shared_ptr<const HiveConfig>& hiveConfig,
-      std::shared_ptr<io::IoStatistics> ioStats);
+      const RowTypePtr& readerOutputType,
+      const std::shared_ptr<io::IoStatistics>& ioStats,
+      FileHandleFactory* fileHandleFactory,
+      folly::Executor* executor,
+      const std::shared_ptr<common::ScanSpec>& scanSpec);
 
   ~IcebergSplitReader() override = default;