Override copy.

facebookincubator · Aug 19, 2021 · 8747172 · 8747172
1 parent aae8c41
commit 8747172
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 46 deletions.
diff --git a/velox/expression/ControlExpr.cpp b/velox/expression/ControlExpr.cpp
@@ -43,10 +43,6 @@ void ConstantExpr::evalSpecialForm(
     BaseVector::ensureWritable(
         rows, (*result)->type(), context->execCtx()->pool(), result);
     (*result)->copy(sharedSubexprValues_.get(), rows, nullptr);
-    if (isString()) {
-      (*result)->asUnchecked<SimpleVector<StringView>>()->copyAsciiDataFrom(
-          sharedSubexprValues_.get(), rows);
-    }
     return;
   }
   *result = sharedSubexprValues_;
@@ -99,10 +95,6 @@ void FieldReference::evalSpecialForm(
     auto indices = useDecode ? decoded.indices() : nullptr;
     (*result)->copy(child.get(), rows, indices);
 
-    if (isString()) {
-      (*result)->as<SimpleVector<StringView>>()->copyAsciiDataFrom(
-          child.get(), rows);
-    }
 
   } else {
     if (child->encoding() == VectorEncoding::Simple::LAZY) {

diff --git a/velox/expression/Expr.cpp b/velox/expression/Expr.cpp
@@ -211,11 +211,6 @@ bool Expr::checkGetSharedSubexprValues(
     evalEncodings(*missingRows, context, &sharedSubexprValues_);
   }
   context->moveOrCopyResult(sharedSubexprValues_, rows, result);
-  // Copy string encoding
-  if (isString()) {
-    (*result)->as<SimpleVector<StringView>>()->copyAsciiDataFrom(
-        sharedSubexprValues_.get(), rows);
-  }
   return true;
 }
 

diff --git a/velox/vector/ConstantVector.h b/velox/vector/ConstantVector.h
@@ -333,11 +333,6 @@ class ConstantVector : public SimpleVector<T> {
       }
       valueVector_ = nullptr;
 
-      // Preserve string encoding
-      if constexpr (std::is_same_v<T, StringView>) {
-        SimpleVector<T>::copyAsciiDataFrom(
-            simple, SelectivityVector(simple->size()));
-      }
     }
     makeNullsBuffer();
     initialized_ = true;

diff --git a/velox/vector/SimpleVector.h b/velox/vector/SimpleVector.h
@@ -213,37 +213,18 @@ class SimpleVector : public BaseVector {
     return max_;
   }
 
-
-  /// Enabled for string vectors, sets the string encoding to match another
-  /// vector encoding
-  template <typename U = T>
-  typename std::enable_if<std::is_same<U, StringView>::value, void>::type
-  copyAsciiDataFrom(const BaseVector* vector, const SelectivityVector& rows) {
-    auto source = vector->asUnchecked<SimpleVector<StringView>>();
-    if (vector->isConstantEncoding()) {
-      resizeAsciiRows(asciiRows_, rows.end());
-      resizeAsciiRows(asciiSetRows_, rows.end());
-
-      bool valid = source->asciiRows_.isValid(0);
-
-      rows.template applyToSelected([&](auto row) {
-        asciiRows_.setValid(row, valid);
-        asciiSetRows_.setValid(row, true);
-      });
-
-    } else if (source->asciiRows_.size() > 0) {
-      resizeAsciiRows(asciiRows_, size());
-      resizeAsciiRows(asciiSetRows_, size());
-      rows.template applyToSelected([&](auto row) {
-        asciiSetRows_.setValid(row, true);
-        asciiRows_.setValid(row, source->asciiRows_.isValid(row));
-      });
+  void copy(
+      const BaseVector* source,
+      const SelectivityVector& rows,
+      const vector_size_t* toSourceRow) override {
+    BaseVector::copy(source, rows, toSourceRow);
+
+    if constexpr (std::is_same_v<T, StringView>) {
+      copyAsciiDataFrom(source, rows);
     }
-
-    asciiRows_.updateBounds();
-    asciiSetRows_.updateBounds();
   }
 
+
   void resize(vector_size_t size) override {
     VELOX_CHECK(false, "Can only resize flat vectors.");
   }
@@ -362,6 +343,37 @@ class SimpleVector : public BaseVector {
   }
 
  protected:
+
+  /// Enabled for string vectors, sets the string encoding to match another
+  /// vector encoding
+  template <typename U = T>
+  typename std::enable_if<std::is_same<U, StringView>::value, void>::type
+  copyAsciiDataFrom(const BaseVector* vector, const SelectivityVector& rows) {
+    auto source = vector->asUnchecked<SimpleVector<StringView>>();
+    if (vector->isConstantEncoding()) {
+      resizeAsciiRows(asciiRows_, rows.end());
+      resizeAsciiRows(asciiSetRows_, rows.end());
+
+      bool valid = source->asciiRows_.isValid(0);
+
+      rows.template applyToSelected([&](auto row) {
+        asciiRows_.setValid(row, valid);
+        asciiSetRows_.setValid(row, true);
+      });
+
+    } else if (source->asciiRows_.size() > 0) {
+      resizeAsciiRows(asciiRows_, size());
+      resizeAsciiRows(asciiSetRows_, size());
+      rows.template applyToSelected([&](auto row) {
+        asciiSetRows_.setValid(row, true);
+        asciiRows_.setValid(row, source->asciiRows_.isValid(row));
+      });
+    }
+
+    asciiRows_.updateBounds();
+    asciiSetRows_.updateBounds();
+  }
+
   template <typename U = T>
   typename std::enable_if<std::is_same<U, StringView>::value, void>::type
   resizeAsciiRows(