AntoinePrv commented on code in PR #50629:
URL: https://github.com/apache/arrow/pull/50629#discussion_r4334620844
##########
cpp/src/parquet/column_reader_test.cc:
##########
@@ -643,19 +650,60 @@ internal::LevelInfo ComputeLevelInfo(const
ColumnDescriptor* descr) {
} // namespace
-using ReadDenseForNullable = bool;
+struct RecordReaderTestParam {
+ bool read_dense_for_nullable;
+ bool allow_levels_optimization;
+};
+
+std::vector<RecordReaderTestParam> RecordReaderTestParams(
+ std::initializer_list<bool> allow_levels_optimizations) {
+ std::vector<RecordReaderTestParam> params;
+ for (bool read_dense_for_nullable : {true, false}) {
+ for (bool allow_levels_optimization : allow_levels_optimizations) {
+ params.push_back({
+ .read_dense_for_nullable = read_dense_for_nullable,
+ .allow_levels_optimization = allow_levels_optimization,
+ });
+ }
+ }
+ return params;
+}
+
+std::string RecordReaderTestParamName(
+ const testing::TestParamInfo<RecordReaderTestParam>& info) {
+ return std::string(info.param.read_dense_for_nullable ? "Dense" : "Spaced") +
+ (info.param.allow_levels_optimization ? "LevelsOptimization"
+ : "NoLevelsOptimization");
+}
+
class RecordReaderPrimitiveTypeTest
- : public ::testing::TestWithParam<ReadDenseForNullable> {
+ : public ::testing::TestWithParam<RecordReaderTestParam> {
public:
const int32_t kNullValue = -1;
+ bool read_dense_for_nullable() const { return
GetParam().read_dense_for_nullable; }
+
+ bool allow_levels_optimization() const { return
GetParam().allow_levels_optimization; }
+
void Init(NodePtr column) {
NodePtr root = GroupNode::Make("root", Repetition::REQUIRED, {column});
schema_descriptor_.Init(root);
descr_ = schema_descriptor_.Column(0);
- record_reader_ = internal::RecordReader::Make(descr_,
ComputeLevelInfo(descr_),
-
::arrow::default_memory_pool(),
- /*read_dictionary=*/false,
GetParam());
+ record_reader_ = internal::RecordReader::Make({
+ .descr = descr_,
+ .leaf_info = ComputeLevelInfo(descr_),
+ .read_dense_for_nullable = read_dense_for_nullable(),
+ .allow_levels_optimization = allow_levels_optimization(),
+ });
+ }
+
+ // Whether the record reader for this column exposes definition and
repetition
+ // levels. The flat optional reader decodes definition levels straight into
the
+ // validity bitmap of the records it produces, so it never materializes
levels and
+ // reports none: `def_levels()` is null and both level counters stay at zero.
+ bool exposes_levels() const {
Review Comment:
This is a bit tailored to the test. In an API, we should make a distinction
between def/rep levels and perhaps another distinction between "does not
expose because there aren't" and "does not expose because they are optimized
away"
Initially, I had left `RecordReader::def_levels` as `noexcept` so this could
be `reader.def_levels() != nullptr` in all cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
