github-actions[bot] commented on code in PR #69723:
URL: https://github.com/apache/doris/pull/69723#discussion_r4414682246
##########
be/src/exec/operator/file_scan_operator.cpp:
##########
@@ -245,41 +138,12 @@ Status
FileScanLocalState::_init_scanners(std::list<ScannerSPtr>* scanners) {
scan_params = _split_source->get_params();
}
RETURN_IF_ERROR(validate_file_scan_support(*scan_params));
- const bool is_load =
-
state()->desc_tbl().get_tuple_descriptor(scan_params->src_tuple_id) != nullptr;
- // TODO: Use scanner v2 for all queries.
- const bool use_file_scanner_v2 =
- should_use_file_scanner_v2(state()->query_options(), is_load,
*scan_params);
- _operator_profile->add_info_string("UseScannerV2", use_file_scanner_v2 ?
"true" : "false");
- const auto* output_tuple_desc =
state()->desc_tbl().get_tuple_descriptor(_output_tuple_id);
- DORIS_CHECK(output_tuple_desc != nullptr);
- const bool metadata_only_count =
- is_count_star_pushdown() &&
_split_source->all_ranges_have_table_level_row_count();
- if (!is_load && !use_file_scanner_v2 && !metadata_only_count &&
- std::ranges::any_of(output_tuple_desc->slots(), [](const
SlotDescriptor* slot) {
- return contains_variant_type(slot->type());
- })) {
- // A syntactic COUNT(*) alone is insufficient: every assigned range
must prove that the
- // legacy scanner will emit metadata counts without decoding a Variant
carrier.
- return Status::NotSupported(
- "External VARIANT columns require FileScannerV2; the legacy
file scanner does "
- "not support VARIANT");
- }
- if (use_file_scanner_v2) {
- _parquet_profile_cache =
-
std::make_unique<format::parquet::ParquetProfileCache>(_scanner_profile.get());
- }
+ _parquet_profile_cache =
+
std::make_unique<format::parquet::ParquetProfileCache>(_scanner_profile.get());
for (int i = 0; i < _max_scanners; ++i) {
- ScannerSPtr scanner;
- if (use_file_scanner_v2) {
- scanner = FileScannerV2::create_shared(state(), this, p._limit,
_split_source,
- _scanner_profile.get(),
p._kv_cache.get(),
- &p._colname_to_slot_id);
- } else {
- scanner = FileScanner::create_shared(state(), this, p._limit,
_split_source,
- _scanner_profile.get(),
p._kv_cache.get(),
- &p._colname_to_slot_id);
- }
+ ScannerSPtr scanner = FileScannerV2::create_shared(
+ state(), this, p._limit, _split_source,
_scanner_profile.get(), p._kv_cache.get(),
Review Comment:
[P2] Preserve the active file path on V2 reader errors. FileScannerV2 is now
selected for every file scan, but its `_get_block_impl` returns
reader/open/refresh failures without the path context that the removed
`FileScanner::_get_block_impl` appended via `get_current_scan_range_name()`.
The scanner/operator layer only propagates the status, so a malformed or
unavailable file in a multi-file scan now reports an ambiguous error with no
URI. Add the current range name at the V2 boundary, while retaining the
ORC-specific prefix, so failures remain attributable to the file that caused
them.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java:
##########
@@ -756,13 +756,10 @@ private LogicalAggregate<? extends Plan>
storageLayerAggregate(
// COUNT(nullable_col); synthetic rows would make one
upper aggregate wrong.
return canNotPush;
}
- // Nullable file COUNT is exact only when this query is routed
to FileScannerV2,
- // which carries the semantic argument and counts definition
levels. Gating on the
- // session switch keeps V1 on its original full-column
evaluation path.
+ // File scans preserve the COUNT argument and count its
definition levels,
+ // so nullable COUNT can use exact storage-layer aggregation.
boolean supportsNullableFileCount = logicalScan instanceof
LogicalFileScan
- && mergeOp == PushDownAggOp.COUNT
- && cascadesContext.getConnectContext() != null
- &&
cascadesContext.getConnectContext().getSessionVariable().enableFileScannerV2;
+ && mergeOp == PushDownAggOp.COUNT;
if (column.isAllowNull() && checkNullSlots.contains(slot)
Review Comment:
[P1] Preserve nullable COUNT compatibility with old BEs. This hunk removes
the `enableFileScannerV2` gate, so `SET enable_file_scanner_v2=false` now
pushes `COUNT(nullable_col)` for a `LogicalFileScan`. The FE sends the argument
in `TPlanNode.push_down_count_slot_ids` (field 55), but branch-4.0/3.1 BEs do
not define that field; their V1 Parquet reader still sees
`push_down_agg_type_opt=COUNT` and materializes the physical row count,
including NULLs. A rolling master-FE/old-BE query therefore returns `COUNT(*)`
instead of `COUNT(col)`, whereas this setting previously kept the query on
normal row evaluation. Keep the compatibility gate or add a capability-safe
fallback before enabling this pushdown.
--
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]
