Skip to content

Commit 084f222

Browse files
rui-mometa-codesync[bot]
authored andcommitted
misc: Make DirectBufferedInput clone fields protected (facebookincubator#16979)
Summary: `StructColumnReader::loadRowGroup` clones the buffered input. To keep using custom `DirectBufferedInput` after cloning, we need to override clone, which requires changing some fields from private to protected. https://github.com/facebookincubator/velox/blob/22b90045ef5737c737593fbedde1f6f4f64d6dc5/velox/dwio/parquet/reader/StructColumnReader.cpp#L124 Pull Request resolved: facebookincubator#16979 Reviewed By: Yuhta Differential Revision: D99354261 Pulled By: bikramSingh91 fbshipit-source-id: 30df4d5bfb93d7217cd573f1fb0bda23c84bdbd9
1 parent d9c1b6e commit 084f222

File tree

2 files changed

+21
-13
lines changed

2 files changed

+21
-13
lines changed

velox/dwio/common/DirectBufferedInput.h

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,14 @@ class DirectBufferedInput : public BufferedInput {
239239
protected:
240240
// Some members are protected to allow custom extended buffered inputs.
241241
// Regions that are candidates for loading.
242+
const StringIdLease fileNum_;
243+
const std::shared_ptr<cache::ScanTracker> tracker_;
244+
const StringIdLease groupId_;
245+
const std::shared_ptr<IoStatistics> ioStatistics_;
246+
const std::shared_ptr<velox::IoStats> ioStats_;
247+
folly::Executor* const executor_;
248+
const uint64_t fileSize_;
249+
const io::ReaderOptions options_;
242250
std::vector<LoadRequest> requests_;
243251

244252
// Distinct coalesced loads in 'coalescedLoads_'.
@@ -301,15 +309,6 @@ class DirectBufferedInput : public BufferedInput {
301309
}
302310
};
303311

304-
const StringIdLease fileNum_;
305-
const std::shared_ptr<cache::ScanTracker> tracker_;
306-
const StringIdLease groupId_;
307-
const std::shared_ptr<IoStatistics> ioStatistics_;
308-
const std::shared_ptr<velox::IoStats> ioStats_;
309-
folly::Executor* const executor_;
310-
const uint64_t fileSize_;
311-
const io::ReaderOptions options_;
312-
313312
// Coalesced loads spanning multiple streams in one IO.
314313
folly::Synchronized<folly::F14FastMap<
315314
const SeekableInputStream*,

velox/dwio/common/tests/BufferedInputTest.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -692,12 +692,21 @@ class CustomDirectBufferedInput
692692
executor,
693693
readerOptions,
694694
std::move(fileReadOps)) {
695-
// Dummy reserve to ensure the accessibility of protected members in
696-
// DirectBufferedInput.
697-
requests_.reserve(1);
698-
coalescedLoads_.reserve(1);
699695
VELOX_NYI("Not implemented in CustomBufferedInputBuilder");
700696
}
697+
698+
protected:
699+
// Expose protected members to verify their accessibility.
700+
using facebook::velox::dwio::common::DirectBufferedInput::coalescedLoads_;
701+
using facebook::velox::dwio::common::DirectBufferedInput::executor_;
702+
using facebook::velox::dwio::common::DirectBufferedInput::fileNum_;
703+
using facebook::velox::dwio::common::DirectBufferedInput::fileSize_;
704+
using facebook::velox::dwio::common::DirectBufferedInput::groupId_;
705+
using facebook::velox::dwio::common::DirectBufferedInput::ioStatistics_;
706+
using facebook::velox::dwio::common::DirectBufferedInput::ioStats_;
707+
using facebook::velox::dwio::common::DirectBufferedInput::options_;
708+
using facebook::velox::dwio::common::DirectBufferedInput::requests_;
709+
using facebook::velox::dwio::common::DirectBufferedInput::tracker_;
701710
};
702711

703712
class CustomBufferedInputBuilder

0 commit comments

Comments
 (0)