Remove incorrect check from MapWriter::initialize (facebookincubator#…

…9598)

Summary:
Pull Request resolved: facebookincubator#9598

MapWriter::initialize incorrectly assumes that the sizes of the keys and values vectors in a MapVector must be the same.

Fixes facebookincubator#9497

Reviewed By: bikramSingh91

Differential Revision: D56515546

fbshipit-source-id: 28ffd0ed3e068db033803aad6c078c42c8049b4b

velox/expression/ComplexWriterTypes.h

@@ -765,18 +765,15 @@ class MapWriter {
     keysVector_ = &keysWriter_->vector();
     valuesVector_ = &valuesWriter_->vector();
 
-    innerOffset_ = std::max(keysVector_->size(), valuesVector_->size());
+    innerOffset_ = std::min(keysVector_->size(), valuesVector_->size());
 
     // Keys can never be null.
     keysVector_->resetNulls();
 
     keysWriter_->ensureSize(1);
     valuesWriter_->ensureSize(1);
 
-    VELOX_DCHECK(
-        keysVector_->size() == valuesVector_->size(),
-        "expect map keys and value vector sized to be synchronized");
-    capacity_ = keysVector_->size();
+    capacity_ = std::min(keysVector_->size(), valuesVector_->size());
   }
 
   key_element_t& lastKeyWriter() {

velox/expression/tests/MapWriterTest.cpp

@@ -645,6 +645,64 @@ TEST_F(MapWriterTest, appendToKeysAndValues) {
   ASSERT_EQ(values->asFlatVector<int64_t>()->valueAt(1), 20);
 }
 
+// Make sure MapWriter correctly handles 'result' MapVector, where keys and
+// values child vectors have different sizes.
+TEST_F(MapWriterTest, differentKeyValueSizes) {
+  using out_t = Map<int32_t, int32_t>;
+  auto mapType = CppToType<out_t>::create();
+
+  // Keys vector is shorter than values vector.
+  auto result = std::make_shared<MapVector>(
+      pool(),
+      mapType,
+      nullptr, // no nulls
+      3,
+      makeIndices({0, 2, 4}), // offsets
+      makeIndices({2, 2, 2}), // sizes
+      makeFlatVector<int32_t>({0, 1, 0, 1, 0, 1}), // keys
+      makeFlatVector<int32_t>({0, 1, 2, 3, 4, 5, 6, 7, 8, 9}) // values
+  );
+
+  EXPECT_EQ(6, result->mapKeys()->size());
+  EXPECT_EQ(10, result->mapValues()->size());
+
+  {
+    // Write map at offset 0.
+    exec::VectorWriter<out_t> vectorWriter;
+    vectorWriter.init(*result);
+    vectorWriter.setOffset(0);
+    auto& mapWriter = vectorWriter.current();
+    mapWriter.copy_from(folly::F14FastMap<int64_t, int64_t>{{1, 2}});
+
+    vectorWriter.commit();
+    vectorWriter.finish();
+  }
+
+  {
+    // Write map at offset 2.
+    exec::VectorWriter<out_t> vectorWriter;
+    vectorWriter.init(*result);
+    vectorWriter.setOffset(2);
+    auto& mapWriter = vectorWriter.current();
+    mapWriter.copy_from(folly::F14FastMap<int64_t, int64_t>{{3, 4}});
+
+    vectorWriter.commit();
+    vectorWriter.finish();
+  }
+
+  // Verify sizes of keys and values vectors.
+  EXPECT_EQ(8, result->mapKeys()->size());
+  EXPECT_EQ(8, result->mapValues()->size());
+
+  auto expected = makeMapVector<int32_t, int32_t>({
+      {{1, 2}},
+      {{0, 2}, {1, 3}},
+      {{3, 4}},
+  });
+
+  assertEqualVectors(expected, result);
+}
+
 // Make sure copy from MapView correctly resizes children vectors.
 TEST_F(MapWriterTest, copyFromViewTypeResizedChildren) {
   using out_t = Map<int64_t, Map<int64_t, int64_t>>;