refactor: minor code cleanup in reader path (#15432)

Summary:
Pull Request resolved: #15432

X-link: facebookincubator/nimble#301

Reviewed By: HuamengJiang, Yuhta

Differential Revision: D86459223

fbshipit-source-id: cda6e66788f8b5d6d5df07e4faca263084ede5d6

Author: xiaoxmeng

velox/dwio/common/ColumnVisitors.h

@@ -491,7 +491,7 @@ class ColumnVisitor {
 
  protected:
   const TFilter& filter_;
-  SelectiveColumnReader* reader_;
+  SelectiveColumnReader* const reader_;
   const bool allowNulls_;
   const vector_size_t* rows_;
   vector_size_t numRows_;

velox/dwio/common/FormatData.h

@@ -131,8 +131,8 @@ class FormatData {
 /// Base class for format-specific reader initialization arguments.
 class FormatParams {
  public:
-  explicit FormatParams(memory::MemoryPool& pool, ColumnReaderStatistics& stats)
-      : pool_(pool), stats_(stats) {}
+  FormatParams(memory::MemoryPool& pool, ColumnReaderStatistics& stats)
+      : pool_(&pool), stats_(&stats) {}
 
   virtual ~FormatParams() = default;
 
@@ -143,16 +143,16 @@ class FormatParams {
       const velox::common::ScanSpec& scanSpec) = 0;
 
   memory::MemoryPool& pool() {
-    return pool_;
+    return *pool_;
   }
 
   ColumnReaderStatistics& runtimeStatistics() {
-    return stats_;
+    return *stats_;
   }
 
  private:
-  memory::MemoryPool& pool_;
-  ColumnReaderStatistics& stats_;
+  memory::MemoryPool* const pool_;
+  ColumnReaderStatistics* const stats_;
 };
 
 } // namespace facebook::velox::dwio::common

velox/dwio/common/ScanSpec.h

@@ -34,10 +34,10 @@ class ColumnStatistics;
 }
 namespace common {
 
-// Describes the filtering and value extraction for a
-// SelectiveColumnReader. This is owned by the TableScan Operator and
-// is passed to SelectiveColumnReaders at construction.  This is
-// mutable by readers to reflect filter order and other adaptations.
+/// Describes the filtering and value extraction for a
+/// SelectiveColumnReader. This is owned by the TableScan Operator and
+/// is passed to SelectiveColumnReaders at construction.  This is
+/// mutable by readers to reflect filter order and other adaptations.
 class ScanSpec {
  public:
   enum class ColumnType : int8_t {
@@ -46,7 +46,7 @@ class ScanSpec {
     kComposite, // A struct with all children not read from file
   };
 
-  // Convert ColumnType to its string name representation.
+  /// Convert ColumnType to its string name representation.
   static std::string_view columnTypeString(ColumnType columnType);
 
   static constexpr column_index_t kNoChannel = ~0;
@@ -56,9 +56,9 @@ class ScanSpec {
 
   explicit ScanSpec(const std::string& name) : fieldName_(name) {}
 
-  // Filter to apply. If 'this' corresponds to a struct/list/map, this
-  // can only be isNull or isNotNull, other filtering is given by
-  // 'children'.
+  /// Filter to apply. If 'this' corresponds to a struct/list/map, this
+  /// can only be isNull or isNotNull, other filtering is given by
+  /// 'children'.
   const common::Filter* filter() const {
     return filterDisabled_ ? nullptr : filter_.get();
   }
@@ -167,8 +167,8 @@ class ScanSpec {
     return projectOut_ || deltaUpdate_;
   }
 
-  // Position in the RowVector returned by the top level scan. Applies
-  // only to children of the root struct where projectOut_ is true.
+  /// Position in the RowVector returned by the top level scan. Applies
+  /// only to children of the root struct where projectOut_ is true.
   column_index_t channel() const {
     return channel_;
   }
@@ -408,29 +408,29 @@ class ScanSpec {
   // Number of times read is called on the corresponding reader. This
   // is used for setup on first use and to produce a read sequence
   // number for LazyVectors.
-  uint64_t numReads_ = 0;
+  uint64_t numReads_{0};
 
   // Ordinal position of 'this' in its containing spec. For a struct
   // member this is the position of the reader in the child
   // readers. If this describes an operation on an array element or a
   // map with numeric key, this is the subscript as defined for array
   // or map.
-  int64_t subscript_ = -1;
+  int64_t subscript_{-1};
   // Column name if this is a struct mamber. String key if this
   // describes an operation on a map value.
   std::string fieldName_;
   // Ordinal position of the extracted value in the containing
   // RowVector. Set only when this describes a struct member.
-  column_index_t channel_ = kNoChannel;
+  column_index_t channel_{kNoChannel};
 
   VectorPtr constantValue_;
-  bool projectOut_ = false;
+  bool projectOut_{false};
 
-  ColumnType columnType_ = ColumnType::kRegular;
+  ColumnType columnType_{ColumnType::kRegular};
 
   // True if a string dictionary or flat map in this field should be
   // returned as flat.
-  bool makeFlat_ = false;
+  bool makeFlat_{false};
   std::shared_ptr<const common::Filter> filter_;
   bool filterDisabled_ = false;
   dwio::common::DeltaColumnUpdater* deltaUpdate_ = nullptr;

velox/dwio/common/SelectiveColumnReader.cpp

@@ -479,15 +479,15 @@ void SelectiveColumnReader::addSkippedParentNulls(
     int64_t from,
     int64_t to,
     int32_t numNulls) {
-  auto rowsPerRowGroup = formatData_->rowsPerRowGroup();
+  const auto rowsPerRowGroup = formatData_->rowsPerRowGroup();
   if (rowsPerRowGroup.has_value() &&
       from / rowsPerRowGroup.value() >
           parentNullsRecordedTo_ / rowsPerRowGroup.value()) {
     // the new nulls are in a different row group than the last.
     parentNullsRecordedTo_ = from;
     numParentNulls_ = 0;
   }
-  if (parentNullsRecordedTo_) {
+  if (parentNullsRecordedTo_ > 0) {
     VELOX_CHECK_EQ(parentNullsRecordedTo_, from);
   }
   numParentNulls_ += numNulls;

velox/dwio/common/SelectiveStructColumnReader.cpp

@@ -407,7 +407,6 @@ void SelectiveStructColumnReaderBase::read(
   }
 
   const uint64_t* structNulls = nulls();
-
   // A struct reader may have a null/non-null filter
   if (scanSpec_->filter()) {
     const auto kind = scanSpec_->filter()->kind();

velox/dwio/common/SelectiveStructColumnReader.h

@@ -110,7 +110,7 @@ class SelectiveStructColumnReaderBase : public SelectiveColumnReader {
 
   // The subscript of childSpecs will be set to this value if the column is
   // constant (either explicitly or because it's missing).
-  static constexpr int32_t kConstantChildSpecSubscript = -1;
+  static constexpr int32_t kConstantChildSpecSubscript{-1};
 
   SelectiveStructColumnReaderBase(
       const TypePtr& requestedType,
@@ -199,7 +199,7 @@ class SelectiveStructColumnReaderBase : public SelectiveColumnReader {
 
   int64_t lazyVectorReadOffset_;
 
-  int64_t currentRowNumber_ = -1;
+  int64_t currentRowNumber_{-1};
 
   const Mutation* mutation_ = nullptr;