Skip to content

Commit 40180ad

Browse files
xiaoxmengmeta-codesync[bot]
authored andcommitted
refactor: Remove NimbleRowReaderOptions, use RowReaderOptions::setIndexEnabled() (facebookincubator#443)
Summary: Pull Request resolved: facebookincubator#443 The `NimbleRowReaderOptions` class was only used to control the `indexEnabled` flag for Nimble row readers. Since `RowReaderOptions` already has `setIndexEnabled()` and `indexEnabled()` methods, we can simplify the code by removing the Nimble-specific options class and using the common `RowReaderOptions` API directly. This change: - Removes the `NimbleRowReaderOptions` class and its header file - Updates `SelectiveNimbleReader` to read `indexEnabled` directly from `RowReaderOptions` - Updates all call sites to use `rowReaderOptions.setIndexEnabled()` instead of creating `NimbleRowReaderOptions` Reviewed By: zzhao0 Differential Revision: D91557015 fbshipit-source-id: c574651eb5e840bacf1bb86b7a081cdb8d845a8e
1 parent 14252ab commit 40180ad

File tree

3 files changed

+6
-61
lines changed

3 files changed

+6
-61
lines changed

dwio/nimble/velox/selective/NimbleRowReaderOptions.h

Lines changed: 0 additions & 46 deletions
This file was deleted.

dwio/nimble/velox/selective/SelectiveNimbleReader.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
#include "dwio/nimble/velox/SchemaUtils.h"
2424
#include "dwio/nimble/velox/selective/ColumnReader.h"
25-
#include "dwio/nimble/velox/selective/NimbleRowReaderOptions.h"
2625
#include "dwio/nimble/velox/selective/ReaderBase.h"
2726
#include "dwio/nimble/velox/selective/RowSizeTracker.h"
2827
#include "velox/common/base/RuntimeMetrics.h"
@@ -384,12 +383,9 @@ void SelectiveNimbleRowReader::initIndexBounds() {
384383
return;
385384
}
386385

387-
// Check if index filtering is disabled via format-specific options
388-
if (auto nimbleOptions = std::dynamic_pointer_cast<NimbleRowReaderOptions>(
389-
options_.formatSpecificOptions())) {
390-
if (!nimbleOptions->indexEnabled()) {
391-
return;
392-
}
386+
// Check if index filtering is disabled.
387+
if (!options_.indexEnabled()) {
388+
return;
393389
}
394390

395391
// Verify that the file has a cluster index

dwio/nimble/velox/selective/tests/E2EIndexTest.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
#include "dwio/nimble/encodings/PrefixEncoding.h"
2020
#include "dwio/nimble/velox/VeloxWriter.h"
21-
#include "dwio/nimble/velox/selective/NimbleRowReaderOptions.h"
2221
#include "dwio/nimble/velox/selective/SelectiveNimbleReader.h"
2322
#include "velox/common/base/RandomUtil.h"
2423
#include "velox/common/base/RuntimeMetrics.h"
@@ -298,10 +297,8 @@ class E2EIndexTestBase : public ::testing::Test {
298297
auto scanSpec = createScanSpec(requestedType, filtersCopy);
299298
rowReaderOptions.setScanSpec(std::move(scanSpec));
300299

301-
// Set format-specific options to control index.
302-
auto nimbleOptions = std::make_shared<NimbleRowReaderOptions>();
303-
nimbleOptions->setIndexEnabled(indexEnabled);
304-
rowReaderOptions.setFormatSpecificOptions(std::move(nimbleOptions));
300+
// Set index enabled option.
301+
rowReaderOptions.setIndexEnabled(indexEnabled);
305302

306303
// Set up a stat writer to capture runtime stats.
307304
TestRuntimeStatWriter statWriter;
@@ -3268,9 +3265,7 @@ TEST_P(E2EIndexTest, filterRestorationAcrossMultipleSplits) {
32683265
RowReaderOptions rowReaderOptions;
32693266
rowReaderOptions.setRequestedType(rowType);
32703267
rowReaderOptions.setScanSpec(scanSpec);
3271-
auto nimbleOptions = std::make_shared<NimbleRowReaderOptions>();
3272-
nimbleOptions->setIndexEnabled(true);
3273-
rowReaderOptions.setFormatSpecificOptions(std::move(nimbleOptions));
3268+
rowReaderOptions.setIndexEnabled(true);
32743269

32753270
auto rowReader = reader->createRowReader(rowReaderOptions);
32763271

0 commit comments

Comments
 (0)