github-actions[bot] commented on code in PR #66035:
URL: https://github.com/apache/doris/pull/66035#discussion_r3649437205
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -799,10 +826,15 @@ format::ColumnDefinition
FileScannerV2::_build_table_column(const SlotDescriptor
}
Status FileScannerV2::_build_table_conjuncts(VExprContextSPtrs* conjuncts)
const {
+ return _build_table_conjuncts(_conjuncts, conjuncts);
Review Comment:
[P1] Preserve append order for next-split partition pruning
This default overload still clones the cost-sorted `_conjuncts`, whereas the
new `_late_arrival_rf_conjuncts` identity path protects only row execution.
With the default cost sort enabled, a cheap late partition RF can move ahead of
an older unsafe/error-preserving predicate; the next split passes that order to
the safe-prefix partition-prune walker, which can reject the split before ever
reaching the older predicate and thereby suppress its error/stateful semantics.
Please build pruning predicates from an append-identity-ordered view (the same
owned-prefix/ordered-suffix state used for row execution) and add a next-split
test covering an unsafe partition predicate followed by a late rejecting RF.
##########
be/src/format_v2/table_reader.cpp:
##########
@@ -734,35 +739,152 @@ Status TableReader::init(TableReadOptions&& options) {
}
_system_properties = create_system_properties(_scan_params);
_mapper_options.mode = TableColumnMappingMode::BY_NAME;
- _conjuncts = std::move(options.conjuncts);
+ return _replace_conjuncts(options.conjuncts);
+}
+
+Status TableReader::_prepare_conjunct(const VExprContextSPtr& source,
VExprContextSPtr* prepared) {
+ DORIS_CHECK(source != nullptr);
+ DORIS_CHECK(source->root() != nullptr);
+ DORIS_CHECK(prepared != nullptr);
+ VExprSPtr root;
+ RETURN_IF_ERROR(clone_table_expr_tree(source->root(), &root));
+ auto conjunct = VExprContext::create_shared(std::move(root));
+ RETURN_IF_ERROR(conjunct->prepare(_runtime_state, RowDescriptor {}));
+ RETURN_IF_ERROR(conjunct->open(_runtime_state));
+ *prepared = std::move(conjunct);
+ return Status::OK();
+}
+
+Status TableReader::_replace_conjuncts(const VExprContextSPtrs& conjuncts) {
+ VExprContextSPtrs prepared;
+ prepared.reserve(conjuncts.size());
+ for (const auto& source : conjuncts) {
+ VExprContextSPtr conjunct;
+ RETURN_IF_ERROR(_prepare_conjunct(source, &conjunct));
Review Comment:
[P2] Keep Scanner-owned suffix contexts analysis-only
`_replace_conjuncts()` prepares and opens every expression even when
`_table_reader_owned_conjunct_count` stops before the unsafe suffix;
`append_conjuncts()` likewise calls `_prepare_conjunct()` before applying
`owned_count`. That suffix is executed by Scanner, not the native TableReader,
so stateful/external functions get an extra FRAGMENT_LOCAL/THREAD_LOCAL
lifecycle (and any open-time resources or errors) for every scanner. Please
prepare/open only the owned prefix and keep the suffix as unopened analysis
metadata.
##########
be/src/exec/operator/scan_operator.cpp:
##########
@@ -91,6 +97,16 @@ Status
ScanLocalStateBase::update_late_arrival_runtime_filter(RuntimeState* stat
if (_conjuncts.size() > conjuncts_before) {
RETURN_IF_ERROR(_on_runtime_filter_update());
}
+ for (const auto& [batch_arrived_rf_num, batch] :
_late_arrival_conjunct_batches) {
+ if (batch_arrived_rf_num <= applied_rf_num) {
+ continue;
+ }
+ for (const auto& conjunct : batch) {
+ VExprContextSPtr cloned;
+ RETURN_IF_ERROR(conjunct->clone(state, cloned));
Review Comment:
[P2] Avoid cloning RF deltas for scanners that cannot consume them
This runs under `_conjuncts_lock` and clones every unseen RF batch for every
Scanner subclass. The base Scanner then clones the complete conjunct snapshot
as well and stores these delta contexts in `_late_arrival_rf_conjuncts`, but
only FileScannerV2 ever consumes or clears that vector; FileScanner V1,
OlapScanner, JdbcScanner, and MetaScanner retain the opened clones until
teardown. Please make delta delivery a FileScannerV2 capability/override (or
otherwise optional) and avoid doing unused clone/open work under the shared
lock.
--
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]
