StringWriter needs to implement finalizeNull

velox/expression/ComplexWriterTypes.h

@@ -50,6 +50,8 @@ class VectorWriterBase {
   virtual void commit(bool isSet) = 0;
   virtual void ensureSize(size_t size) = 0;
   virtual void finish() {}
+  // Implementations that write variable length data or complex types should
+  // override this to reset their state and that of their children.
   virtual void finalizeNull() {}
   virtual ~VectorWriterBase() {}
   vector_size_t offset_ = 0;

velox/expression/VectorWriters.h

@@ -373,16 +373,21 @@ struct VectorWriter<
 
   void commitNull() {
     proxy_.vector_->setNull(proxy_.offset_, true);
+    finalizeNull();
+  }
+
+  void finalizeNull() override {
+    proxy_.prepareForReuse(false);
   }
 
   void commit(bool isSet) override {
     // this code path is called when the slice is top-level
     if (isSet) {
       proxy_.finalize();
+      proxy_.prepareForReuse(true);
     } else {
       commitNull();
     }
-    proxy_.prepareForReuse(isSet);
   }
 
   void setOffset(vector_size_t offset) override {

velox/expression/tests/StringWriterTest.cpp

@@ -18,9 +18,11 @@
 #include <glog/logging.h>
 #include "folly/Range.h"
 #include "gtest/gtest.h"
+#include "velox/expression/VectorWriters.h"
 #include "velox/functions/prestosql/tests/utils/FunctionBaseTest.h"
 
 namespace facebook::velox::expressions::test {
+using namespace facebook::velox::test;
 
 class StringWriterTest : public functions::test::FunctionBaseTest {};
 
@@ -103,4 +105,26 @@ TEST_F(StringWriterTest, copyFromCString) {
 
   ASSERT_EQ(vector->valueAt(0), "1 2 3 4 5 "_sv);
 }
+
+TEST_F(StringWriterTest, vectorWriter) {
+  auto vector = makeFlatVector<StringView>(3);
+  exec::VectorWriter<Varchar> writer;
+  writer.init(*vector);
+  writer.setOffset(0);
+  writer.current().copy_from("1 2 3");
+  writer.commitNull();
+
+  writer.setOffset(1);
+  writer.current().copy_from("4 5 6");
+  writer.commit(true);
+
+  writer.setOffset(2);
+  writer.current().copy_from("7 8 9");
+  writer.commit(false);
+  writer.finish();
+
+  auto expected = std::vector<std::optional<std::string>>{
+      std::nullopt, "4 5 6", std::nullopt};
+  assertEqualVectors(vector, makeNullableFlatVector(expected));
+}
 } // namespace facebook::velox::expressions::test