github-actions[bot] commented on code in PR #66032:
URL: https://github.com/apache/doris/pull/66032#discussion_r3649929009
##########
be/benchmark/parquet/benchmark_parquet_reader.hpp:
##########
@@ -284,10 +339,17 @@ inline Block make_block(const
std::vector<format::ColumnDefinition>& schema) {
}
struct ReaderSession {
+ ~ReaderSession() {
Review Comment:
[P2] Keep complex-expression teardown outside the timed scan
`run_reader()` resumes timing before the loop-scoped `ReaderSession` is
destroyed, and this destructor closes every `opened_conjuncts` entry. Only the
new `COMPLEX_RESIDUAL_SCAN` case populates that vector, so its samples uniquely
include expression-context close/destruction even though the benchmark contract
excludes teardown from steady-state reader timing. Please destroy/reset the
session while timing is still paused, then resume for the common loop boundary.
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -788,16 +761,16 @@ Status FileScannerV2::_build_projected_columns(const
format::TableReader& table_
&column, it->second,
build_context.schema_column.has_value() ?
&*build_context.schema_column : nullptr,
prefer_exact_name_match));
+ _split_value_slot_descs.insert_or_assign(
Review Comment:
[P1] Preserve typed identity values before registering file slots
Registering every projected slot here now sends file-backed Iceberg identity
values through `_parse_partition_value()`. With
`enable.mapping.varbinary=true`, FE sends a UUID string but the mapped
`VARBINARY(16)` serde has no `from_string()` implementation; with
`enable.mapping.timestamp_tz=true`, FE sends an offset-free local datetime but
this path leaves `FormatOptions.timezone` null, so TIMESTAMPTZ parsing rejects
it. Because partition pruning is enabled by default, ordinary scans projecting
either identity field can fail during split preparation; please skip
unsupported values or add a typed, timezone-aware transport and coverage for
both mappings.
##########
be/src/exec/scan/file_scanner_v2.cpp:
##########
@@ -471,33 +460,18 @@ Status FileScannerV2::_get_block_impl(RuntimeState*
state, Block* block, bool* e
}
Status FileScannerV2::_filter_output_block(Block* block) {
- if (_scanner_residual_conjuncts.empty() || block->rows() == 0) {
- return Status::OK();
- }
- SCOPED_TIMER(_scanner_residual_filter_timer);
- const size_t rows_before_filter = block->rows();
- auto status = VExprContext::filter_block(_scanner_residual_conjuncts,
block, block->columns());
- if (!status.ok() && _params != nullptr &&
- _get_current_format_type() == TFileFormatType::FORMAT_ORC) {
- status.prepend("Orc row reader nextBatch failed. reason = ");
- }
- RETURN_IF_ERROR(status);
- const int64_t filtered_rows = cast_set<int64_t>(rows_before_filter -
block->rows());
- _counter.num_rows_unselected += filtered_rows;
- if (_scanner_residual_rows_filtered_counter != nullptr) {
- COUNTER_UPDATE(_scanner_residual_rows_filtered_counter, filtered_rows);
- }
- return Status::OK();
-}
-
-size_t FileScannerV2::_last_block_rows_read(const Block& block) const {
- const auto& stats = _table_reader->last_materialized_block_stats();
- return stats.has_materialized_input ? stats.rows : block.rows();
+ return
_contextualize_output_filter_status(Scanner::_filter_output_block(block),
Review Comment:
[P1] Keep late runtime filters behind the unsafe Scanner suffix
This now evaluates Scanner’s complete late-RF snapshot, but
`update_late_arrival_runtime_filter()` cost-sorts that full vector. A cheap
newly arrived RF can move before an existing stateful/error-preserving
conjunct; if it rejects the batch, conjunct execution stops and suppresses the
earlier invocation/error. The removed residual path kept the first unsafe
conjunct and its suffix at the Scanner boundary and appended late RFs after it,
so please preserve that barrier (or forbid sorting across it) and add an
all-rejecting late-RF test.
--
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]
