Introduce encoding::Decoder to bundle encoding factory with options

tanjialiang · facebook-github-bot · commit 454e69d791d3 · 2026-03-23T12:08:16.000-07:00
Summary:
Introduce `encoding::Decoder` class that bundles a decode function with
`Encoding::Options` into a single object. This replaces the pattern of
threading a bare `std::function` encoding factory through multiple layers
(`NimbleData`, `ChunkedDecoder`, `NimbleParams`).

Named constructors:
- `encoding::Decoder::create(Options)` — uses `EncodingFactory::decode`
- `encoding::Decoder::createLegacy(Options)` — uses `legacy::EncodingFactory::decode`

This is a pure refactoring with no behavioral change. It simplifies the
selective reader code by replacing 10+ line inline lambda ternaries with
concise `encoding::Decoder::create()` / `createLegacy()` calls, and
prepares the codebase for threading additional options (like varint row
counts) without touching every callsite.

The `Decoder` class lives in `namespace facebook::nimble::encoding` and in a
separate BUCK target (`encoding_decoder`) to avoid a circular dependency
between `encodings` and `legacy/encodings`.

Differential Revision: D97367204
diff --git a/dwio/nimble/encodings/EncodingDecoder.cpp b/dwio/nimble/encodings/EncodingDecoder.cpp
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "dwio/nimble/encodings/EncodingDecoder.h"
+
+#include "dwio/nimble/encodings/EncodingFactory.h"
+#include "dwio/nimble/encodings/legacy/EncodingFactory.h"
+
+namespace facebook::nimble::encoding {
+
+/* static */ Decoder Decoder::create(Encoding::Options options) {
+  return Decoder(
+      [options](
+          std::string_view data,
+          StringBufferFactory stringBufferFactory,
+          velox::memory::MemoryPool* pool) -> std::unique_ptr<Encoding> {
+        return EncodingFactory::decode(
+            *pool, data, std::move(stringBufferFactory), options);
+      },
+      options);
+}
+
+/* static */ Decoder Decoder::createLegacy(Encoding::Options options) {
+  return Decoder(
+      [](std::string_view data,
+         StringBufferFactory stringBufferFactory,
+         velox::memory::MemoryPool* pool) -> std::unique_ptr<Encoding> {
+        return legacy::EncodingFactory::decode(
+            *pool, data, std::move(stringBufferFactory));
+      },
+      options);
+}
+
+} // namespace facebook::nimble::encoding
diff --git a/dwio/nimble/encodings/EncodingDecoder.h b/dwio/nimble/encodings/EncodingDecoder.h
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include <functional>
+#include <memory>
+#include <string_view>
+
+#include "dwio/nimble/encodings/Encoding.h"
+
+namespace facebook::nimble::encoding {
+
+/// Bundles an encoding decode function with its options.
+///
+/// This replaces the pattern of threading a bare std::function encoding factory
+/// alongside a separate `bool useVarintRowCount` through multiple layers.
+/// Consumers can query `useVarintRowCount()` from the same object that
+/// performs decoding.
+class Decoder {
+ public:
+  using StringBufferFactory = std::function<void*(uint32_t)>;
+
+  /// Named constructors — declared here, defined in EncodingDecoder.cpp
+  /// to avoid header dependencies on EncodingFactory and
+  /// legacy::EncodingFactory.
+  static Decoder create(Encoding::Options options = {});
+  static Decoder createLegacy(Encoding::Options options = {});
+
+  std::unique_ptr<Encoding> decode(
+      std::string_view data,
+      StringBufferFactory stringBufferFactory,
+      velox::memory::MemoryPool* pool) const {
+    return decodeFunc_(data, std::move(stringBufferFactory), pool);
+  }
+
+  bool useVarintRowCount() const {
+    return options_.useVarintRowCount;
+  }
+
+ private:
+  using DecodeFunction = std::function<std::unique_ptr<Encoding>(
+      std::string_view,
+      StringBufferFactory,
+      velox::memory::MemoryPool*)>;
+
+  Decoder(DecodeFunction decodeFunc, Encoding::Options options)
+      : decodeFunc_{std::move(decodeFunc)}, options_{std::move(options)} {}
+
+  const DecodeFunction decodeFunc_;
+  const Encoding::Options options_{};
+};
+
+} // namespace facebook::nimble::encoding
diff --git a/dwio/nimble/encodings/tests/ReadWithVisitorTest.cpp b/dwio/nimble/encodings/tests/ReadWithVisitorTest.cpp
@@ -29,6 +29,7 @@
 
 #include "dwio/nimble/common/Buffer.h"
 #include "dwio/nimble/common/tests/NimbleFileWriter.h"
+#include "dwio/nimble/encodings/EncodingDecoder.h"
 #include "dwio/nimble/encodings/EncodingFactory.h"
 #include "dwio/nimble/encodings/EncodingUtils.h"
 #include "dwio/nimble/encodings/tests/EncodingLayoutTestHelper.h"
@@ -123,11 +124,7 @@ class ReadWithVisitorTest : public ::testing::Test,
         ctx.readerBase->nimbleSchema(),
         *ctx.streams,
         ctx.rowSizeTracker.get(),
-        [](memory::MemoryPool& pool,
-           std::string_view data,
-           std::function<void*(uint32_t)> sbf) -> std::unique_ptr<Encoding> {
-          return EncodingFactory::decode(pool, data, std::move(sbf));
-        });
+        encoding::Decoder::create());
 
     auto reader = buildColumnReader(
         rowType,
diff --git a/dwio/nimble/velox/selective/ChunkedDecoder.cpp b/dwio/nimble/velox/selective/ChunkedDecoder.cpp
@@ -49,14 +49,14 @@ void ChunkedDecoder::loadNextChunk() {
   inputData_ += length;
   inputSize_ -= length;
   currentStringBuffers_.clear();
-  encoding_ = encodingFactory_(
-      *pool_,
+  encoding_ = decoder_.decode(
       std::string_view(chunkData, chunkSize),
       [&](uint32_t totalLength) {
         auto& buffer = currentStringBuffers_.emplace_back(
             velox::AlignedBuffer::allocate<char>(totalLength, pool_));
         return buffer->asMutable<void>();
-      });
+      },
+      pool_);
   remainingValues_ = encoding_->rowCount();
   NIMBLE_CHECK_GT(remainingValues_, 0);
   VLOG(1) << encoding_->debugString();
diff --git a/dwio/nimble/velox/selective/ChunkedDecoder.h b/dwio/nimble/velox/selective/ChunkedDecoder.h
@@ -29,23 +29,14 @@ class ChunkedDecoder {
  public:
   ChunkedDecoder(
       std::unique_ptr<velox::dwio::common::SeekableInputStream> input,
-      bool decodeValuesWithNulls,
       std::shared_ptr<index::StreamIndex> streamIndex,
+      bool decodeValuesWithNulls,
       velox::memory::MemoryPool* pool,
-      std::function<std::unique_ptr<Encoding>(
-          velox::memory::MemoryPool&,
-          std::string_view,
-          std::function<void*(uint32_t)>)> encodingFactory =
-          [](velox::memory::MemoryPool& pool,
-             std::string_view data,
-             std::function<void*(uint32_t)> stringBufferFactory)
-          -> std::unique_ptr<Encoding> {
-        return EncodingFactory::decode(pool, data, stringBufferFactory);
-      })
+      encoding::Decoder decoder = encoding::Decoder::create())
       : input_{std::move(input)},
         pool_{pool},
         decodeValuesWithNulls_{decodeValuesWithNulls},
-        encodingFactory_{std::move(encodingFactory)},
+        decoder_{std::move(decoder)},
         streamIndex_{std::move(streamIndex)},
         streamRowCount_{
             streamIndex_ ? std::optional<uint32_t>(streamIndex_->rowCount())
@@ -418,11 +409,7 @@ class ChunkedDecoder {
   // encode nulls alongside values). When false, decode values without nulls
   // (standard case for scalar types).
   const bool decodeValuesWithNulls_;
-  const std::function<std::unique_ptr<Encoding>(
-      velox::memory::MemoryPool&,
-      std::string_view,
-      std::function<void*(uint32_t)>)>
-      encodingFactory_;
+  const encoding::Decoder decoder_;
   // Optional stream index for accelerating skip operations
   const std::shared_ptr<index::StreamIndex> streamIndex_;
   // Total row count in the stream, set from stream index if available.
diff --git a/dwio/nimble/velox/selective/FlatMapColumnReader.cpp b/dwio/nimble/velox/selective/FlatMapColumnReader.cpp
@@ -118,9 +118,10 @@ std::vector<KeyNode<T>> makeKeyNodes(
     if (inMapInput != nullptr) {
       node.inMap = std::make_unique<ChunkedDecoder>(
           std::move(inMapInput),
-          /*decodeValuesWithNulls=*/false,
           /*streamIndex=*/nullptr,
-          &memoryPool);
+          /*decodeValuesWithNulls=*/false,
+          &memoryPool,
+          params.decoder());
     } else {
       // Missing in-map stream: either the writer skipped it because all rows
       // are in-map (value streams exist), or the key has no data in this
diff --git a/dwio/nimble/velox/selective/NimbleData.cpp b/dwio/nimble/velox/selective/NimbleData.cpp
@@ -28,16 +28,13 @@ NimbleData::NimbleData(
     StripeStreams& streams,
     memory::MemoryPool& memoryPool,
     ChunkedDecoder* inMapDecoder,
-    std::function<std::unique_ptr<Encoding>(
-        velox::memory::MemoryPool&,
-        std::string_view,
-        std::function<void*(uint32_t)>)> encodingFactory,
+    encoding::Decoder decoder,
     bool getStringBuffersFromDecoder)
     : nimbleType_(nimbleType),
       streams_(&streams),
       pool_(&memoryPool),
       inMapDecoder_(inMapDecoder),
-      encodingFactory_{encodingFactory} {
+      decoder_{std::move(decoder)} {
   switch (nimbleType->kind()) {
     case Kind::Scalar:
       // Nulls in scalar types will be decoded along with values.
@@ -161,8 +158,8 @@ ChunkedDecoder NimbleData::makeScalarDecoder() {
   const auto streamId = nimbleType_->asScalar().scalarDescriptor().offset();
   return ChunkedDecoder(
       streams_->enqueue(streamId),
-      /*decodeValuesWithNulls=*/false,
       streams_->streamIndex(streamId),
+      /*decodeValuesWithNulls=*/false,
       pool_);
 }
 
@@ -172,8 +169,8 @@ ChunkedDecoder NimbleData::makeMicrosDecoder() {
       nimbleType_->asTimestampMicroNano().microsDescriptor().offset();
   return ChunkedDecoder(
       streams_->enqueue(streamId),
-      /*decodeValuesWithNulls=*/false,
       streams_->streamIndex(streamId),
+      /*decodeValuesWithNulls=*/false,
       pool_);
 }
 
@@ -183,8 +180,8 @@ ChunkedDecoder NimbleData::makeNanosDecoder() {
       nimbleType_->asTimestampMicroNano().nanosDescriptor().offset();
   return ChunkedDecoder(
       streams_->enqueue(streamId),
-      /*decodeValuesWithNulls=*/false,
       streams_->streamIndex(streamId),
+      /*decodeValuesWithNulls=*/false,
       pool_);
 }
 
@@ -211,8 +208,8 @@ std::unique_ptr<ChunkedDecoder> NimbleData::makeDecoder(
   }
   return std::make_unique<ChunkedDecoder>(
       std::move(input),
-      decodeValuesWithNulls,
       streams_->streamIndex(descriptor.offset()),
+      decodeValuesWithNulls,
       pool_);
 }
 
@@ -224,7 +221,7 @@ std::unique_ptr<velox::dwio::common::FormatData> NimbleParams::toFormatData(
       *streams_,
       pool(),
       inMapDecoder_,
-      encodingFactory_,
+      decoder_,
       getStringBuffersFromDecoder_);
 }
 
diff --git a/dwio/nimble/velox/selective/NimbleData.h b/dwio/nimble/velox/selective/NimbleData.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include "dwio/nimble/encodings/EncodingDecoder.h"
 #include "dwio/nimble/encodings/EncodingFactory.h"
 #include "dwio/nimble/velox/selective/ReaderBase.h"
 #include "dwio/nimble/velox/selective/RowSizeTracker.h"
@@ -32,10 +33,7 @@ class NimbleData : public velox::dwio::common::FormatData {
       StripeStreams& streams,
       velox::memory::MemoryPool& memoryPool,
       ChunkedDecoder* inMapDecoder,
-      std::function<std::unique_ptr<Encoding>(
-          velox::memory::MemoryPool&,
-          std::string_view,
-          std::function<void*(uint32_t)>)> encodingFactory,
+      encoding::Decoder decoder,
       bool getStringBuffersFromDecoder);
 
   /// Read internal node nulls. For leaf nodes, we only copy `incomingNulls' if
@@ -110,11 +108,7 @@ class NimbleData : public velox::dwio::common::FormatData {
   ChunkedDecoder* const inMapDecoder_;
   std::unique_ptr<ChunkedDecoder> nullsDecoder_;
   velox::BufferPtr inMap_;
-  std::function<std::unique_ptr<Encoding>(
-      velox::memory::MemoryPool&,
-      std::string_view,
-      std::function<void*(uint32_t)>)>
-      encodingFactory_;
+  encoding::Decoder decoder_;
 };
 
 class NimbleParams : public velox::dwio::common::FormatParams {
@@ -125,18 +119,15 @@ class NimbleParams : public velox::dwio::common::FormatParams {
       const std::shared_ptr<const Type>& nimbleType,
       StripeStreams& streams,
       RowSizeTracker* rowSizeTracker,
-      std::function<std::unique_ptr<Encoding>(
-          velox::memory::MemoryPool&,
-          std::string_view,
-          std::function<void*(uint32_t)>)> encodingFactory,
+      encoding::Decoder decoder,
       bool getStringBuffersFromDecoder = false,
       bool preserveFlatMapsInMemory = false)
       : FormatParams(pool, stats),
         nimbleType_(nimbleType),
         streams_(&streams),
         rowSizeTracker_(rowSizeTracker),
         preserveFlatMapsInMemory_(preserveFlatMapsInMemory),
-        encodingFactory_(std::move(encodingFactory)),
+        decoder_(std::move(decoder)),
         getStringBuffersFromDecoder_{getStringBuffersFromDecoder} {}
 
   std::unique_ptr<velox::dwio::common::FormatData> toFormatData(
@@ -150,7 +141,7 @@ class NimbleParams : public velox::dwio::common::FormatParams {
         type,
         *streams_,
         rowSizeTracker_,
-        encodingFactory_,
+        decoder_,
         getStringBuffersFromDecoder_,
         preserveFlatMapsInMemory_);
   }
@@ -175,17 +166,17 @@ class NimbleParams : public velox::dwio::common::FormatParams {
     return rowSizeTracker_;
   }
 
+  const encoding::Decoder& decoder() const {
+    return decoder_;
+  }
+
  private:
   const std::shared_ptr<const Type> nimbleType_;
   StripeStreams* const streams_{nullptr};
   RowSizeTracker* const rowSizeTracker_{nullptr};
   const bool preserveFlatMapsInMemory_{false};
   ChunkedDecoder* inMapDecoder_{nullptr};
-  std::function<std::unique_ptr<Encoding>(
-      velox::memory::MemoryPool&,
-      std::string_view,
-      std::function<void*(uint32_t)> stringBufferFactory)>
-      encodingFactory_;
+  encoding::Decoder decoder_;
   bool getStringBuffersFromDecoder_{false};
 };
 
diff --git a/dwio/nimble/velox/selective/SelectiveNimbleIndexReader.cpp b/dwio/nimble/velox/selective/SelectiveNimbleIndexReader.cpp
@@ -18,8 +18,8 @@
 
 #include <utility>
 
+#include "dwio/nimble/encodings/EncodingDecoder.h"
 #include "dwio/nimble/encodings/EncodingFactory.h"
-#include "dwio/nimble/encodings/legacy/EncodingFactory.h"
 #include "dwio/nimble/index/IndexReader.h"
 #include "dwio/nimble/velox/SchemaUtils.h"
 #include "dwio/nimble/velox/selective/ColumnReader.h"
@@ -571,19 +571,8 @@ void SelectiveNimbleIndexReader::loadStripeWithIndex(uint32_t stripeIndex) {
       readerBase_->nimbleSchema(),
       streams_,
       options_.trackRowSize() ? rowSizeTracker_.get() : nullptr,
-      options_.passStringBuffersFromDecoder()
-          ? [](velox::memory::MemoryPool& pool,
-               std::string_view data,
-               std::function<void*(uint32_t)> stringBufferFactory)
-                -> std::unique_ptr<Encoding> {
-        return EncodingFactory::decode(pool, data, std::move(stringBufferFactory));
-      }
-          : [](velox::memory::MemoryPool& pool,
-               std::string_view data,
-               std::function<void*(uint32_t)> stringBufferFactory)
-                -> std::unique_ptr<Encoding> {
-        return legacy::EncodingFactory::decode(pool, data, std::move(stringBufferFactory));
-      },
+      options_.passStringBuffersFromDecoder() ? encoding::Decoder::create()
+                                              : encoding::Decoder::createLegacy(),
       options_.passStringBuffersFromDecoder(),
       options_.preserveFlatMapsInMemory());
 
diff --git a/dwio/nimble/velox/selective/SelectiveNimbleReader.cpp b/dwio/nimble/velox/selective/SelectiveNimbleReader.cpp
diff --git a/dwio/nimble/velox/selective/tests/ChunkedDecoderTest.cpp b/dwio/nimble/velox/selective/tests/ChunkedDecoderTest.cpp
