github-actions[bot] commented on code in PR #66149:
URL: https://github.com/apache/doris/pull/66149#discussion_r3664882722
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -1725,22 +1840,77 @@ Status ColumnChunkReader<IN_COLLECTION,
OFFSET_INDEX>::filter_fixed_width_values
selection.total_values);
}
+ VExprSPtrs raw_conjuncts;
+ VExprSPtrs null_map_conjuncts;
+ for (const auto& conjunct : conjuncts) {
+ if (conjunct->can_execute_on_null_map(_field_schema->data_type,
column_id)) {
+ null_map_conjuncts.push_back(conjunct);
+ } else {
+ raw_conjuncts.push_back(conjunct);
+ }
+ }
physical_matches->clear();
+ IColumn::Filter physical_conversion_nulls;
if (selection.selected_values == 0) {
RETURN_IF_ERROR(_page_decoder->skip_values(selection.total_values));
+ } else if (raw_conjuncts.empty()) {
+ RETURN_IF_ERROR(_page_decoder->skip_values(selection.total_values));
Review Comment:
Predicate-only NULL evaluation cannot skip the payload here. An optional
INT32 DATE containing INT32_MAX is converted to NULL by ordinary non-strict
materialization (so IS NULL matches), while strict mode returns
DataQualityError. This branch sees only a present definition level, treats the
row as non-NULL, and suppresses the strict error. Please either feed SerDe
conversion failures into `selected_nulls` or restrict the definition-only fast
path to types whose non-NULL payload cannot fail conversion, with
strict/non-strict differential tests.
##########
be/src/exprs/vcompound_pred.h:
##########
@@ -67,6 +67,52 @@ class VCompoundPred : public VectorizedFnCall {
return Status::OK();
}
+ bool can_execute_on_raw_fixed_values(const DataTypePtr& data_type,
+ int column_id) const override {
+ return !_children.empty() &&
+ (_op == TExprOpcode::COMPOUND_AND || _op ==
TExprOpcode::COMPOUND_OR ||
+ (_op == TExprOpcode::COMPOUND_NOT && _children.size() == 1)) &&
Review Comment:
`COMPOUND_NOT` is not safe on this raw ABI because the decoder never
evaluates its child for physical NULL rows. For example, `NOT(nullable_col <=>
5)` must keep a NULL row (`NULL <=> 5` is false), but the reader never sends
that row to the raw kernel and remaps it to false before NOT can apply. Keep
NOT residual unless the reader can evaluate the expression's truth on physical
NULLs, and add nullable fixed/binary coverage.
##########
be/src/core/data_type_serde/data_type_datetimev2_serde.cpp:
##########
@@ -710,6 +742,24 @@ Status
DataTypeDateTimeV2SerDe::read_column_from_parquet(IColumn& column,
return state.materialize_dictionary(column, source, num_values);
}
+bool DataTypeDateTimeV2SerDe::supports_parquet_raw_predicate(
+ const ParquetDecodeContext& context) const {
+ return context.encoding != ParquetValueEncoding::DICTIONARY &&
+ (context.physical_type == ParquetPhysicalType::INT64 ||
+ context.physical_type == ParquetPhysicalType::INT96) &&
+ context.logical_type == ParquetLogicalType::TIMESTAMP;
+}
+
+Status DataTypeDateTimeV2SerDe::read_parquet_raw_predicate(
+ ParquetDecodeSource& source, const ParquetDecodeContext& context,
size_t num_values,
+ bool enable_strict_mode, ParquetLogicalValueConsumer& consumer) const {
+ if (!supports_parquet_raw_predicate(context)) {
+ return Status::NotSupported("Unsupported Parquet raw predicate
conversion for DATETIMEV2");
+ }
+ DateTimeV2PredicateParquetConsumer predicate_consumer(context,
enable_strict_mode, consumer);
Review Comment:
This consumer owns `_logical_values` and `_conversion_nulls`, but is created
and destroyed for every read-range/page fragment. The same pattern exists in
DATE/NUMBER/DECIMAL/TIME/TIMESTAMPTZ, and the reader also creates a second
local conversion-null filter; a max-size Decimal256 fragment is about 2 MiB of
POD scratch plus null buffers repeatedly allocated. The native-reader contract
requires persistent bounded high-water scratch. Please move these buffers into
persistent leaf-reader/caller-owned state and add reuse/retained-capacity
coverage.
##########
be/src/format_v2/parquet/parquet_scan.cpp:
##########
@@ -1452,6 +1455,54 @@ bool get_typed_dictionary_raw_values(PrimitiveType
primitive_type, const IColumn
}
}
+Status try_apply_runtime_filters_to_dictionary(size_t block_position,
+ const ParquetColumnSchema&
column_schema,
+ const VExprContextSPtrs&
conjuncts,
+ const IColumn& dictionary,
+ IColumn::Filter*
dictionary_filter, bool* applied) {
+ DORIS_CHECK(dictionary_filter != nullptr);
+ DORIS_CHECK(applied != nullptr);
+ *applied = false;
+ if (!std::ranges::all_of(conjuncts, [](const auto& conjunct) {
+ return conjunct != nullptr && conjunct->root() != nullptr &&
+ conjunct->root()->is_rf_wrapper() &&
+ conjunct->root()->can_evaluate_dictionary_filter() &&
+ conjunct->root()->get_impl() != nullptr;
+ })) {
+ return Status::OK();
+ }
+
+ Block dictionary_block;
+ const size_t dictionary_size = dictionary.size();
+ const auto dummy_type = std::make_shared<DataTypeUInt8>();
+ for (size_t position = 0; position < block_position; ++position) {
+ dictionary_block.insert(
+ {ColumnUInt8::create(dictionary_size, 0), dummy_type,
"dictionary_dummy"});
Review Comment:
Each placeholder here is a dense `dictionary_size` byte column, and
`select_with_runtime_filter()` repeats the pattern with `selected_rows`. Since
a predicate slot can follow a wide projection, auxiliary memory becomes
O(block_position * rows): e.g. 1,000 prior slots × 100,000 dictionary entries
is about 100 MB for one filter setup. Use shared constant/sparse placeholders
or remap the single-column expression to position zero, and test a nonzero/wide
slot.
##########
be/src/core/data_type_serde/data_type_datetimev2_serde.cpp:
##########
@@ -193,6 +197,34 @@ class RejectDateTimeV2BinaryConsumer final : public
ParquetBinaryValueConsumer {
}
};
+class DateTimeV2PredicateParquetConsumer final : public
ParquetFixedValueConsumer {
+public:
+ DateTimeV2PredicateParquetConsumer(const ParquetDecodeContext& context,
bool enable_strict_mode,
+ ParquetLogicalValueConsumer& consumer)
+ : _context(context), _enable_strict_mode(enable_strict_mode),
_consumer(consumer) {}
+
+ Status consume(const uint8_t* values, size_t num_values, size_t
value_width) override {
+ _logical_values.clear();
+ _conversion_nulls.clear();
+ _conversion_nulls.resize_fill(num_values, 0);
+ ParquetMaterializationState state;
+ state.enable_strict_mode = _enable_strict_mode;
+ state.conversion_failure_null_map = &_conversion_nulls;
Review Comment:
This local null map bypasses the established DATETIMEV2 compatibility
policy. For nullable INT96 or UTC-adjusted TIMESTAMP overflow in non-strict
mode, ordinary materialization sends the failure to compatibility scratch,
stores the legacy zero DATETIME default, and leaves the output null bit clear.
Here the same value becomes a synthetic NULL, so a raw predicate such as `col
!= DATETIME '2020-01-01 00:00:00'` drops a row the ordinary path keeps. Route
direct conversion through the same compatibility policy (or disable it for
these timestamp modes) and add a pushdown-on/off differential test.
##########
be/src/format_v2/parquet/reader/native/column_chunk_reader.cpp:
##########
@@ -804,6 +825,55 @@ class FixedWidthPredicateConsumer final : public
ParquetFixedValueConsumer {
int _column_id;
IColumn::Filter* _matches;
IColumn* _projected_column;
+ IColumn::Filter* _conversion_nulls;
+};
+
+class BinaryPredicateConsumer final : public ParquetFixedValueConsumer,
+ public ParquetBinaryValueConsumer {
+public:
+ BinaryPredicateConsumer(const VExprSPtrs& conjuncts, DataTypePtr
data_type, int column_id,
+ IColumn::Filter* matches, IColumn*
projected_column)
+ : _conjuncts(conjuncts),
+ _data_type(std::move(data_type)),
+ _column_id(column_id),
+ _matches(matches),
+ _projected_column(projected_column) {
+ DORIS_CHECK(_matches != nullptr);
+ }
+
+ Status consume(const uint8_t* values, size_t num_values, size_t
value_width) override {
+ _refs.resize(num_values);
Review Comment:
This reintroduces the string staging the native PLAIN contract is meant to
avoid. `BinaryPredicateConsumer` does not override
`consume_plain_byte_array()`, so the default allocates/fills a
`vector<StringRef>` per fragment after the decoder has already produced
payload/value offsets; FIXED_LEN uses this local `_refs` vector too, and
projected survivors are inserted one row at a time below. Please consume the
offset/span callback directly, keep reusable binary scratch in the leaf reader,
and bulk-append projected survivor spans, with projected multi-page coverage.
--
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]
