fix(nimble): Keep nulls in dictionary reads without AVX2 (facebookincubator#18621)

Summary:

Reading a Nimble string column that keeps its dictionary encoding dropped every null on any platform where `process::hasAvx2()` is false, which is all of aarch64. A 200-row column holding `always_the_same` with a null every 7th row came back as a `DictionaryVector` carrying no null flags, so row 0 read as `always_the_same` instead of null.

The dense dictionary-index path wrote nulls only into the reader's read-range bitmap. It then relied on `returnReaderNulls_` to hand that bitmap back from `resultNulls()`. `setReturnNullsMode` clears that flag whenever `useBulkPath()` is false, and `process::hasAvx2()` makes that permanent off AVX2. `resultNulls()` then returns the output-indexed buffer instead, which nothing on this path filled. It now copies the read-range nulls into that buffer, the way the sparse row-set path already did.

Reviewed By: vandreykiv

Differential Revision: D116833280

Author: Yuhta

velox/dwio/nimble/encodings/common/Encoding.h

@@ -527,6 +527,25 @@ void readDenseMaterializedIndices(
       /*sourceBegin=*/valueOutputOffset,
       rawOuterNonNullRows,
       rawOutputValues);
+
+  // Nulls were materialized into the reader's read-range bitmap only.
+  // `resultNulls()' hands that bitmap back while `returnReaderNulls_' holds,
+  // but `setReturnNullsMode' clears the flag whenever `useBulkPath()' is false
+  // -- notably on a platform without AVX2, where every read takes that branch.
+  // `resultNulls()' then returns the output-indexed `resultNulls_', which
+  // nothing on this path writes, so the output would silently lose its nulls.
+  // Copy the read-range nulls across, mirroring what
+  // `readSparseMaterializedIndices' does for the sparse row set.
+  if (!visitor.reader().returnReaderNulls()) {
+    auto* rawResultNulls = visitor.reader().rawResultNulls();
+    NIMBLE_CHECK_NOT_NULL(
+        rawResultNulls,
+        "prepareResultNulls must allocate result nulls before the dense index "
+        "path writes them");
+    velox::bits::copyBits(
+        rawNulls, readOffset, rawResultNulls, valueOutputOffset, numReadRows);
+    visitor.reader().setHasNulls();
+  }
   visitor.addNumValues(numReadRows);
   visitor.setRowIndex(visitor.numRows());
 }

velox/dwio/nimble/encodings/tests/ReadWithVisitorTest.cpp

@@ -4256,9 +4256,16 @@ TEST_P(ReadWithVisitorNonLegacyTest, readDenseMaterializedIndicesWithNulls) {
 
   ASSERT_EQ(reader->numValues(), 0);
 
-  // Call the helper with nulls.
+  // Call the helper with nulls. prepareResultNulls must mirror what
+  // ChunkedDecoder wires up in production: the dense index path materializes
+  // nulls into the read-range bitmap only, so it needs an allocated,
+  // output-indexed result-nulls buffer to copy them into whenever
+  // returnReaderNulls_ is false -- as it is here, the scan spec carries a
+  // filter.
   ReadWithVisitorParams params{.numScanned = 0};
-  params.prepareResultNulls = [] {};
+  params.prepareResultNulls = [&] {
+    reader->prepareNulls(rows, /*hasNulls=*/true, /*extraRows=*/8);
+  };
   detail::readDenseMaterializedIndices(
       *encoding,
       visitor,