github-actions[bot] commented on code in PR #66412:
URL: https://github.com/apache/doris/pull/66412#discussion_r3710912502
##########
be/src/format_v2/parquet/reader/native_column_reader.cpp:
##########
@@ -292,13 +292,18 @@ Status NativeColumnReader::create(
}
auto logical_type = projected_type(column_schema, projection, false);
- auto native_type = projected_type(column_schema, projection, true);
+ auto native_type = logical_type;
+ std::unique_ptr<VariantMaterializationNode> variant_plan;
+ if (column_schema.contains_variant) {
Review Comment:
[P2] Keep the projected Variant guard
`ParquetColumnSchema::contains_variant` describes the full root, not the
requested projection. For a `STRUCT<label INT, payload VARIANT>` scan
projecting only `label`, `build_variant_plan()` visits only `label` and
produces `contains_variant=false`, but this code still retains a non-null plan.
That allocates the physical scratch column for each Row-Group and routes every
selected batch through `materialize_variant_columns()`, recursively rebuilding
and validating ordinary wrappers even though no selected node is Variant. The
old `plan->contains_variant` guards skipped this work. Please discard/ignore a
projection-aware plan whose flag is false and add an ordinary-sibling-only
projection regression.
##########
be/benchmark/parquet/benchmark_parquet_reader.hpp:
##########
@@ -408,6 +448,75 @@ inline VExprContextSPtr
make_complex_residual_predicate(int selectivity_percent,
return VExprContext::create_shared(std::move(compound));
}
+inline VExprSPtr make_compound_predicate(TExprOpcode::type opcode, VExprSPtr
left,
+ VExprSPtr right) {
+ const auto bool_type = make_nullable(std::make_shared<DataTypeUInt8>());
+ TExprNode node;
+ node.__set_node_type(TExprNodeType::COMPOUND_PRED);
+ node.__set_opcode(opcode);
+ node.__set_type(bool_type->to_thrift());
+ node.__set_num_children(2);
+ node.__set_is_nullable(true);
+ auto compound = VCompoundPred::create_shared(node);
+ compound->add_child(std::move(left));
+ compound->add_child(std::move(right));
+ return compound;
+}
+
+inline VExprContextSPtr make_multi_column_or_predicate(const ReaderScenario&
scenario,
+ const std::array<int,
3>& positions,
+ const DataTypePtr&
value_type) {
+ const auto literal = [&](int value) -> VExprSPtr {
+ if (scenario.value_type == ValueType::DECIMAL64) {
+ return VLiteral::create_shared(
+ remove_nullable(value_type),
+ Field::create_field<TYPE_DECIMAL64>(Decimal64 {value *
100}));
+ }
+ return VLiteral::create_shared(remove_nullable(value_type),
+ Field::create_field<TYPE_INT>(value));
+ };
+ const auto make_between = [&](int position) {
+ auto slot = [&] {
+ return VSlotRef::create_shared(position, position, -1, value_type,
+ "c" + std::to_string(position));
+ };
+ auto lower = make_int32_comparison("ge", TExprOpcode::GE, slot(),
literal(0));
+ auto upper = make_int32_comparison("lt", TExprOpcode::LT, slot(),
+
literal(scenario.selectivity_percent));
+ return make_compound_predicate(TExprOpcode::COMPOUND_AND,
std::move(lower),
+ std::move(upper));
+ };
+ auto first_two = make_compound_predicate(TExprOpcode::COMPOUND_OR,
make_between(positions[0]),
+ make_between(positions[1]));
+ return VExprContext::create_shared(make_compound_predicate(
+ TExprOpcode::COMPOUND_OR, std::move(first_two),
make_between(positions[2])));
+}
+
+inline VExprContextSPtr make_multi_column_dnf_predicate(const ReaderScenario&
scenario,
+ const std::array<int,
2>& positions,
+ const DataTypePtr&
value_type) {
+ const auto literal = [&](int value) {
+ return VLiteral::create_shared(remove_nullable(value_type),
+ Field::create_field<TYPE_INT>(value));
+ };
+ const auto make_branch = [&](int category) {
+ auto category_match = make_int32_comparison(
+ "eq", TExprOpcode::EQ,
+ VSlotRef::create_shared(positions[0], positions[0], -1,
value_type, "c0"),
+ literal(category));
+ auto capacity_match = make_int32_comparison(
+ "lt", TExprOpcode::LT,
Review Comment:
[P2] Make the DNF benchmark branch-sensitive
The fixture emits only categories `row % 3`, while every branch uses the
identical `c1 < selectivity` bound, so `(c0=0 AND p) OR (c0=1 AND p) OR (c0=2
AND p)` reduces to `p` for every non-NULL row. An incorrect executor that
discards branch identity and ANDs one Boolean per column therefore produces the
same cardinality here. The focused unit test covers a separate fixed truth
table, but this matrix spans the encoding/NULL/selectivity axes and
`scan_reader()` only sums row counts, so equal-cardinality row swaps or payload
corruption still pass. Please use distinct per-category bounds with
cross-branch counterexamples and validate an expected row-id/payload checksum
plus legacy/raw equality outside the timed region.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
