Fix crash when select count(*) from table with delta delete (facebook…

…incubator#9984)

Summary:
When no column is projected out and there is deletion, we use the mutation object in argument, but the code path did not set the mutation object field to point to the argument.  Fix this by moving the setting of the field before the branching.

Pull Request resolved: facebookincubator#9984

Reviewed By: xiaoxmeng

Differential Revision: D57970942

fbshipit-source-id: acfcdc1af3aa9f7eaa7634749c71e96949b69959

velox/dwio/common/Mutation.h

@@ -29,4 +29,8 @@ struct Mutation {
   random::RandomSkipTracker* randomSkip = nullptr;
 };
 
+inline bool hasDeletion(const Mutation* mutation) {
+  return mutation && (mutation->deletedRows || mutation->randomSkip);
+}
+
 } // namespace facebook::velox::dwio::common

velox/dwio/common/Reader.cpp

@@ -205,7 +205,7 @@ void RowReader::readWithRowNumber(
     flatRowNum = rowNumVector->asUnchecked<FlatVector<int64_t>>();
   }
   auto* rawRowNum = flatRowNum->mutableRawValues();
-  if (numChildren == 0 && !mutation) {
+  if (numChildren == 0 && !hasDeletion(mutation)) {
     VELOX_DCHECK_EQ(rowsToRead, result->size());
     std::iota(rawRowNum, rawRowNum + rowsToRead, previousRow);
   } else {

velox/dwio/common/SelectiveColumnReader.h

@@ -174,7 +174,7 @@ class SelectiveColumnReader {
   // read(). If 'this' has no filter, returns 'rows' passed to last
   // read().
   const RowSet outputRows() const {
-    if (scanSpec_->hasFilter() || hasMutation()) {
+    if (scanSpec_->hasFilter() || hasDeletion()) {
       return outputRows_;
     }
     return inputRows_;
@@ -552,7 +552,7 @@ class SelectiveColumnReader {
   // copy.
   char* copyStringValue(folly::StringPiece value);
 
-  virtual bool hasMutation() const {
+  virtual bool hasDeletion() const {
     return false;
   }
 

velox/dwio/common/SelectiveColumnReaderInternal.h

@@ -99,7 +99,7 @@ void SelectiveColumnReader::prepareRead(
   numValues_ = 0;
   valueSize_ = sizeof(T);
   inputRows_ = rows;
-  if (scanSpec_->filter() || hasMutation()) {
+  if (scanSpec_->filter() || hasDeletion()) {
     outputRows_.reserve(rows.size());
   }
   ensureValuesCapacity<T>(rows.size());

velox/dwio/common/SelectiveStructColumnReader.cpp

@@ -83,8 +83,10 @@ void SelectiveStructColumnReaderBase::next(
     VectorPtr& result,
     const Mutation* mutation) {
   process::TraceContext trace("SelectiveStructColumnReaderBase::next");
+  mutation_ = mutation;
+  hasDeletion_ = common::hasDeletion(mutation);
   if (children_.empty()) {
-    if (mutation) {
+    if (hasDeletion_) {
       if (fillMutatedOutputRows_) {
         fillOutputRowsFromMutation(numValues);
         numValues = outputRows_.size();
@@ -121,8 +123,6 @@ void SelectiveStructColumnReaderBase::next(
   if (numValues > oldSize) {
     std::iota(&rows_[oldSize], &rows_[rows_.size()], oldSize);
   }
-  mutation_ = mutation;
-  hasMutation_ = mutation && (mutation->deletedRows || mutation->randomSkip);
   read(readOffset_, rows_, nullptr);
   getValues(outputRows(), &result);
 }
@@ -134,7 +134,7 @@ void SelectiveStructColumnReaderBase::read(
   numReads_ = scanSpec_->newRead();
   prepareRead<char>(offset, rows, incomingNulls);
   RowSet activeRows = rows;
-  if (hasMutation_) {
+  if (hasDeletion_) {
     // We handle the mutation after prepareRead so that output rows and format
     // specific initializations (e.g. RepDef in Parquet) are done properly.
     VELOX_DCHECK(!nullsInReadRange_, "Only top level can have mutation");

velox/dwio/common/SelectiveStructColumnReader.h

@@ -128,8 +128,8 @@ class SelectiveStructColumnReaderBase : public SelectiveColumnReader {
   // know how much to skip when seeking forward within the row group.
   void recordParentNullsInChildren(vector_size_t offset, RowSet rows);
 
-  bool hasMutation() const override {
-    return hasMutation_;
+  bool hasDeletion() const final {
+    return hasDeletion_;
   }
 
   // Returns true if we'll return a constant for that childSpec (i.e. we don't
@@ -155,7 +155,7 @@ class SelectiveStructColumnReaderBase : public SelectiveColumnReader {
 
   // After read() call mutation_ could go out of scope.  Need to keep this
   // around for lazy columns.
-  bool hasMutation_ = false;
+  bool hasDeletion_ = false;
 
   bool fillMutatedOutputRows_ = false;
 
@@ -274,7 +274,7 @@ void SelectiveFlatMapColumnReaderHelper<T, KeyNode, FormatData>::read(
     const uint64_t* incomingNulls) {
   reader_.numReads_ = reader_.scanSpec_->newRead();
   reader_.prepareRead<char>(offset, rows, incomingNulls);
-  VELOX_DCHECK(!reader_.hasMutation());
+  VELOX_DCHECK(!reader_.hasDeletion());
   auto activeRows = rows;
   auto* mapNulls = reader_.nullsInReadRange_
       ? reader_.nullsInReadRange_->as<uint64_t>()