diff --git a/be/src/exec/operator/scan_operator.cpp b/be/src/exec/operator/scan_operator.cpp index a9e92d08e08613..3bb234d16b19b9 100644 --- a/be/src/exec/operator/scan_operator.cpp +++ b/be/src/exec/operator/scan_operator.cpp @@ -1046,6 +1046,12 @@ TPushAggOp::type ScanLocalState::get_push_down_agg_type() { return _parent->cast()._push_down_agg_type; } +template +const std::optional>& ScanLocalState::get_push_down_count_slot_ids() + const { + return _parent->cast()._push_down_count_slot_ids; +} + template int64_t ScanLocalState::limit_per_scanner() { return _parent->cast()._limit_per_scanner; @@ -1231,6 +1237,9 @@ Status ScanOperatorX::init(const TPlanNode& tnode, RuntimeState* } else { _push_down_agg_type = TPushAggOp::type::NONE; } + if (tnode.__isset.push_down_count_slot_ids) { + _push_down_count_slot_ids = tnode.push_down_count_slot_ids; + } if (tnode.__isset.topn_filter_source_node_ids) { _topn_filter_source_node_ids = tnode.topn_filter_source_node_ids; diff --git a/be/src/exec/operator/scan_operator.h b/be/src/exec/operator/scan_operator.h index 8b12ccf0bc1195..ae93e155852c00 100644 --- a/be/src/exec/operator/scan_operator.h +++ b/be/src/exec/operator/scan_operator.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include #include @@ -74,6 +75,7 @@ class ScanLocalStateBase : public PipelineXLocalState<> { virtual void set_scan_ranges(RuntimeState* state, const std::vector& scan_ranges) = 0; virtual TPushAggOp::type get_push_down_agg_type() = 0; + virtual const std::optional>& get_push_down_count_slot_ids() const = 0; // If scan operator is serial operator(like topn), its real parallelism is 1. // Otherwise, its real parallelism is query_parallel_instance_num. @@ -252,6 +254,7 @@ class ScanLocalState : public ScanLocalStateBase { const std::vector& scan_ranges) override {} TPushAggOp::type get_push_down_agg_type() override; + const std::optional>& get_push_down_count_slot_ids() const override; std::vector execution_dependencies() override { if (_filter_dependencies.empty()) { @@ -452,6 +455,16 @@ class ScanOperatorX : public OperatorX { TPushAggOp::type _push_down_agg_type; + // Semantic arguments of a pushed-down COUNT. This is deliberately optional because absence + // and an empty list have different meanings during a BE-first rolling upgrade: + // + // - nullopt: an old FE did not send the field, so the new BE must use the normal scan; + // - empty: the new FE explicitly planned COUNT(*)/COUNT(1); + // - non-empty: the new FE explicitly planned COUNT(col). + // + // Treating nullopt as empty would silently reinterpret an old plan as COUNT(*). + std::optional> _push_down_count_slot_ids; + // Record the value of the aggregate function 'count' from doris's be int64_t _push_down_count = -1; const int _parallel_tasks = 0; diff --git a/be/src/exec/scan/file_scanner.h b/be/src/exec/scan/file_scanner.h index 87a54243f16c96..f575cdae89263f 100644 --- a/be/src/exec/scan/file_scanner.h +++ b/be/src/exec/scan/file_scanner.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -80,6 +81,10 @@ class FileScanner : public Scanner { const VExprContextSPtrs& TEST_runtime_filter_partition_prune_ctxs() const { return _runtime_filter_partition_prune_ctxs; } + static TPushAggOp::type TEST_effective_push_down_agg_type( + TPushAggOp::type agg_type, const std::optional>& count_slot_ids) { + return _effective_push_down_agg_type(agg_type, count_slot_ids); + } #endif FileScanner(RuntimeState* state, FileScanLocalState* parent, int64_t limit, @@ -334,9 +339,28 @@ class FileScanner : public Scanner { void _update_adaptive_batch_size_before_truncate(const Block& block); void _update_adaptive_batch_size_after_truncate(const Block& block); + static TPushAggOp::type _effective_push_down_agg_type( + TPushAggOp::type agg_type, const std::optional>& count_slot_ids) { + if (agg_type != TPushAggOp::type::COUNT) { + return agg_type; + } + // V1's CountReader receives only the file's total row count and emits that many synthetic + // rows. This is exact for COUNT(*)/COUNT(1), but it has no column metadata for NULL or CAST + // semantics. For example, a 10,000-row file with 9,015 non-null values must return 10,000 + // for COUNT(*) and 9,015 for COUNT(nullable_col); CountReader can produce only the former. + // Therefore a non-empty argument list must use the normal reader. nullopt is an old FE plan + // that predates the argument field; treating it as empty would silently reinterpret unknown + // semantics as COUNT(*). + return count_slot_ids.has_value() && count_slot_ids->empty() ? TPushAggOp::type::COUNT + : TPushAggOp::type::NONE; + } + TPushAggOp::type _get_push_down_agg_type() const { - return _local_state == nullptr ? TPushAggOp::type::NONE - : _local_state->get_push_down_agg_type(); + if (_local_state == nullptr) { + return TPushAggOp::type::NONE; + } + return _effective_push_down_agg_type(_local_state->get_push_down_agg_type(), + _local_state->get_push_down_count_slot_ids()); } // enable the file meta cache only when diff --git a/be/src/exec/scan/file_scanner_v2.cpp b/be/src/exec/scan/file_scanner_v2.cpp index 99586b5cfa96e2..f06e9fb9533774 100644 --- a/be/src/exec/scan/file_scanner_v2.cpp +++ b/be/src/exec/scan/file_scanner_v2.cpp @@ -272,6 +272,10 @@ void FileScannerV2::TEST_report_file_cache_profile( bool FileScannerV2::TEST_should_skip_not_found(const Status& status, bool ignore_not_found) { return _should_skip_not_found(status, ignore_not_found); } + +bool FileScannerV2::TEST_should_skip_empty(const Status& status, bool stopped) { + return _should_skip_empty(status, stopped); +} #endif bool FileScannerV2::is_supported(const TFileScanRangeParams& params, const TFileRangeDesc& range) { @@ -322,6 +326,8 @@ Status FileScannerV2::init(RuntimeState* state, const VExprContextSPtrs& conjunc RETURN_IF_ERROR(Scanner::init(state, conjuncts)); _get_block_timer = ADD_TIMER_WITH_LEVEL(_local_state->scanner_profile(), "FileScannerV2GetBlockTime", 1); + _empty_file_counter = + ADD_COUNTER_WITH_LEVEL(_local_state->scanner_profile(), "EmptyFileNum", TUnit::UNIT, 1); _not_found_file_counter = ADD_COUNTER_WITH_LEVEL(_local_state->scanner_profile(), "NotFoundFileNum", TUnit::UNIT, 1); _file_counter = @@ -394,6 +400,20 @@ Status FileScannerV2::_get_block_impl(RuntimeState* state, Block* block, bool* e *eof = false; continue; } + if (_should_skip_empty(status, _should_stop || _io_ctx->should_stop)) { + // END_OF_FILE here means the reader discovered a valid split with no data while + // opening or probing it, not that the Scanner has exhausted all splits. Examples + // are a zero-byte CSV with an explicit schema and a Doris Native file containing + // only its 12-byte header. Treat it like V1's empty-file path: finish this range, + // discard partial reader state, and let the loop fetch the next split. + RETURN_IF_ERROR(_table_reader->abort_split()); + COUNTER_UPDATE(_empty_file_counter, 1); + _state->update_num_finished_scan_range(1); + _has_prepared_split = false; + block->clear_column_data(cast_set(_projected_columns.size())); + *eof = false; + continue; + } RETURN_IF_ERROR(status); } if (*eof) { @@ -423,9 +443,12 @@ Status FileScannerV2::_prepare_next_split(bool* eos) { const auto format_type = get_range_format_type(*_params, _current_range); _init_adaptive_batch_size_state(format_type); - if (_should_run_adaptive_batch_size()) { - // JNI readers open eagerly in prepare_split(). Seed the probe size first so readers - // such as Paimon also use it for their first physical read batch. + if (_block_size_predictor != nullptr) { + // JNI readers open eagerly in prepare_split(). Always seed the probe before preparing + // the next split: its metadata-COUNT decision is not available yet, and the state + // exposed by TableReader can still describe the preceding split. Metadata shortcuts + // ignore this batch size, while row-scan fallbacks need it for their first physical + // read batch. _table_reader->set_batch_size(_predict_reader_batch_rows()); } std::map partition_values; @@ -438,6 +461,16 @@ Status FileScannerV2::_prepare_next_split(bool* eos) { _state->update_num_finished_scan_range(1); continue; } + if (_should_skip_empty(status, _should_stop || _io_ctx->should_stop)) { + // Schema discovery can reach EOF before a split becomes prepared. A header-only Native + // file follows this path, while a reader that discovers emptiness on its first + // get_block() follows the symmetric branch in _get_block_impl(). Both paths must + // advance exactly one scan range and preserve later files in the same scan. + RETURN_IF_ERROR(_table_reader->abort_split()); + COUNTER_UPDATE(_empty_file_counter, 1); + _state->update_num_finished_scan_range(1); + continue; + } RETURN_IF_ERROR(status); if (_table_reader->current_split_pruned()) { _state->update_num_finished_scan_range(1); @@ -458,6 +491,21 @@ Status FileScannerV2::_init_table_reader(const TFileRangeDesc& range) { VExprContextSPtrs table_conjuncts; RETURN_IF_ERROR(_build_table_conjuncts(&table_conjuncts)); + std::optional> push_down_count_columns; + const auto& push_down_count_slot_ids = _local_state->get_push_down_count_slot_ids(); + if (push_down_count_slot_ids.has_value()) { + push_down_count_columns.emplace(); + push_down_count_columns->reserve(push_down_count_slot_ids->size()); + for (const auto slot_id : *push_down_count_slot_ids) { + const auto global_index_it = _slot_id_to_global_index.find(slot_id); + if (global_index_it == _slot_id_to_global_index.end()) { + return Status::InternalError( + "Pushed-down COUNT argument is not a projected file scan slot, slot_id={}", + slot_id); + } + push_down_count_columns->push_back(global_index_it->second); + } + } RETURN_IF_ERROR(_table_reader->init({ .projected_columns = _projected_columns, .conjuncts = std::move(table_conjuncts), @@ -468,6 +516,7 @@ Status FileScannerV2::_init_table_reader(const TFileRangeDesc& range) { .scanner_profile = _local_state->scanner_profile(), .file_slot_descs = &_file_slot_descs, .push_down_agg_type = _local_state->get_push_down_agg_type(), + .push_down_count_columns = std::move(push_down_count_columns), .condition_cache_digest = _local_state->get_condition_cache_digest(), })); return Status::OK(); @@ -540,6 +589,14 @@ bool FileScannerV2::_should_skip_not_found(const Status& status, bool ignore_not return ignore_not_found && status.is(); } +bool FileScannerV2::_should_skip_empty(const Status& status, bool stopped) { + // Several readers use END_OF_FILE both for a valid zero-row split and for an interrupted IO. + // For example, DeletionVectorReader returns END_OF_FILE("stop read.") after try_stop() marks + // the shared IOContext. That status must unwind the stopped scanner; counting it as an empty + // file would incorrectly finish the scan range and increment EmptyFileNum. + return !stopped && status.is(); +} + bool FileScannerV2::_should_enable_file_meta_cache() const { return ExecEnv::GetInstance()->file_meta_cache()->enabled() && _split_source->num_scan_ranges() < config::max_external_file_meta_cache_num / 3; @@ -809,10 +866,17 @@ bool FileScannerV2::_should_enable_adaptive_batch_size(TFileFormatType::type for } bool FileScannerV2::_should_run_adaptive_batch_size() const { - // COUNT pushdown emits synthetic rows from file metadata and does not materialize file columns, - // so there is no useful row-width sample to learn from. - return _block_size_predictor != nullptr && - _local_state->get_push_down_agg_type() != TPushAggOp::type::COUNT; + DORIS_CHECK(_table_reader != nullptr); + return _should_run_adaptive_batch_size(_block_size_predictor != nullptr, + _table_reader->current_split_uses_metadata_count()); +} + +bool FileScannerV2::_should_run_adaptive_batch_size(bool predictor_initialized, + bool current_split_uses_metadata_count) { + // Metadata COUNT emits synthetic rows and has no physical row width to learn from. A raw COUNT + // opcode is not sufficient here: unsupported argument counts, mappings, filters, or deletes + // make TableReader fall back to materializing normal rows, which still need adaptive batching. + return predictor_initialized && !current_split_uses_metadata_count; } size_t FileScannerV2::_predict_reader_batch_rows() { diff --git a/be/src/exec/scan/file_scanner_v2.h b/be/src/exec/scan/file_scanner_v2.h index 2bddc5d5e69e6c..0cd03030b56851 100644 --- a/be/src/exec/scan/file_scanner_v2.h +++ b/be/src/exec/scan/file_scanner_v2.h @@ -87,6 +87,12 @@ class FileScannerV2 final : public Scanner { static void TEST_report_file_cache_profile( RuntimeProfile* profile, const io::FileCacheStatistics& file_cache_statistics); static bool TEST_should_skip_not_found(const Status& status, bool ignore_not_found); + static bool TEST_should_skip_empty(const Status& status, bool stopped); + static bool TEST_should_run_adaptive_batch_size(bool predictor_initialized, + bool current_split_uses_metadata_count) { + return _should_run_adaptive_batch_size(predictor_initialized, + current_split_uses_metadata_count); + } #endif FileScannerV2(RuntimeState* state, FileScanLocalState* parent, int64_t limit, @@ -121,6 +127,7 @@ class FileScannerV2 final : public Scanner { Status _prepare_table_reader_split(const TFileRangeDesc& range, std::map partition_values); static bool _should_skip_not_found(const Status& status, bool ignore_not_found); + static bool _should_skip_empty(const Status& status, bool stopped); bool _should_enable_file_meta_cache() const; std::optional _create_global_rowid_context( const TFileRangeDesc& range) const; @@ -138,6 +145,8 @@ class FileScannerV2 final : public Scanner { void _init_adaptive_batch_size_state(TFileFormatType::type format_type); bool _should_enable_adaptive_batch_size(TFileFormatType::type format_type) const; bool _should_run_adaptive_batch_size() const; + static bool _should_run_adaptive_batch_size(bool predictor_initialized, + bool current_split_uses_metadata_count); size_t _predict_reader_batch_rows(); void _update_adaptive_batch_size(const Block& block); static RealtimeCounterDeltas _collect_realtime_counter_deltas( @@ -181,6 +190,7 @@ class FileScannerV2 final : public Scanner { ShardedKVCache* _kv_cache = nullptr; RuntimeProfile::Counter* _get_block_timer = nullptr; + RuntimeProfile::Counter* _empty_file_counter = nullptr; RuntimeProfile::Counter* _not_found_file_counter = nullptr; RuntimeProfile::Counter* _file_counter = nullptr; RuntimeProfile::Counter* _file_read_bytes_counter = nullptr; diff --git a/be/src/format_v2/column_mapper.cpp b/be/src/format_v2/column_mapper.cpp index 313bbf376f860e..6713183c09d816 100644 --- a/be/src/format_v2/column_mapper.cpp +++ b/be/src/format_v2/column_mapper.cpp @@ -790,6 +790,19 @@ static bool collect_struct_element_chain(const VExprSPtr& expr, std::vector, table + // STRUCT, rows [NULL, 20], and `s.a > 10`, filtering in the file domain would drop + // NULL first and hide the table-contract violation. The reverse direction is safe: a required + // file value can always be wrapped as a nullable table value after filtering. + return !file_type->is_nullable() || table_type->is_nullable(); +} + static bool rewrite_struct_element_path_to_file_expr( const VExprSPtr& expr, const std::vector& mappings, const std::map& global_to_file_slot, @@ -816,6 +829,22 @@ static bool rewrite_struct_element_path_to_file_expr( return false; } + // Check every value-producing level, including the root struct. A nullable parent also makes + // a child access nullable even when the child type itself is required, so checking only the + // final leaf is insufficient. If any file level is more nullable than its table counterpart, + // keep the complete predicate above TableReader so schema validation observes all NULLs before + // row filtering. + if (!can_filter_before_table_nullability_alignment(rewrite_it->second.file_type, + rewrite_it->second.table_type)) { + return false; + } + for (size_t idx = 0; idx < struct_element_chain.size(); ++idx) { + if (!can_filter_before_table_nullability_alignment( + resolved.file_child_types[idx], struct_element_chain[idx]->data_type())) { + return false; + } + } + // File-local conjuncts are prepared against the file-reader Block, so both the root slot and // every struct selector must be expressed in file schema terms. For a renamed Iceberg field, // keeping the table selector would prepare `element_at(file_struct, 'renamed')` and @@ -844,6 +873,209 @@ static bool rewrite_struct_element_path_to_file_expr( return true; } +static VExprSPtr cast_file_expr_to_table_type(const VExprSPtr& file_expr, + const DataTypePtr& table_type, + RewriteContext* rewrite_context) { + DORIS_CHECK(file_expr != nullptr); + DORIS_CHECK(table_type != nullptr); + DORIS_CHECK(rewrite_context != nullptr); + auto cast_expr = Cast::create_shared(table_type); + cast_expr->add_child(file_expr); + rewrite_context->add_created_expr(cast_expr); + return cast_expr; +} + +// Prefer comparing in the physical file leaf type when a table predicate uses a promoted struct +// child. For example, with table STRUCT, old-file STRUCT, and `s.a = 10`, the +// localized predicate should be `file_s.a::INT = 10::INT`, not +// `CAST(file_s.a::INT AS BIGINT) = 10::BIGINT`. Converting one literal avoids a cast for every row. +// +// This rewrite is valid only when every possible file value survives file-to-table conversion and +// the particular literal survives a table-to-file-to-table round trip. A value such as BIGINT +// 2147483648 cannot be represented by an INT file leaf, so that case deliberately falls back to +// `CAST(file_s.a AS BIGINT) = 2147483648`, which preserves the original table-level semantics. +static bool rewrite_binary_struct_literal_predicate( + const VExprSPtr& expr, const std::vector& filter_mappings, + const std::map& global_to_file_slot, + RewriteContext* rewrite_context, bool* can_localize) { + DORIS_CHECK(can_localize != nullptr); + if (!is_binary_comparison_predicate(expr)) { + return false; + } + auto children = expr->children(); + int struct_child_idx = -1; + int literal_child_idx = -1; + if (is_struct_element_expr(children[0])) { + struct_child_idx = 0; + literal_child_idx = 1; + } else if (is_struct_element_expr(children[1])) { + struct_child_idx = 1; + literal_child_idx = 0; + } else { + return false; + } + + const auto table_leaf_type = children[struct_child_idx]->data_type(); + DORIS_CHECK(table_leaf_type != nullptr); + auto table_literal = unwrap_literal_for_file_cast(children[literal_child_idx], table_leaf_type); + if (table_literal == nullptr || + !rewrite_struct_element_path_to_file_expr(children[struct_child_idx], filter_mappings, + global_to_file_slot, rewrite_context)) { + return false; + } + + const auto file_leaf_type = children[struct_child_idx]->data_type(); + DORIS_CHECK(file_leaf_type != nullptr); + const FileSlotRewriteInfo leaf_rewrite_info { + .block_position = 0, + .file_type = file_leaf_type, + .table_type = table_leaf_type, + .file_column_name = {}, + }; + auto file_literal = + rewrite_literal_to_file_type(table_literal, leaf_rewrite_info, rewrite_context); + if (file_literal != nullptr) { + children[literal_child_idx] = std::move(file_literal); + } else { + if (!is_lossless_file_to_table_numeric_cast(file_leaf_type, table_leaf_type)) { + // A narrowing or otherwise lossy cast can fail or produce NULL while TableReader + // materializes the table schema. Evaluating it here could filter the offending row + // before that validation, so keep the complete predicate above TableReader. + *can_localize = false; + return true; + } + children[struct_child_idx] = cast_file_expr_to_table_type(children[struct_child_idx], + table_leaf_type, rewrite_context); + children[literal_child_idx] = original_table_literal(table_literal, rewrite_context); + } + expr->set_children(std::move(children)); + return true; +} + +// IN must use one comparison type for its probe and every candidate. Rewrite the complete literal +// set only when all values are exactly representable in the file leaf type; one unsafe value makes +// the whole predicate fall back to a table-type cast. For example, an INT file leaf can evaluate +// `BIGINT IN (10, 20)` as `INT IN (10, 20)`, but `BIGINT IN (10, 2147483648)` must stay BIGINT. +static bool rewrite_in_struct_literal_predicate( + const VExprSPtr& expr, const std::vector& filter_mappings, + const std::map& global_to_file_slot, + RewriteContext* rewrite_context, bool* can_localize) { + DORIS_CHECK(can_localize != nullptr); + if (expr->node_type() != TExprNodeType::IN_PRED || expr->get_num_children() < 2 || + !is_struct_element_expr(expr->children()[0])) { + return false; + } + auto children = expr->children(); + const auto table_leaf_type = children[0]->data_type(); + DORIS_CHECK(table_leaf_type != nullptr); + VExprSPtrs table_literals; + table_literals.reserve(children.size() - 1); + for (size_t child_idx = 1; child_idx < children.size(); ++child_idx) { + auto table_literal = unwrap_literal_for_file_cast(children[child_idx], table_leaf_type); + if (table_literal == nullptr) { + return false; + } + table_literals.push_back(std::move(table_literal)); + } + if (!rewrite_struct_element_path_to_file_expr(children[0], filter_mappings, global_to_file_slot, + rewrite_context)) { + return false; + } + + const auto file_leaf_type = children[0]->data_type(); + DORIS_CHECK(file_leaf_type != nullptr); + const FileSlotRewriteInfo leaf_rewrite_info { + .block_position = 0, + .file_type = file_leaf_type, + .table_type = table_leaf_type, + .file_column_name = {}, + }; + VExprSPtrs file_literals; + file_literals.reserve(table_literals.size()); + for (const auto& table_literal : table_literals) { + auto file_literal = + rewrite_literal_to_file_type(table_literal, leaf_rewrite_info, rewrite_context); + if (file_literal == nullptr) { + if (!is_lossless_file_to_table_numeric_cast(file_leaf_type, table_leaf_type)) { + *can_localize = false; + return true; + } + children[0] = + cast_file_expr_to_table_type(children[0], table_leaf_type, rewrite_context); + for (size_t literal_idx = 0; literal_idx < table_literals.size(); ++literal_idx) { + children[literal_idx + 1] = + original_table_literal(table_literals[literal_idx], rewrite_context); + } + expr->set_children(std::move(children)); + return true; + } + file_literals.push_back(std::move(file_literal)); + } + + for (size_t literal_idx = 0; literal_idx < file_literals.size(); ++literal_idx) { + children[literal_idx + 1] = std::move(file_literals[literal_idx]); + } + expr->set_children(std::move(children)); + return true; +} + +static VExprSPtr rewrite_struct_or_slot_expr_to_file_expr( + const VExprSPtr& expr, + const std::map& global_to_file_slot, + const std::vector& filter_mappings, RewriteContext* rewrite_context, + bool* can_localize) { + if (is_struct_element_expr(expr)) { + const auto table_leaf_type = expr->data_type(); + if (!rewrite_struct_element_path_to_file_expr(expr, filter_mappings, global_to_file_slot, + rewrite_context)) { + // The scanner still evaluates the original table-level conjunct after TableReader + // finalizes the output block. Skipping an unlocalizable file conjunct is therefore + // safer than preparing a partially rewritten expression against the wrong struct + // layout. In particular, do not generate file-local conjuncts for computed complex + // parents such as `element_at(element_at(map_values(m), 1), 'field')`; only direct + // slot-rooted struct chains are supported here. + *can_localize = false; + return expr; + } + DORIS_CHECK(table_leaf_type != nullptr); + DORIS_CHECK(expr->data_type() != nullptr); + if (!expr->data_type()->equals(*table_leaf_type)) { + if (!is_lossless_file_to_table_numeric_cast(expr->data_type(), table_leaf_type)) { + *can_localize = false; + return expr; + } + // Path localization changes the leaf to the physical file type. For example, after an + // Iceberg evolution from STRUCT to STRUCT, the localized old-file + // predicate is initially `element_at(file_col, 'a')::INT = 10::BIGINT`. Cast only the + // leaf back to BIGINT so the comparison has matching operands without forcing a cast + // of the entire evolved struct (whose children may also have been added or reordered). + return cast_file_expr_to_table_type(expr, table_leaf_type, rewrite_context); + } + return expr; + } + + DORIS_CHECK(expr->is_slot_ref()); + const auto* slot_ref = assert_cast(expr.get()); + const auto rewrite_it = global_to_file_slot.find(slot_ref_global_index(*slot_ref)); + if (rewrite_it == global_to_file_slot.end()) { + return expr; + } + const auto& rewrite_info = rewrite_it->second; + auto file_slot = create_file_slot_ref(*slot_ref, rewrite_info, rewrite_context); + if (rewrite_info.file_type->equals(*rewrite_info.table_type)) { + return file_slot; + } + if (needs_complex_file_slot_cast(rewrite_info.file_type, rewrite_info.table_type)) { + // Generic file-local expressions cannot safely cast an evolved complex file slot back to + // the table type. For example, ARRAY_CONTAINS(MAP_KEYS(m), 'person5') only reads map keys, + // but CAST(file_m AS table_m) first forces an incompatible old value struct into the new + // layout. Keep such predicates at table level, after TableReader materializes evolution. + *can_localize = false; + return expr; + } + return cast_file_expr_to_table_type(file_slot, rewrite_info.table_type, rewrite_context); +} + static VExprSPtr rewrite_table_expr_to_file_expr( const VExprSPtr& expr, const std::map& global_to_file_slot, @@ -875,52 +1107,18 @@ static VExprSPtr rewrite_table_expr_to_file_expr( if (rewrite_in_slot_literal_predicate(expr, global_to_file_slot, rewrite_context)) { return expr; } - if (is_struct_element_expr(expr)) { - if (!rewrite_struct_element_path_to_file_expr(expr, filter_mappings, global_to_file_slot, - rewrite_context)) { - // The scanner still evaluates the original table-level conjunct after TableReader - // finalizes the output block. Skipping an unlocalizable file conjunct is therefore - // safer than preparing a partially rewritten expression against the wrong struct - // layout. In particular, do not generate file-local conjuncts for computed complex - // parents such as `element_at(element_at(map_values(m), 1), 'field')`; only direct - // slot-rooted struct chains are supported here. - *can_localize = false; - } + if (rewrite_binary_struct_literal_predicate(expr, filter_mappings, global_to_file_slot, + rewrite_context, can_localize)) { return expr; } - if (expr->is_slot_ref()) { - const auto* slot_ref = assert_cast(expr.get()); - const auto rewrite_it = global_to_file_slot.find(slot_ref_global_index(*slot_ref)); - if (rewrite_it != global_to_file_slot.end()) { - const auto& rewrite_info = rewrite_it->second; - auto file_slot = create_file_slot_ref(*slot_ref, rewrite_info, rewrite_context); - if (rewrite_info.file_type->equals(*rewrite_info.table_type)) { - return file_slot; - } - if (needs_complex_file_slot_cast(rewrite_info.file_type, rewrite_info.table_type)) { - // Generic file-local expressions cannot safely cast an evolved complex file slot - // back to the table type. Example: - // - // table filter: ARRAY_CONTAINS(MAP_KEYS(m), 'person5') - // old file: m MAP> - // table: m MAP> - // - // Although MAP_KEYS only reads the key column, wrapping the file slot as - // `CAST(file_m AS table_m)` forces the value struct cast first and fails because - // the old and new value structs have different fields. Keep such filters at the - // table level, where TableReader materializes the evolved complex value before - // Scanner evaluates the original conjunct. Direct slot-rooted struct child paths - // are handled by rewrite_struct_element_path_to_file_expr() above. - *can_localize = false; - return expr; - } - auto cast_expr = Cast::create_shared(rewrite_info.table_type); - cast_expr->add_child(std::move(file_slot)); - rewrite_context->add_created_expr(cast_expr); - return cast_expr; - } + if (rewrite_in_struct_literal_predicate(expr, filter_mappings, global_to_file_slot, + rewrite_context, can_localize)) { return expr; } + if (is_struct_element_expr(expr) || expr->is_slot_ref()) { + return rewrite_struct_or_slot_expr_to_file_expr(expr, global_to_file_slot, filter_mappings, + rewrite_context, can_localize); + } // The input is a split-local cloned tree. A previous split-local clone may already have // inserted Cast(slot). Keep that rewrite idempotent: rewrite the cast child from table slot to // the current split's file slot, and drop the cast when the current split no longer needs it. diff --git a/be/src/format_v2/file_reader.cpp b/be/src/format_v2/file_reader.cpp index 4f5b247c791efd..b2603a17958eae 100644 --- a/be/src/format_v2/file_reader.cpp +++ b/be/src/format_v2/file_reader.cpp @@ -62,7 +62,14 @@ std::string FileScanRequest::debug_string() const { out << column_id << ":" << block_position; } out << "}, conjunct_count=" << conjuncts.size() - << ", delete_conjunct_count=" << delete_conjuncts.size() << "}"; + << ", delete_conjunct_count=" << delete_conjuncts.size() + << ", count_star_placeholder_columns={"; + const char* delimiter = ""; + for (const auto column_id : count_star_placeholder_columns) { + out << delimiter << column_id.value(); + delimiter = ","; + } + out << "}}"; return out.str(); } diff --git a/be/src/format_v2/file_reader.h b/be/src/format_v2/file_reader.h index 59f684121ce1e3..96d940702486b4 100644 --- a/be/src/format_v2/file_reader.h +++ b/be/src/format_v2/file_reader.h @@ -77,6 +77,18 @@ struct FileScanRequest { // Delete predicates converted to file-local expressions. A TRUE result means that the row is // deleted, so readers must invert each result when building their keep filter. VExprContextSPtrs delete_conjuncts; + // File-local ids retained only because Nereids keeps a minimum-width output tuple for an + // explicit COUNT(*). These columns have no semantic value: for example, after pruning a scan + // may retain an unsupported TIME_MILLIS leaf even though COUNT(*) only needs one row per + // surviving input row. A reader may synthesize defaults instead of reading a marked column + // while it remains non-predicate. If filters or equality deletes promote the same id to + // predicate_columns, the value is semantically required and must still be validated and read. + std::vector count_star_placeholder_columns; + + bool is_count_star_placeholder(LocalColumnId column_id) const { + return std::ranges::find(count_star_placeholder_columns, column_id) != + count_star_placeholder_columns.end(); + } }; // Helper for constructing the scan-column layout in FileScanRequest. diff --git a/be/src/format_v2/native/native_reader.cpp b/be/src/format_v2/native/native_reader.cpp index 0e348f3de2ccf5..5d0984084a6d41 100644 --- a/be/src/format_v2/native/native_reader.cpp +++ b/be/src/format_v2/native/native_reader.cpp @@ -231,6 +231,16 @@ Status NativeReader::_read_next_pblock(std::string* buffer, bool* eof) const { return Status::OK(); } + const auto remaining_prefix_bytes = _file_size - _current_offset; + if (remaining_prefix_bytes < sizeof(uint64_t)) { + // Header-only is the one valid empty-file representation and returned above. Once any + // prefix byte exists, all eight bytes are mandatory. For example, `header + 4 bytes` is a + // truncated Native file, not EOF that FileScannerV2 may skip as an empty split. + return Status::InternalError( + "truncated native block length in file {} at offset {}, expect {}, remaining {}", + _file_description->path, _current_offset, sizeof(uint64_t), remaining_prefix_bytes); + } + uint64_t block_len = 0; Slice len_slice(reinterpret_cast(&block_len), sizeof(block_len)); size_t bytes_read = 0; @@ -247,8 +257,22 @@ Status NativeReader::_read_next_pblock(std::string* buffer, bool* eof) const { } _current_offset += sizeof(block_len); if (block_len == 0) { - *eof = (_current_offset >= _file_size); - return Status::OK(); + // A header-only file reaches the `_current_offset >= _file_size` branch above and is a + // valid empty Native file. Once an explicit length prefix is present, however, zero is not + // an EOF marker in the Native format. For example, `header + uint64(0)` is truncated input, + // not a zero-row split, and must not escape as EOF for FileScannerV2 to count as empty. + return Status::InternalError("zero-length native block in file {} at offset {}", + _file_description->path, _current_offset - sizeof(block_len)); + } + + const auto remaining_block_bytes = cast_set(_file_size - _current_offset); + if (block_len > remaining_block_bytes) { + // Validate before allocating `block_len` bytes. Besides reporting a precise corruption + // error, this prevents a truncated file with a damaged length prefix from requesting an + // allocation much larger than the physical file. + return Status::InternalError( + "truncated native block body in file {} at offset {}, expect {}, remaining {}", + _file_description->path, _current_offset, block_len, remaining_block_bytes); } buffer->assign(block_len, '\0'); diff --git a/be/src/format_v2/parquet/parquet_column_schema.cpp b/be/src/format_v2/parquet/parquet_column_schema.cpp index b42d47987a54cb..1cdfed80bd273b 100644 --- a/be/src/format_v2/parquet/parquet_column_schema.cpp +++ b/be/src/format_v2/parquet/parquet_column_schema.cpp @@ -358,11 +358,16 @@ Status build_node_schema_with_mode(const ::parquet::SchemaDescriptor& schema, } column_schema->type_descriptor = resolve_parquet_type(column_schema->descriptor); column_schema->type = column_schema->type_descriptor.doris_type; + if (column_schema->type == nullptr && + !column_schema->type_descriptor.unsupported_reason.empty()) { + // Keep unsupported logical leaves in the file schema using their physical storage + // type. For example, a file `{id: INT32, clock: TIME_MILLIS}` remains readable for + // `SELECT id`: schema mapping sees `clock` as its physical INT32 but never creates its + // reader. `SELECT clock` still fails explicitly in ParquetColumnReaderFactory before + // any physical value is decoded, preserving the unsupported-type contract. + column_schema->type = column_schema->type_descriptor.physical_doris_type; + } if (column_schema->type == nullptr) { - if (!column_schema->type_descriptor.unsupported_reason.empty()) { - return Status::NotSupported("Unsupported parquet column '{}': {}", node.name(), - column_schema->type_descriptor.unsupported_reason); - } return Status::NotSupported("Unsupported parquet column type for column {}", node.name()); } diff --git a/be/src/format_v2/parquet/parquet_file_context.cpp b/be/src/format_v2/parquet/parquet_file_context.cpp index 52151c7942af2b..aa8f2622ade43b 100644 --- a/be/src/format_v2/parquet/parquet_file_context.cpp +++ b/be/src/format_v2/parquet/parquet_file_context.cpp @@ -27,7 +27,6 @@ #include #include #include -#include #include #include "common/check.h" @@ -45,6 +44,75 @@ namespace doris::format::parquet { namespace detail { +namespace { + +bool page_cache_range_less(const ParquetPageCacheRange& lhs, const ParquetPageCacheRange& rhs) { + return lhs.offset < rhs.offset || (lhs.offset == rhs.offset && lhs.size < rhs.size); +} + +} // namespace + +ParquetPageCacheRangeIndex::ParquetPageCacheRangeIndex(size_t max_ranges) + : _max_ranges(max_ranges) { + DORIS_CHECK(_max_ranges > 0); +} + +void ParquetPageCacheRangeIndex::insert(ParquetPageCacheRange range) { + std::lock_guard lock(_mutex); + auto it = std::lower_bound(_ranges.begin(), _ranges.end(), range, page_cache_range_less); + if (it != _ranges.end() && it->offset == range.offset && it->size == range.size) { + return; + } + if (_ranges.size() >= _max_ranges) { + _ranges.erase(_ranges.begin()); + it = std::lower_bound(_ranges.begin(), _ranges.end(), range, page_cache_range_less); + } + _ranges.insert(it, range); +} + +void ParquetPageCacheRangeIndex::erase(ParquetPageCacheRange range) { + std::lock_guard lock(_mutex); + const auto it = std::lower_bound(_ranges.begin(), _ranges.end(), range, page_cache_range_less); + if (it != _ranges.end() && it->offset == range.offset && it->size == range.size) { + _ranges.erase(it); + } +} + +std::vector ParquetPageCacheRangeIndex::ranges() const { + std::lock_guard lock(_mutex); + return _ranges; +} + +size_t ParquetPageCacheRangeIndex::size() const { + std::lock_guard lock(_mutex); + return _ranges.size(); +} + +ParquetPageCacheRangeDirectory::ParquetPageCacheRangeDirectory(size_t max_files) + : _max_files(max_files) { + DORIS_CHECK(_max_files > 0); +} + +std::shared_ptr ParquetPageCacheRangeDirectory::get_or_create( + const std::string& file_key) { + DORIS_CHECK(!file_key.empty()); + std::lock_guard lock(_mutex); + if (const auto it = _indexes.find(file_key); it != _indexes.end()) { + return it->second; + } + if (_indexes.size() >= _max_files) { + _indexes.erase(_indexes.begin()); + } + auto index = std::make_shared(); + _indexes.emplace(file_key, index); + return index; +} + +size_t ParquetPageCacheRangeDirectory::size() const { + std::lock_guard lock(_mutex); + return _indexes.size(); +} + std::vector plan_page_cache_range_read( int64_t position, int64_t nbytes, const std::vector& cached_ranges) { if (position < 0 || nbytes <= 0) { @@ -133,59 +201,14 @@ bool should_use_merge_range_reader(const std::vector& ran namespace { -// StoragePageCache only supports exact-key lookup. Keep lightweight range metadata here so later -// Arrow ReadAt requests can reuse cached bytes when their requested ranges are subsets of, or are -// fully covered by, previously cached ranges. Stale metadata is pruned on lookup. -std::mutex cached_page_range_index_mutex; -std::unordered_map> cached_page_range_index; -constexpr size_t MAX_CACHED_PAGE_RANGE_FILES = 4096; -constexpr size_t MAX_CACHED_PAGE_RANGES_PER_FILE = 65536; - -void register_cached_page_range(const std::string& file_key, int64_t position, int64_t nbytes) { - DORIS_CHECK(nbytes > 0); - std::lock_guard lock(cached_page_range_index_mutex); - if (cached_page_range_index.find(file_key) == cached_page_range_index.end() && - cached_page_range_index.size() >= MAX_CACHED_PAGE_RANGE_FILES) { - cached_page_range_index.erase(cached_page_range_index.begin()); - } - auto& ranges = cached_page_range_index[file_key]; - auto it = std::find_if(ranges.begin(), ranges.end(), [&](const ParquetPageCacheRange& range) { - return range.offset == position && range.size == nbytes; - }); - if (it == ranges.end()) { - if (ranges.size() >= MAX_CACHED_PAGE_RANGES_PER_FILE) { - ranges.erase(ranges.begin()); - } - ranges.push_back(ParquetPageCacheRange {position, nbytes}); - } -} - -void unregister_cached_page_range(const std::string& file_key, - const ParquetPageCacheRange& stale_range) { - std::lock_guard lock(cached_page_range_index_mutex); - auto it = cached_page_range_index.find(file_key); - if (it == cached_page_range_index.end()) { - return; - } - auto& ranges = it->second; - ranges.erase(std::remove_if(ranges.begin(), ranges.end(), - [&](const ParquetPageCacheRange& range) { - return range.offset == stale_range.offset && - range.size == stale_range.size; - }), - ranges.end()); - if (ranges.empty()) { - cached_page_range_index.erase(it); - } -} - -std::vector cached_page_ranges_for_file(const std::string& file_key) { - std::lock_guard lock(cached_page_range_index_mutex); - auto it = cached_page_range_index.find(file_key); - if (it == cached_page_range_index.end()) { - return {}; - } - return it->second; +detail::ParquetPageCacheRangeDirectory& cached_page_range_directory() { + // Directory lookup is paid once when a reader opens. ReadAt() then synchronizes only on the + // shared index for this file, so unrelated Parquet files no longer serialize on a process-wide + // hot-path mutex. Strong references deliberately keep range discovery alive after reader A + // closes: reader B can reuse cached [100, 200) for a request [120, 150). The directory and each + // per-file index are independently capped, bounding stale metadata left by page-cache eviction. + static detail::ParquetPageCacheRangeDirectory directory; + return directory; } std::string build_page_cache_file_key(const io::FileReader& file_reader, @@ -215,7 +238,11 @@ class DorisRandomAccessFile final : public arrow::io::RandomAccessFile { _base_file_reader(_file_reader), _io_ctx(io_ctx), _enable_page_cache(enable_page_cache), - _page_cache_file_key(std::move(page_cache_file_key)) { + _page_cache_file_key(std::move(page_cache_file_key)), + _cached_page_range_index( + _enable_page_cache && !_page_cache_file_key.empty() + ? cached_page_range_directory().get_or_create(_page_cache_file_key) + : nullptr) { DORIS_CHECK(_file_reader != nullptr); if (auto tracing_reader = std::dynamic_pointer_cast(_file_reader)) { _file_reader_stats = tracing_reader->stats(); @@ -228,6 +255,10 @@ class DorisRandomAccessFile final : public arrow::io::RandomAccessFile { arrow::Status Close() override { if (!_closed) { collect_active_merge_range_profile(); + std::lock_guard lock(_page_cache_mutex); + // Page payloads and their bounded per-file range index intentionally outlive this + // reader for warm scans. Only reader-specific projected ranges are released here. + std::vector().swap(_page_cache_ranges); _closed = true; } return arrow::Status::OK(); @@ -365,7 +396,8 @@ class DorisRandomAccessFile final : public arrow::io::RandomAccessFile { private: bool page_cache_enabled() const { return _enable_page_cache && !config::disable_storage_page_cache && - StoragePageCache::instance() != nullptr && !_page_cache_file_key.empty(); + StoragePageCache::instance() != nullptr && !_page_cache_file_key.empty() && + _cached_page_range_index != nullptr; } bool range_in_page_cache_scope(int64_t position, int64_t nbytes) const { @@ -393,7 +425,7 @@ class DorisRandomAccessFile final : public arrow::io::RandomAccessFile { if (!StoragePageCache::instance()->lookup( page_cache_key(cached_range.offset, cached_range.size), &handle, segment_v2::DATA_PAGE)) { - unregister_cached_page_range(_page_cache_file_key, cached_range); + _cached_page_range_index->erase(cached_range); return false; } Slice cached = handle.data(); @@ -406,8 +438,8 @@ class DorisRandomAccessFile final : public arrow::io::RandomAccessFile { } bool try_read_from_cached_ranges(int64_t position, int64_t nbytes, void* out) { - auto plan = detail::plan_page_cache_range_read( - position, nbytes, cached_page_ranges_for_file(_page_cache_file_key)); + auto plan = detail::plan_page_cache_range_read(position, nbytes, + _cached_page_range_index->ranges()); if (plan.empty()) { return false; } @@ -458,7 +490,8 @@ class DorisRandomAccessFile final : public arrow::io::RandomAccessFile { PageCacheHandle handle; StoragePageCache::instance()->insert(page_cache_key(position, nbytes), page, &handle, segment_v2::DATA_PAGE); - register_cached_page_range(_page_cache_file_key, position, nbytes); + _cached_page_range_index->insert( + ParquetPageCacheRange {.offset = position, .size = nbytes}); ++_page_cache_stats.write_count; ++_page_cache_stats.compressed_write_count; } @@ -516,6 +549,7 @@ class DorisRandomAccessFile final : public arrow::io::RandomAccessFile { std::string _page_cache_file_key; mutable std::mutex _page_cache_mutex; std::vector _page_cache_ranges; + std::shared_ptr _cached_page_range_index; ParquetPageCacheStats _page_cache_stats; }; diff --git a/be/src/format_v2/parquet/parquet_file_context.h b/be/src/format_v2/parquet/parquet_file_context.h index 0dca52244957d7..67e9102139dbf2 100644 --- a/be/src/format_v2/parquet/parquet_file_context.h +++ b/be/src/format_v2/parquet/parquet_file_context.h @@ -21,6 +21,9 @@ #include #include #include +#include +#include +#include #include #include "common/status.h" @@ -66,6 +69,39 @@ struct ParquetPageCacheStats { namespace detail { +class ParquetPageCacheRangeIndex { +public: + static constexpr size_t DEFAULT_MAX_RANGES = 65536; + + explicit ParquetPageCacheRangeIndex(size_t max_ranges = DEFAULT_MAX_RANGES); + + void insert(ParquetPageCacheRange range); + void erase(ParquetPageCacheRange range); + + std::vector ranges() const; + size_t size() const; + +private: + const size_t _max_ranges; + mutable std::mutex _mutex; + std::vector _ranges; +}; + +class ParquetPageCacheRangeDirectory { +public: + static constexpr size_t DEFAULT_MAX_FILES = 4096; + + explicit ParquetPageCacheRangeDirectory(size_t max_files = DEFAULT_MAX_FILES); + + std::shared_ptr get_or_create(const std::string& file_key); + size_t size() const; + +private: + const size_t _max_files; + mutable std::mutex _mutex; + std::unordered_map> _indexes; +}; + // Build the copy plan for a ReadAt(position, nbytes) request from the range metadata of // previously cached entries. // StoragePageCache cannot do range lookup by itself; it can only lookup an exact key. The diff --git a/be/src/format_v2/parquet/parquet_reader.cpp b/be/src/format_v2/parquet/parquet_reader.cpp index 753b3628bfa19b..cd84ab74ef953e 100644 --- a/be/src/format_v2/parquet/parquet_reader.cpp +++ b/be/src/format_v2/parquet/parquet_reader.cpp @@ -114,8 +114,66 @@ void collect_request_leaf_column_ids( collect_scan_column(column); } for (const auto& column : request.non_predicate_columns) { - collect_scan_column(column); + if (!request.is_count_star_placeholder(column.column_id())) { + collect_scan_column(column); + } + } +} + +Status validate_all_projected_leaves_supported(const ParquetColumnSchema& column_schema) { + if (column_schema.kind == ParquetColumnSchemaKind::PRIMITIVE) { + if (!column_schema.type_descriptor.unsupported_reason.empty()) { + return Status::NotSupported("Unsupported parquet column '{}': {}", column_schema.name, + column_schema.type_descriptor.unsupported_reason); + } + return Status::OK(); + } + for (const auto& child : column_schema.children) { + DORIS_CHECK(child != nullptr); + RETURN_IF_ERROR(validate_all_projected_leaves_supported(*child)); + } + return Status::OK(); +} + +Status validate_projected_leaves_supported(const ParquetColumnSchema& column_schema, + const format::LocalColumnIndex& projection) { + if (column_schema.kind == ParquetColumnSchemaKind::PRIMITIVE || + projection.project_all_children || projection.children.empty()) { + return validate_all_projected_leaves_supported(column_schema); + } + for (const auto& child_projection : projection.children) { + const auto child_it = + std::ranges::find_if(column_schema.children, [&](const auto& child_schema) { + return child_schema->local_id == child_projection.local_id(); + }); + DORIS_CHECK(child_it != column_schema.children.end()); + RETURN_IF_ERROR(validate_projected_leaves_supported(**child_it, child_projection)); + } + return Status::OK(); +} + +Status validate_requested_columns_supported( + const std::vector>& file_schema, + const format::FileScanRequest& request) { + auto validate_scan_column = [&](const format::LocalColumnIndex& projection) -> Status { + const auto local_id = projection.local_id(); + if (local_id == format::ROW_POSITION_COLUMN_ID || + local_id == format::GLOBAL_ROWID_COLUMN_ID) { + return Status::OK(); + } + DORIS_CHECK(local_id >= 0 && local_id < static_cast(file_schema.size())); + DORIS_CHECK(file_schema[local_id] != nullptr); + return validate_projected_leaves_supported(*file_schema[local_id], projection); + }; + for (const auto& column : request.predicate_columns) { + RETURN_IF_ERROR(validate_scan_column(column)); + } + for (const auto& column : request.non_predicate_columns) { + if (!request.is_count_star_placeholder(column.column_id())) { + RETURN_IF_ERROR(validate_scan_column(column)); + } } + return Status::OK(); } std::vector build_page_cache_ranges( @@ -454,6 +512,12 @@ Status ParquetReader::open(std::shared_ptr request) { DORIS_CHECK(local_id >= 0 && local_id < num_fields); } + // Reject requested unsupported logical leaves before row-group statistics, dictionaries, + // bloom filters or page indexes inspect their physical fallback type. For example, a predicate + // on TIME_MILLIS must fail here even when its INT32 statistics would prune every row group; + // otherwise the same unsupported SELECT could fail or silently succeed depending on data. + RETURN_IF_ERROR(validate_requested_columns_supported(_state->file_schema, *request_snapshot)); + RowGroupScanPlan row_group_plan; ParquetScanRange scan_range; scan_range.start_offset = _file_description->range_start_offset; @@ -598,6 +662,19 @@ Status ParquetReader::get_aggregate_result(const format::FileAggregateRequest& r request.agg_type); } + for (const auto& aggregate_column : request.columns) { + const auto local_id = aggregate_column.projection.local_id(); + if (local_id < 0 || local_id >= static_cast(_state->file_schema.size())) { + return Status::InvalidArgument("Invalid parquet aggregate column id {}", local_id); + } + DORIS_CHECK(_state->file_schema[local_id] != nullptr); + // Aggregate pushdown can return directly from footer statistics without constructing a + // column reader. Validate first so MIN/MAX(TIME_MILLIS), or an all-pruned COUNT request, + // cannot expose the physical INT32 fallback as a supported logical value. + RETURN_IF_ERROR(validate_projected_leaves_supported(*_state->file_schema[local_id], + aggregate_column.projection)); + } + // Aggregate row count in all selected row groups. For MIN/MAX aggregate, this is used to determine whether there is no row group selected. for (const auto& row_group_plan : _state->scan_plan.row_groups) { auto row_group_metadata = @@ -614,6 +691,16 @@ Status ParquetReader::get_aggregate_result(const format::FileAggregateRequest& r } const auto& count_projection = request.columns[0].projection; const auto& root_schema = projected_root_schema(_state->file_schema, count_projection); + // A required primitive COUNT(col) still carries its projection so the unsupported-type + // validation above cannot be bypassed. Once validated, its definition level proves that + // every selected row is non-NULL, so preserve the already-computed footer row count and + // avoid reading definition levels merely to rediscover COUNT(col) == COUNT(*). Complex + // roots continue through the shape reader because their count semantics and read-row + // accounting are derived from nested levels. + if (root_schema.kind == ParquetColumnSchemaKind::PRIMITIVE && + root_schema.max_definition_level == 0) { + return Status::OK(); + } result->count = 0; for (const auto& row_group_plan : _state->scan_plan.row_groups) { std::shared_ptr<::parquet::RowGroupReader> row_group; diff --git a/be/src/format_v2/parquet/parquet_scan.cpp b/be/src/format_v2/parquet/parquet_scan.cpp index 1abddc777aa442..eafb741a53161d 100644 --- a/be/src/format_v2/parquet/parquet_scan.cpp +++ b/be/src/format_v2/parquet/parquet_scan.cpp @@ -177,11 +177,41 @@ std::vector request_scan_columns(const format::FileSca scan_columns.reserve(request.predicate_columns.size() + request.non_predicate_columns.size()); scan_columns.insert(scan_columns.end(), request.predicate_columns.begin(), request.predicate_columns.end()); - scan_columns.insert(scan_columns.end(), request.non_predicate_columns.begin(), - request.non_predicate_columns.end()); + for (const auto& column : request.non_predicate_columns) { + if (!request.is_count_star_placeholder(column.column_id())) { + scan_columns.push_back(column); + } + } return scan_columns; } +std::vector physical_non_predicate_columns( + const format::FileScanRequest& request) { + std::vector columns; + columns.reserve(request.non_predicate_columns.size()); + for (const auto& column : request.non_predicate_columns) { + if (!request.is_count_star_placeholder(column.column_id())) { + columns.push_back(column); + } + } + return columns; +} + +void materialize_count_star_placeholders(const format::FileScanRequest& request, size_t rows, + Block* file_block) { + DORIS_CHECK(file_block != nullptr); + for (const auto& column : request.non_predicate_columns) { + if (!request.is_count_star_placeholder(column.column_id())) { + continue; + } + const auto block_position = request.local_positions.at(column.column_id()).value(); + auto placeholder = file_block->get_by_position(block_position).column->assert_mutable(); + DCHECK(placeholder->empty()); + placeholder->insert_many_defaults(rows); + file_block->replace_by_position(block_position, std::move(placeholder)); + } +} + std::vector build_row_group_prefetch_ranges( const ::parquet::FileMetaData& metadata, const std::vector>& file_schema, @@ -750,6 +780,9 @@ Status ParquetScanScheduler::open_next_row_group( } for (const auto& col : request.non_predicate_columns) { const auto local_id = col.local_id(); + if (request.is_count_star_placeholder(col.column_id())) { + continue; + } if (local_id == format::ROW_POSITION_COLUMN_ID) { _current_non_predicate_columns[local_id] = column_reader_factory.create_row_position_column_reader( @@ -775,7 +808,8 @@ Status ParquetScanScheduler::open_next_row_group( // With no row-level filters there is no lazy-read decision to wait for, so start warming // output chunks immediately after their readers are created. Filtered scans still defer // this until at least one row survives the predicate phase. - prefetch_current_row_group_columns(file_context, file_schema, request.non_predicate_columns, + prefetch_current_row_group_columns(file_context, file_schema, + physical_non_predicate_columns(request), &_current_non_predicate_prefetched); } *has_row_group = true; @@ -1374,6 +1408,7 @@ Status ParquetScanScheduler::read_current_row_group_batch( _raw_rows_read += batch_rows; if (_current_predicate_columns.empty() && _current_non_predicate_columns.empty()) { *rows = static_cast(batch_rows); + materialize_count_star_placeholders(request, *rows, file_block); if (_scan_profile.selected_rows != nullptr) { COUNTER_UPDATE(_scan_profile.selected_rows, batch_rows); } @@ -1431,7 +1466,8 @@ Status ParquetScanScheduler::read_current_row_group_batch( // Do not prefetch lazy output columns until at least one row survives filtering. This is // the same decision point where the v2 reader switches from predicate-only reads to // materializing non-predicate columns, so fully filtered batches avoid unnecessary IO. - prefetch_current_row_group_columns(file_context, file_schema, request.non_predicate_columns, + prefetch_current_row_group_columns(file_context, file_schema, + physical_non_predicate_columns(request), &_current_non_predicate_prefetched); } @@ -1472,6 +1508,7 @@ Status ParquetScanScheduler::read_current_row_group_batch( file_block->replace_by_position(block_position, std::move(column)); } } + materialize_count_star_placeholders(request, selected_rows, file_block); *rows = static_cast(selected_rows); return Status::OK(); } diff --git a/be/src/format_v2/parquet/parquet_type.cpp b/be/src/format_v2/parquet/parquet_type.cpp index d35181d0397178..8411d53f0fba91 100644 --- a/be/src/format_v2/parquet/parquet_type.cpp +++ b/be/src/format_v2/parquet/parquet_type.cpp @@ -293,6 +293,7 @@ ParquetTypeDescriptor resolve_parquet_type(const ::parquet::ColumnDescriptor* co result.physical_type = column->physical_type(); result.converted_type = column->converted_type(); result.fixed_length = column->type_length(); + result.physical_doris_type = physical_type_to_doris_type(column); if (auto logical_type = logical_type_to_doris_type(column, &result); logical_type != nullptr) { result.doris_type = logical_type; @@ -306,7 +307,7 @@ ParquetTypeDescriptor resolve_parquet_type(const ::parquet::ColumnDescriptor* co result.doris_type = nullptr; result.supports_record_reader = false; } else { - result.doris_type = physical_type_to_doris_type(column); + result.doris_type = result.physical_doris_type; if (result.physical_type == ::parquet::Type::INT96) { result.extra_type_info = ParquetExtraTypeInfo::IMPALA_TIMESTAMP; } diff --git a/be/src/format_v2/parquet/parquet_type.h b/be/src/format_v2/parquet/parquet_type.h index 5d21aae6bae092..a4d99abc0e982a 100644 --- a/be/src/format_v2/parquet/parquet_type.h +++ b/be/src/format_v2/parquet/parquet_type.h @@ -54,6 +54,9 @@ enum class ParquetTimeUnit { // ============================================================================ struct ParquetTypeDescriptor { DataTypePtr doris_type; + // Physical fallback used only to keep file schema construction alive when the logical type is + // unsupported. Column reader creation still rejects unsupported_reason before decoding. + DataTypePtr physical_doris_type; ParquetExtraTypeInfo extra_type_info = ParquetExtraTypeInfo::NONE; ParquetTimeUnit time_unit = ParquetTimeUnit::UNKNOWN; ::parquet::Type::type physical_type = ::parquet::Type::UNDEFINED; diff --git a/be/src/format_v2/table/hudi_reader.cpp b/be/src/format_v2/table/hudi_reader.cpp index 4294d51d043d9e..6c8917d867074c 100644 --- a/be/src/format_v2/table/hudi_reader.cpp +++ b/be/src/format_v2/table/hudi_reader.cpp @@ -81,6 +81,11 @@ bool HudiHybridReader::current_split_pruned() const { return _current_split_reader->current_split_pruned(); } +bool HudiHybridReader::current_split_uses_metadata_count() const { + DORIS_CHECK(_current_split_reader != nullptr); + return _current_split_reader->current_split_uses_metadata_count(); +} + Status HudiHybridReader::abort_split() { DORIS_CHECK(_current_split_reader != nullptr); return _current_split_reader->abort_split(); @@ -145,6 +150,7 @@ Status HudiHybridReader::_init_child_reader(format::TableReader* reader, .runtime_state = _runtime_state, .scanner_profile = _scanner_profile, .push_down_agg_type = _push_down_agg_type, + .push_down_count_columns = _push_down_count_columns, .condition_cache_digest = _condition_cache_digest, })); // Zero means no adaptive prediction has been produced yet. Preserve the child's normal diff --git a/be/src/format_v2/table/hudi_reader.h b/be/src/format_v2/table/hudi_reader.h index e22c6bd866f061..77a71cd79ba166 100644 --- a/be/src/format_v2/table/hudi_reader.h +++ b/be/src/format_v2/table/hudi_reader.h @@ -60,6 +60,7 @@ class HudiHybridReader final : public format::TableReader { Status prepare_split(const format::SplitReadOptions& options) override; Status get_block(Block* block, bool* eos) override; bool current_split_pruned() const override; + bool current_split_uses_metadata_count() const override; Status abort_split() override; Status close() override; void set_batch_size(size_t batch_size) override; diff --git a/be/src/format_v2/table/iceberg_reader.cpp b/be/src/format_v2/table/iceberg_reader.cpp index 962a414a4c23f1..75df8e4bd6cf20 100644 --- a/be/src/format_v2/table/iceberg_reader.cpp +++ b/be/src/format_v2/table/iceberg_reader.cpp @@ -568,14 +568,13 @@ void IcebergTableReader::_append_equality_delete_row_count_carrier( DORIS_CHECK(request != nullptr); // Columnar readers establish a filter batch's row count from predicate columns. If all // equality keys are missing, the predicate consists only of NULL literals and the filter block - // would otherwise have zero rows. Read one physical column eagerly as a row-count carrier; - // normal final materialization ignores this hidden dependency. - const auto carrier_it = std::ranges::find_if( - _data_reader.file_schema, [](const format::ColumnDefinition& field) { - return field.column_type == format::ColumnType::DATA_COLUMN; - }); - DORIS_CHECK(carrier_it != _data_reader.file_schema.end()); - _append_file_scan_column(request, format::LocalColumnId(carrier_it->file_local_id()), + // would otherwise have zero rows. Use the virtual row-position column as the carrier instead + // of an arbitrary physical column. For example, a data file may start with an unsupported + // TIME_MILLIS leaf while the query projects only a supported `id`; selecting that TIME leaf as + // a hidden carrier would make Parquet reject a column the query never requested. Row position + // has one value per input row in both Parquet and ORC, is already used by delete predicates, + // and is explicitly excluded from physical logical-type validation. + _append_file_scan_column(request, format::LocalColumnId(format::ROW_POSITION_COLUMN_ID), &request->predicate_columns); } diff --git a/be/src/format_v2/table/paimon_reader.cpp b/be/src/format_v2/table/paimon_reader.cpp index 258bbb5c021dc0..74a38515622f81 100644 --- a/be/src/format_v2/table/paimon_reader.cpp +++ b/be/src/format_v2/table/paimon_reader.cpp @@ -106,6 +106,11 @@ bool PaimonHybridReader::current_split_pruned() const { return _current_split_reader->current_split_pruned(); } +bool PaimonHybridReader::current_split_uses_metadata_count() const { + DORIS_CHECK(_current_split_reader != nullptr); + return _current_split_reader->current_split_uses_metadata_count(); +} + Status PaimonHybridReader::abort_split() { DORIS_CHECK(_current_split_reader != nullptr); return _current_split_reader->abort_split(); @@ -173,6 +178,7 @@ Status PaimonHybridReader::_init_child_reader(format::TableReader* reader, .runtime_state = _runtime_state, .scanner_profile = _scanner_profile, .push_down_agg_type = _push_down_agg_type, + .push_down_count_columns = _push_down_count_columns, .condition_cache_digest = _condition_cache_digest, })); // Zero means no adaptive prediction has been produced yet. Preserve the child's normal diff --git a/be/src/format_v2/table/paimon_reader.h b/be/src/format_v2/table/paimon_reader.h index fc94777b6dafb3..634f2bd7a6400c 100644 --- a/be/src/format_v2/table/paimon_reader.h +++ b/be/src/format_v2/table/paimon_reader.h @@ -66,6 +66,7 @@ class PaimonHybridReader final : public format::TableReader { Status prepare_split(const format::SplitReadOptions& options) override; Status get_block(Block* block, bool* eos) override; bool current_split_pruned() const override; + bool current_split_uses_metadata_count() const override; Status abort_split() override; Status close() override; void set_batch_size(size_t batch_size) override; diff --git a/be/src/format_v2/table_reader.cpp b/be/src/format_v2/table_reader.cpp index 5f37538c1f2639..e8725ed65fa49b 100644 --- a/be/src/format_v2/table_reader.cpp +++ b/be/src/format_v2/table_reader.cpp @@ -527,6 +527,7 @@ Status TableReader::init(TableReadOptions&& options) { _scanner_profile = options.scanner_profile; _file_slot_descs = options.file_slot_descs; _push_down_agg_type = options.push_down_agg_type; + _push_down_count_columns = options.push_down_count_columns; _initial_condition_cache_digest = options.condition_cache_digest; _condition_cache_digest = _initial_condition_cache_digest; _projected_columns = std::move(options.projected_columns); @@ -861,6 +862,7 @@ Status TableReader::prepare_split(const SplitReadOptions& options) { _deletion_vector = nullptr; _aggregate_pushdown_tried = false; _remaining_table_level_count = -1; + _current_split_uses_metadata_count = false; _current_reader_reached_eof = false; RETURN_IF_ERROR(_evaluate_partition_prune_conjuncts(options.partition_prune_conjuncts, &_current_split_pruned)); @@ -875,12 +877,18 @@ Status TableReader::prepare_split(const SplitReadOptions& options) { // active and no predicate can arrive later. The metadata path can return several batches for // one split; after its first synthetic batch there is no way to recover the real rows if a // runtime filter arrives before the next scheduler turn. - if (_push_down_agg_type == TPushAggOp::type::COUNT && options.all_runtime_filters_applied && + // Table-level metadata only contains the number of rows; it cannot evaluate an expression or + // the NULL state of a COUNT argument. Require the new FE's explicit empty argument list, which + // means COUNT(*)/COUNT(1). A non-empty list means COUNT(col), while nullopt comes from an old FE + // whose COUNT semantics are unknown during a BE-first rolling upgrade. + if (_push_down_agg_type == TPushAggOp::type::COUNT && _push_down_count_columns.has_value() && + _push_down_count_columns->empty() && options.all_runtime_filters_applied && _conjuncts.empty() && options.current_range.__isset.table_format_params && options.current_range.table_format_params.__isset.table_level_row_count) { DORIS_CHECK(options.current_range.table_format_params.table_level_row_count >= -1); _remaining_table_level_count = options.current_range.table_format_params.table_level_row_count; + _current_split_uses_metadata_count = _is_table_level_count_active(); } if (_is_table_level_count_active()) { return Status::OK(); diff --git a/be/src/format_v2/table_reader.h b/be/src/format_v2/table_reader.h index 73bade81bc4c65..cddddccd6bd3e1 100644 --- a/be/src/format_v2/table_reader.h +++ b/be/src/format_v2/table_reader.h @@ -135,6 +135,10 @@ struct TableReadOptions { const std::vector* file_slot_descs = nullptr; // Push-down aggregate type. const TPushAggOp::type push_down_agg_type = TPushAggOp::type::NONE; + // Table/global indices of explicit COUNT arguments. nullopt means an old FE did not send the + // semantic argument field, while an explicit empty vector means COUNT(*)/COUNT(1). Keeping + // those states separate prevents a rolling-upgrade plan from being reinterpreted by a new BE. + const std::optional> push_down_count_columns = std::nullopt; // Initial digest of predicates available during scanner open. Scanner-driven splits override it // with SplitReadOptions::condition_cache_digest after collecting late-arrival runtime filters. // A zero digest disables condition cache. @@ -147,7 +151,7 @@ struct SplitReadOptions { // Latest scanner conjuncts rewritten to table/global column indices. Runtime filters may // arrive after TableReader::init(), so scanner-driven splits replace the initial snapshot. // nullopt preserves the initial snapshot for standalone TableReader callers. - std::optional conjuncts; + std::optional conjuncts = std::nullopt; // Independent clones used for partition pruning because evaluation prepares and opens them // against a synthetic partition block before the file reader opens its row-level conjuncts. VExprContextSPtrs partition_prune_conjuncts; @@ -207,6 +211,9 @@ class TableReader { virtual Status prepare_split(const SplitReadOptions& options); virtual bool current_split_pruned() const { return _current_split_pruned; } + virtual bool current_split_uses_metadata_count() const { + return _current_split_uses_metadata_count; + } // Discard the active split after the caller decides an error is ignorable, for example a // stale external-table file listing that returns NOT_FOUND. The next prepare_split() must start @@ -220,6 +227,7 @@ class TableReader { } _delete_rows = nullptr; _remaining_table_level_count = -1; + _current_split_uses_metadata_count = false; _current_split_pruned = false; return Status::OK(); } @@ -315,6 +323,7 @@ class TableReader { _current_task.reset(); _current_file_description.reset(); _remaining_table_level_count = -1; + _current_split_uses_metadata_count = false; return Status::OK(); } @@ -398,6 +407,20 @@ class TableReader { RETURN_IF_ERROR(close_current_reader()); return Status::OK(); } + // COUNT(*) has no semantic column argument, but Nereids retains a minimum-width scan slot + // so the scan node still has an output tuple. Record only the current non-predicate file + // columns before table-format hooks add row-position or equality-delete dependencies. This + // marker is independent of aggregate eligibility: with position deletes, for example, + // metadata COUNT must fall back to reading rows, but an arbitrary unsupported TIME_MILLIS + // placeholder still must not be validated or decoded merely to carry the surviving count. + if (_push_down_agg_type == TPushAggOp::type::COUNT && + _push_down_count_columns.has_value() && _push_down_count_columns->empty()) { + file_request->count_star_placeholder_columns.reserve( + file_request->non_predicate_columns.size()); + for (const auto& column : file_request->non_predicate_columns) { + file_request->count_star_placeholder_columns.push_back(column.column_id()); + } + } RETURN_IF_ERROR(customize_file_scan_request(file_request.get())); RETURN_IF_ERROR(_open_local_filter_exprs(*file_request)); _data_reader.file_block_layout.clear(); @@ -944,6 +967,9 @@ class TableReader { RETURN_IF_ERROR(status); RETURN_IF_ERROR( _materialize_aggregate_pushdown_rows(_push_down_agg_type, file_result, block)); + if (_push_down_agg_type == TPushAggOp::type::COUNT) { + _current_split_uses_metadata_count = true; + } *pushed_down = true; RETURN_IF_ERROR(close_current_reader()); return Status::OK(); @@ -979,7 +1005,31 @@ class TableReader { return false; } if (agg_type == TPushAggOp::type::COUNT) { - return true; + // Old FEs do not serialize push_down_count_slot_ids. During the supported BE-first + // rolling upgrade, nullopt therefore means "COUNT semantics are unknown", not + // COUNT(*). Fall back to reading rows until the FE explicitly sends either an empty + // list for COUNT(*) or one slot for COUNT(col). + if (!_push_down_count_columns.has_value()) { + return false; + } + // COUNT(*) needs no column metadata. COUNT(col) currently supports one direct file + // column; multiple COUNT arguments fall back to the normal scan so every upper + // aggregate receives the original rows. + if (_push_down_count_columns->empty()) { + return true; + } + if (_push_down_count_columns->size() != 1) { + return false; + } + const auto& mapping = _push_down_count_mapping(); + // Metadata COUNT skips TableReader's normal materialization path. Only a trivial + // mapping is safe: for example, a nullable Parquet INT mapped to a NOT NULL table + // BIGINT normally needs both an INT->BIGINT cast and nullability validation. Counting + // footer values directly would bypass both operations and could hide invalid data. + return mapping.file_local_id.has_value() && mapping.file_type != nullptr && + mapping.table_type != nullptr && mapping.is_trivial && + mapping.virtual_column_type == TableVirtualColumnType::INVALID && + mapping.default_expr == nullptr; } // For MIN/MAX, only support direct file-to-table column mappings. The two emitted rows // must be enough for the upper MIN/MAX aggregate without evaluating default expressions or @@ -1462,24 +1512,19 @@ class TableReader { request->agg_type = agg_type; request->columns.clear(); if (agg_type == TPushAggOp::type::COUNT) { - // COUNT pushdown historically meant COUNT(*) and therefore carried no columns. For - // complex COUNT(col), materializing the full MAP/LIST/STRUCT value only to test the - // top-level NULL bit can be extremely expensive. When the scan projects exactly one - // directly-mapped complex column, pass that file column to the reader so formats such - // as Parquet can count the column shape from metadata/levels without decoding payload - // values like MAP value strings. Other COUNT cases stay on the existing row-count path - // to avoid changing count(*) semantics. - if (_data_reader.column_mapper->mappings().size() == 1) { - const auto& mapping = _data_reader.column_mapper->mappings()[0]; - if (mapping.file_local_id.has_value() && mapping.file_type != nullptr && - is_complex_type(remove_nullable(mapping.file_type)->get_primitive_type()) && - mapping.virtual_column_type == TableVirtualColumnType::INVALID && - mapping.default_expr == nullptr) { - FileAggregateRequest::Column column; - column.projection = - LocalColumnIndex::top_level(LocalColumnId(*mapping.file_local_id)); - request->columns.push_back(std::move(column)); - } + DORIS_CHECK(_push_down_count_columns.has_value()); + // An empty explicit list is the semantic signal for COUNT(*). Do not inspect the + // mapping count: `SELECT COUNT(*) FROM t` may still project one nullable column because + // the planner keeps a placeholder slot. In a 10,000-row file where that arbitrary slot + // has 9,015 non-null values, passing the slot would ask Parquet/ORC metadata for + // COUNT(slot)=9,015 instead of the required row count 10,000. + if (!_push_down_count_columns->empty()) { + const auto& mapping = _push_down_count_mapping(); + DORIS_CHECK(mapping.file_local_id.has_value()); + FileAggregateRequest::Column column; + column.projection = + LocalColumnIndex::top_level(LocalColumnId(*mapping.file_local_id)); + request->columns.push_back(std::move(column)); } return Status::OK(); } @@ -1496,6 +1541,18 @@ class TableReader { return Status::OK(); } + const ColumnMapping& _push_down_count_mapping() const { + DORIS_CHECK(_push_down_count_columns.has_value()); + DORIS_CHECK(_push_down_count_columns->size() == 1); + const auto mapping_it = + std::ranges::find(_data_reader.column_mapper->mappings(), + _push_down_count_columns->front(), &ColumnMapping::global_index); + // FileScannerV2 translates FE SlotIds through the same projected-column list used to build + // the mapper, so a missing mapping is an FE/BE contract violation rather than a fallback. + DORIS_CHECK(mapping_it != _data_reader.column_mapper->mappings().end()); + return *mapping_it; + } + Status _materialize_aggregate_pushdown_rows(TPushAggOp::type agg_type, const FileAggregateResult& file_result, Block* block) { @@ -1600,6 +1657,7 @@ class TableReader { const std::vector* _file_slot_descs = nullptr; FileFormat _format; TPushAggOp::type _push_down_agg_type = TPushAggOp::type::NONE; + std::optional> _push_down_count_columns; size_t _batch_size = 0; uint64_t _initial_condition_cache_digest = 0; uint64_t _condition_cache_digest = 0; @@ -1613,6 +1671,10 @@ class TableReader { int64_t _condition_cache_hit_count = 0; bool _current_reader_reached_eof = false; int64_t _remaining_table_level_count = -1; + // True only after the active split selects a table-level row-count shortcut or successfully + // materializes COUNT rows from file metadata. FileScannerV2 uses this result, rather than the + // raw aggregate opcode, to keep adaptive batching enabled for normal row-scan fallbacks. + bool _current_split_uses_metadata_count = false; // Snapshot supplied by FileScannerV2 for the active split. It gates every shortcut that emits // irreversible aggregate rows, not only the table-level row-count shortcut in prepare_split(). bool _all_runtime_filters_applied_for_split = true; diff --git a/be/test/exec/scan/file_scanner_v2_test.cpp b/be/test/exec/scan/file_scanner_v2_test.cpp index 34b51a5b40da34..762598456e09e1 100644 --- a/be/test/exec/scan/file_scanner_v2_test.cpp +++ b/be/test/exec/scan/file_scanner_v2_test.cpp @@ -99,6 +99,29 @@ TFileRangeDesc legacy_paimon_jni_range_without_reader_type() { return range; } +TEST(FileScannerTest, V1CountPushdownRequiresExplicitCountStarArguments) { + EXPECT_EQ(TPushAggOp::type::COUNT, FileScanner::TEST_effective_push_down_agg_type( + TPushAggOp::type::COUNT, std::vector {})); + + // A missing field is an old FE plan with unknown COUNT semantics, not COUNT(*). + EXPECT_EQ(TPushAggOp::type::NONE, FileScanner::TEST_effective_push_down_agg_type( + TPushAggOp::type::COUNT, std::nullopt)); + // V1 cannot evaluate COUNT(col) NULL/CAST semantics before replacing the reader with + // CountReader, so an explicit argument must use the normal scan path. + EXPECT_EQ(TPushAggOp::type::NONE, FileScanner::TEST_effective_push_down_agg_type( + TPushAggOp::type::COUNT, std::vector {7})); + + // The COUNT argument field must not affect other storage-layer aggregate operations. + EXPECT_EQ(TPushAggOp::type::MINMAX, FileScanner::TEST_effective_push_down_agg_type( + TPushAggOp::type::MINMAX, std::nullopt)); +} + +TEST(FileScannerV2Test, AdaptiveBatchSizeRunsForCountFallbackOnly) { + EXPECT_TRUE(FileScannerV2::TEST_should_run_adaptive_batch_size(true, false)); + EXPECT_FALSE(FileScannerV2::TEST_should_run_adaptive_batch_size(true, true)); + EXPECT_FALSE(FileScannerV2::TEST_should_run_adaptive_batch_size(false, false)); +} + struct RetryableCloseState { int close_calls = 0; }; @@ -707,6 +730,16 @@ TEST(FileScannerV2Test, NotFoundIsSkippedOnlyWhenConfigured) { EXPECT_FALSE(FileScannerV2::TEST_should_skip_not_found(Status::OK(), true)); } +TEST(FileScannerV2Test, EndOfFileIsSkippedAsEmptySplit) { + EXPECT_TRUE(FileScannerV2::TEST_should_skip_empty(Status::EndOfFile("empty file"), false)); + // Deletion-vector and Parquet readers also use EOF to unwind an interrupted read. Once either + // scanner stop flag is visible, the same status is no longer evidence of an empty file. + EXPECT_FALSE(FileScannerV2::TEST_should_skip_empty(Status::EndOfFile("stop read."), true)); + EXPECT_FALSE( + FileScannerV2::TEST_should_skip_empty(Status::InternalError("read failed"), false)); + EXPECT_FALSE(FileScannerV2::TEST_should_skip_empty(Status::OK(), false)); +} + // Scenario: partition slots are identified from the explicit FE category when present, otherwise // from the legacy is_file_slot flag. Scanner-generated rowid columns must never be treated as // partition columns even if FE marks them as non-file slots. diff --git a/be/test/format_v2/column_mapper_test.cpp b/be/test/format_v2/column_mapper_test.cpp index 3bca3c4152c888..c0318d451187ed 100644 --- a/be/test/format_v2/column_mapper_test.cpp +++ b/be/test/format_v2/column_mapper_test.cpp @@ -28,6 +28,7 @@ #include "common/consts.h" #include "core/assert_cast.h" #include "core/block/block.h" +#include "core/column/column_struct.h" #include "core/column/column_vector.h" #include "core/data_type/data_type_array.h" #include "core/data_type/data_type_decimal.h" @@ -38,6 +39,7 @@ #include "core/data_type/data_type_struct.h" #include "core/data_type/data_type_timestamptz.h" #include "core/data_type/data_type_varbinary.h" +#include "exprs/vectorized_fn_call.h" #include "exprs/vexpr.h" #include "exprs/vexpr_context.h" #include "exprs/vin_predicate.h" @@ -316,6 +318,38 @@ class TestFunctionExpr final : public VExpr { std::string _expr_name; }; +class ExecutableStructElementExpr final : public VExpr { +public: + explicit ExecutableStructElementExpr(DataTypePtr child_type) + : VExpr(std::move(child_type), false) { + set_node_type(TExprNodeType::FUNCTION_CALL); + TFunctionName fn_name; + fn_name.__set_function_name(_expr_name); + _fn.__set_name(fn_name); + } + + const std::string& expr_name() const override { return _expr_name; } + + Status clone_node(VExprSPtr* cloned_expr) const override { + DORIS_CHECK(cloned_expr != nullptr); + *cloned_expr = std::make_shared(data_type()); + return Status::OK(); + } + + Status execute_column_impl(VExprContext* context, const Block* block, const Selector* selector, + size_t count, ColumnPtr& result_column) const override { + ColumnPtr struct_column; + RETURN_IF_ERROR( + get_child(0)->execute_column(context, block, selector, count, struct_column)); + const auto& input = assert_cast(*struct_column); + result_column = input.get_column_ptr(0); + return Status::OK(); + } + +private: + const std::string _expr_name = "element_at"; +}; + VExprSPtr table_slot(int slot_id, int column_id, DataTypePtr type, const std::string& name) { return VSlotRef::create_shared(slot_id, column_id, -1, std::move(type), name); } @@ -340,6 +374,40 @@ VExprSPtr element_at(const VExprSPtr& parent, DataTypePtr child_type, return expr; } +VExprSPtr executable_struct_element(const VExprSPtr& parent, DataTypePtr child_type, + const std::string& child_name) { + auto expr = std::make_shared(std::move(child_type)); + expr->add_child(parent); + expr->add_child(literal(str(), Field::create_field(child_name))); + return expr; +} + +VExprSPtr executable_binary_predicate(TExprOpcode::type opcode, const VExprSPtr& left, + const VExprSPtr& right) { + const auto result_type = u8(); + TFunctionName fn_name; + fn_name.__set_function_name(opcode == TExprOpcode::GT ? "gt" : "eq"); + TFunction fn; + fn.__set_name(fn_name); + fn.__set_binary_type(TFunctionBinaryType::BUILTIN); + fn.__set_arg_types({left->data_type()->to_thrift(), right->data_type()->to_thrift()}); + fn.__set_ret_type(result_type->to_thrift()); + fn.__set_has_var_args(false); + + TExprNode node; + node.__set_node_type(TExprNodeType::BINARY_PRED); + node.__set_opcode(opcode); + node.__set_type(result_type->to_thrift()); + node.__set_fn(fn); + node.__set_num_children(2); + node.__set_is_nullable(false); + + auto expr = VectorizedFnCall::create_shared(node); + expr->add_child(left); + expr->add_child(right); + return expr; +} + VExprSPtr array_element_at(const VExprSPtr& parent, DataTypePtr child_type, int64_t ordinal) { auto expr = std::make_shared("element_at", std::move(child_type)); expr->add_child(parent); @@ -2993,6 +3061,251 @@ TEST(ColumnMapperScanRequestTest, NestedElementAtConjunctUsesFileChildTypeForRen EXPECT_EQ(localized_parent_type->get_element_name(1), "bb"); } +// Scenario: Iceberg promotes a nested struct leaf from INT to BIGINT while an old file still +// stores INT. Because 15 is exactly representable as INT and every INT survives promotion to +// BIGINT, localize `s.b::BIGINT > 15::BIGINT` as `file_s.b::INT > 15::INT`. Rewriting one literal +// avoids casting every file value while preserving the table predicate exactly. +TEST_F(ColumnMapperCastTest, NestedElementAtConjunctRewritesExactLiteralToFileType) { + const auto file_int_type = i32(); + const auto table_bigint_type = i64(); + + auto table_b = field_id_col("b", 11, table_bigint_type); + auto table_struct = struct_col("s", 10, {table_b}); + auto file_b = field_id_col("b", 11, file_int_type, 0); + auto file_struct = struct_col("s", 10, {file_b}, 5); + + auto table_leaf = executable_struct_element( + table_slot(0, 0, table_struct.type, table_struct.name), table_bigint_type, "b"); + auto filter_expr = executable_binary_predicate( + TExprOpcode::GT, table_leaf, + literal(table_bigint_type, Field::create_field(15))); + TableFilter filter {.conjunct = VExprContext::create_shared(filter_expr), + .global_indices = {GlobalIndex(0)}}; + + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_FIELD_ID}); + ASSERT_TRUE(mapper.create_mapping({table_struct}, {}, {file_struct}).ok()); + + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, {table_struct}, &request, &state).ok()); + ASSERT_EQ(request.conjuncts.size(), 1); + const auto& localized_root = request.conjuncts[0]->root(); + ASSERT_EQ(localized_root->get_num_children(), 2); + const auto& localized_leaf = localized_root->children()[0]; + EXPECT_EQ(localized_leaf->expr_name(), "element_at"); + EXPECT_TRUE(localized_leaf->data_type()->equals(*file_int_type)); + const auto& localized_literal = localized_root->children()[1]; + EXPECT_TRUE(localized_literal->is_literal()); + EXPECT_TRUE(localized_literal->data_type()->equals(*file_int_type)); + + auto values = ColumnInt32::create(); + values->insert_value(10); + values->insert_value(20); + MutableColumns children; + children.push_back(std::move(values)); + Block block; + block.insert({ColumnStruct::create(std::move(children)), mapper.mappings()[0].file_type, "s"}); + + auto* conjunct = request.conjuncts[0].get(); + auto status = conjunct->prepare(&state, RowDescriptor()); + ASSERT_TRUE(status.ok()) << status; + status = conjunct->open(&state); + ASSERT_TRUE(status.ok()) << status; + IColumn::Filter result(block.rows(), 1); + bool can_filter_all = false; + status = conjunct->execute_filter(&block, result.data(), block.rows(), false, &can_filter_all); + ASSERT_TRUE(status.ok()) << status; + EXPECT_FALSE(can_filter_all); + EXPECT_EQ(result, IColumn::Filter({0, 1})); + conjunct->close(); +} + +// Scenario: an old file allows NULL for a nested leaf that the current table declares required. +// Although every non-NULL INT value and the literal 15 can be promoted to BIGINT exactly, the +// predicate must stay above TableReader. If `file_s.b > 15` ran first for rows [NULL, 20], it would +// discard NULL and prevent table-schema materialization from reporting the nullable-to-required +// contract violation. +TEST_F(ColumnMapperCastTest, + NestedElementAtConjunctStaysTableLevelForNullableFileLeafMappedToRequiredTableLeaf) { + const auto file_nullable_int_type = make_nullable(i32()); + const auto table_bigint_type = i64(); + + auto table_b = field_id_col("b", 11, table_bigint_type); + auto table_struct = struct_col("s", 10, {table_b}); + auto file_b = field_id_col("b", 11, file_nullable_int_type, 0); + auto file_struct = struct_col("s", 10, {file_b}, 5); + + auto table_leaf = executable_struct_element( + table_slot(0, 0, table_struct.type, table_struct.name), table_bigint_type, "b"); + auto filter_expr = executable_binary_predicate( + TExprOpcode::GT, table_leaf, + literal(table_bigint_type, Field::create_field(15))); + TableFilter filter {.conjunct = VExprContext::create_shared(filter_expr), + .global_indices = {GlobalIndex(0)}}; + + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_FIELD_ID}); + ASSERT_TRUE(mapper.create_mapping({table_struct}, {}, {file_struct}).ok()); + + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, {table_struct}, &request, &state).ok()); + ASSERT_EQ(request.predicate_columns.size() + request.non_predicate_columns.size(), 1); + const auto& scan_column = request.predicate_columns.empty() ? request.non_predicate_columns[0] + : request.predicate_columns[0]; + EXPECT_EQ(scan_column.column_id(), LocalColumnId(5)); + EXPECT_TRUE(request.conjuncts.empty()); +} + +// Scenario: a narrowing file-to-table cast can produce NULL or an error for values that do not fit +// the table leaf. Evaluating that cast below TableReader can filter those rows before +// _align_column_nullability() validates the required table child. Keep the predicate at table level +// so schema materialization observes every source row first. +TEST_F(ColumnMapperCastTest, NestedElementAtConjunctStaysTableLevelForNonLosslessFileToTableCast) { + const auto file_bigint_type = i64(); + const auto table_int_type = i32(); + + auto table_a = field_id_col("a", 11, table_int_type); + auto table_struct = struct_col("s", 10, {table_a}); + auto file_a = field_id_col("a", 11, file_bigint_type, 0); + auto file_struct = struct_col("s", 10, {file_a}, 5); + + auto table_leaf = executable_struct_element( + table_slot(0, 0, table_struct.type, table_struct.name), table_int_type, "a"); + auto filter_expr = executable_binary_predicate( + TExprOpcode::EQ, table_leaf, literal(table_int_type, Field::create_field(1))); + TableFilter filter {.conjunct = VExprContext::create_shared(filter_expr), + .global_indices = {GlobalIndex(0)}}; + + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_FIELD_ID}); + ASSERT_TRUE(mapper.create_mapping({table_struct}, {}, {file_struct}).ok()); + + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, {table_struct}, &request, &state).ok()); + ASSERT_EQ(request.predicate_columns.size() + request.non_predicate_columns.size(), 1); + const auto& scan_column = request.predicate_columns.empty() ? request.non_predicate_columns[0] + : request.predicate_columns[0]; + EXPECT_EQ(scan_column.column_id(), LocalColumnId(5)); + EXPECT_TRUE(request.conjuncts.empty()); +} + +// Scenario: the table literal is outside the old file leaf's INT range. Rewriting +// BIGINT 2147483648 to INT would change the predicate, so keep the literal as BIGINT and cast the +// file leaf instead: `CAST(file_s.b::INT AS BIGINT) = 2147483648::BIGINT`. +TEST_F(ColumnMapperCastTest, NestedElementAtConjunctFallsBackForOutOfRangeLiteral) { + const auto file_int_type = i32(); + const auto table_bigint_type = i64(); + + auto table_b = field_id_col("b", 11, table_bigint_type); + auto table_struct = struct_col("s", 10, {table_b}); + auto file_b = field_id_col("b", 11, file_int_type, 0); + auto file_struct = struct_col("s", 10, {file_b}, 5); + auto table_leaf = executable_struct_element( + table_slot(0, 0, table_struct.type, table_struct.name), table_bigint_type, "b"); + auto filter_expr = executable_binary_predicate( + TExprOpcode::EQ, table_leaf, + literal(table_bigint_type, Field::create_field(2147483648LL))); + TableFilter filter {.conjunct = VExprContext::create_shared(filter_expr), + .global_indices = {GlobalIndex(0)}}; + + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_FIELD_ID}); + ASSERT_TRUE(mapper.create_mapping({table_struct}, {}, {file_struct}).ok()); + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, {table_struct}, &request, &state).ok()); + + ASSERT_EQ(request.conjuncts.size(), 1); + const auto& localized_root = request.conjuncts[0]->root(); + ASSERT_EQ(localized_root->get_num_children(), 2); + const auto& localized_cast = localized_root->children()[0]; + ASSERT_NE(dynamic_cast(localized_cast.get()), nullptr); + EXPECT_TRUE(localized_cast->data_type()->equals(*table_bigint_type)); + ASSERT_EQ(localized_cast->get_num_children(), 1); + EXPECT_EQ(localized_cast->children()[0]->expr_name(), "element_at"); + EXPECT_TRUE(localized_cast->children()[0]->data_type()->equals(*file_int_type)); + EXPECT_TRUE(localized_root->children()[1]->data_type()->equals(*table_bigint_type)); +} + +// Scenario: the struct leaf is on the right side of the comparison. Literal localization must not +// depend on operand order: `15::BIGINT > s.b::BIGINT` becomes `15::INT > file_s.b::INT`. +TEST_F(ColumnMapperCastTest, NestedElementAtConjunctRewritesReverseComparisonLiteral) { + const auto file_int_type = i32(); + const auto table_bigint_type = i64(); + + auto table_b = field_id_col("b", 11, table_bigint_type); + auto table_struct = struct_col("s", 10, {table_b}); + auto file_b = field_id_col("b", 11, file_int_type, 0); + auto file_struct = struct_col("s", 10, {file_b}, 5); + auto table_leaf = executable_struct_element( + table_slot(0, 0, table_struct.type, table_struct.name), table_bigint_type, "b"); + auto filter_expr = executable_binary_predicate( + TExprOpcode::GT, literal(table_bigint_type, Field::create_field(15)), + table_leaf); + TableFilter filter {.conjunct = VExprContext::create_shared(filter_expr), + .global_indices = {GlobalIndex(0)}}; + + TableColumnMapper mapper({.mode = TableColumnMappingMode::BY_FIELD_ID}); + ASSERT_TRUE(mapper.create_mapping({table_struct}, {}, {file_struct}).ok()); + FileScanRequest request; + ASSERT_TRUE(mapper.create_scan_request({filter}, {table_struct}, &request, &state).ok()); + + ASSERT_EQ(request.conjuncts.size(), 1); + const auto& localized_root = request.conjuncts[0]->root(); + EXPECT_TRUE(localized_root->children()[0]->data_type()->equals(*file_int_type)); + EXPECT_TRUE(localized_root->children()[0]->is_literal()); + EXPECT_EQ(localized_root->children()[1]->expr_name(), "element_at"); + EXPECT_TRUE(localized_root->children()[1]->data_type()->equals(*file_int_type)); +} + +// Scenario: IN uses one probe type for every candidate. All exact literals may move to the INT +// file domain, but one out-of-range literal makes the complete IN predicate fall back to BIGINT. +TEST_F(ColumnMapperCastTest, NestedElementAtInPredicateUsesAllOrNothingLiteralRewrite) { + const auto file_int_type = i32(); + const auto table_bigint_type = i64(); + auto table_b = field_id_col("b", 11, table_bigint_type); + auto table_struct = struct_col("s", 10, {table_b}); + auto file_b = field_id_col("b", 11, file_int_type, 0); + auto file_struct = struct_col("s", 10, {file_b}, 5); + + const auto build_filter = [&](int64_t second_value) { + auto table_leaf = executable_struct_element( + table_slot(0, 0, table_struct.type, table_struct.name), table_bigint_type, "b"); + auto predicate = create_in_predicate(); + predicate->add_child(table_leaf); + predicate->add_child(literal(table_bigint_type, Field::create_field(10))); + predicate->add_child( + literal(table_bigint_type, Field::create_field(second_value))); + return TableFilter {.conjunct = VExprContext::create_shared(predicate), + .global_indices = {GlobalIndex(0)}}; + }; + + TableColumnMapper exact_mapper({.mode = TableColumnMappingMode::BY_FIELD_ID}); + ASSERT_TRUE(exact_mapper.create_mapping({table_struct}, {}, {file_struct}).ok()); + FileScanRequest exact_request; + ASSERT_TRUE( + exact_mapper + .create_scan_request({build_filter(20)}, {table_struct}, &exact_request, &state) + .ok()); + ASSERT_EQ(exact_request.conjuncts.size(), 1); + const auto& exact_root = exact_request.conjuncts[0]->root(); + EXPECT_EQ(exact_root->children()[0]->expr_name(), "element_at"); + for (const auto& child : exact_root->children()) { + EXPECT_TRUE(child->data_type()->equals(*file_int_type)); + } + + TableColumnMapper fallback_mapper({.mode = TableColumnMappingMode::BY_FIELD_ID}); + ASSERT_TRUE(fallback_mapper.create_mapping({table_struct}, {}, {file_struct}).ok()); + FileScanRequest fallback_request; + ASSERT_TRUE(fallback_mapper + .create_scan_request({build_filter(2147483648LL)}, {table_struct}, + &fallback_request, &state) + .ok()); + ASSERT_EQ(fallback_request.conjuncts.size(), 1); + const auto& fallback_root = fallback_request.conjuncts[0]->root(); + const auto& fallback_cast = fallback_root->children()[0]; + ASSERT_NE(dynamic_cast(fallback_cast.get()), nullptr); + EXPECT_TRUE(fallback_cast->data_type()->equals(*table_bigint_type)); + EXPECT_TRUE(fallback_cast->children()[0]->data_type()->equals(*file_int_type)); + EXPECT_TRUE(fallback_root->children()[1]->data_type()->equals(*table_bigint_type)); + EXPECT_TRUE(fallback_root->children()[2]->data_type()->equals(*table_bigint_type)); +} + // Scenario: output projection reads one struct child while the row filter reads a different nested // struct child. File-local conjunct rewrite must use the merged scan projection type. In the SQL // shape below, `SELECT element_at(s, 'c') WHERE element_at(element_at(s, 'b'), 'cc') LIKE ...` diff --git a/be/test/format_v2/native/native_reader_test.cpp b/be/test/format_v2/native/native_reader_test.cpp index aaa7aa90e0681e..2745ecf852c38b 100644 --- a/be/test/format_v2/native/native_reader_test.cpp +++ b/be/test/format_v2/native/native_reader_test.cpp @@ -295,7 +295,7 @@ TEST(NativeV2ReaderTest, RejectsInvalidHeaderAndEmptyFile) { static_cast(io::global_local_filesystem()->delete_file(empty_path)); } -TEST(NativeV2ReaderTest, RejectsUnsupportedVersionAndHeaderOnlyFile) { +TEST(NativeV2ReaderTest, RejectsUnsupportedVersionAndReportsHeaderOnlyFileAsEmpty) { std::filesystem::create_directories("./log"); RuntimeState state; RuntimeProfile profile("native_v2_reader_header_boundary_test"); @@ -322,7 +322,8 @@ TEST(NativeV2ReaderTest, RejectsUnsupportedVersionAndHeaderOnlyFile) { auto header_only_reader = create_reader(header_only_path, &state, &profile); ASSERT_TRUE(header_only_reader->init(&state).ok()); std::vector schema; - EXPECT_FALSE(header_only_reader->get_schema(&schema).ok()); + const auto header_only_status = header_only_reader->get_schema(&schema); + EXPECT_TRUE(header_only_status.is()) << header_only_status; static_cast(io::global_local_filesystem()->delete_file(header_only_path)); } @@ -350,6 +351,34 @@ TEST(NativeV2ReaderTest, RejectsTruncatedBlockDuringSchemaProbe) { static_cast(io::global_local_filesystem()->delete_file(path)); } +TEST(NativeV2ReaderTest, RejectsPartialBlockLengthAsCorruption) { + std::filesystem::create_directories("./log"); + RuntimeState state; + RuntimeProfile profile("native_v2_reader_partial_length_test"); + + std::string header; + header.append(DORIS_NATIVE_MAGIC, sizeof(DORIS_NATIVE_MAGIC)); + uint8_t version_buffer[sizeof(uint32_t)]; + encode_fixed32_le(version_buffer, DORIS_NATIVE_FORMAT_VERSION); + header.append(reinterpret_cast(version_buffer), sizeof(version_buffer)); + + for (size_t prefix_bytes = 1; prefix_bytes < sizeof(uint64_t); ++prefix_bytes) { + const auto path = "./log/native_v2_partial_length_" + std::to_string(prefix_bytes) + "_" + + UniqueId::gen_uid().to_string() + ".native"; + auto content = header; + content.append(prefix_bytes, '\0'); + ASSERT_TRUE(write_file(path, content).ok()); + + auto reader = create_reader(path, &state, &profile); + ASSERT_TRUE(reader->init(&state).ok()); + std::vector schema; + const auto status = reader->get_schema(&schema); + EXPECT_TRUE(status.is()) << status; + EXPECT_NE(status.to_string().find("truncated native block length"), std::string::npos); + static_cast(io::global_local_filesystem()->delete_file(path)); + } +} + TEST(NativeV2ReaderTest, RejectsZeroLengthBlockAndInvalidPBlock) { std::filesystem::create_directories("./log"); RuntimeState state; @@ -374,7 +403,9 @@ TEST(NativeV2ReaderTest, RejectsZeroLengthBlockAndInvalidPBlock) { auto zero_len_reader = create_reader(zero_len_path, &state, &profile); ASSERT_TRUE(zero_len_reader->init(&state).ok()); std::vector schema; - EXPECT_FALSE(zero_len_reader->get_schema(&schema).ok()); + const auto zero_len_status = zero_len_reader->get_schema(&schema); + EXPECT_TRUE(zero_len_status.is()) << zero_len_status; + EXPECT_NE(zero_len_status.to_string().find("zero-length native block"), std::string::npos); static_cast(io::global_local_filesystem()->delete_file(zero_len_path)); const auto invalid_pblock_path = diff --git a/be/test/format_v2/parquet/parquet_page_cache_range_test.cpp b/be/test/format_v2/parquet/parquet_page_cache_range_test.cpp index fac89b31c422d8..940f94a373a013 100644 --- a/be/test/format_v2/parquet/parquet_page_cache_range_test.cpp +++ b/be/test/format_v2/parquet/parquet_page_cache_range_test.cpp @@ -115,6 +115,45 @@ TEST(ParquetPageCacheRangeTest, InvalidRequestMisses) { EXPECT_TRUE(detail::plan_page_cache_range_read(100, -1, cached_ranges).empty()); } +TEST(ParquetPageCacheRangeTest, PerFileRangeIndexDeduplicatesAndEvictsAtCapacity) { + detail::ParquetPageCacheRangeIndex index(3); + index.insert({200, 20}); + index.insert({100, 30}); + index.insert({100, 10}); + index.insert({100, 30}); + + EXPECT_EQ(index.size(), 3); + index.insert({300, 40}); + const auto ranges = index.ranges(); + ASSERT_EQ(ranges.size(), 3); + EXPECT_EQ(ranges[0].offset, 100); + EXPECT_EQ(ranges[0].size, 30); + EXPECT_EQ(ranges[1].offset, 200); + EXPECT_EQ(ranges[1].size, 20); + EXPECT_EQ(ranges[2].offset, 300); + + index.erase({100, 30}); + EXPECT_EQ(index.size(), 2); +} + +TEST(ParquetPageCacheRangeTest, DirectorySharesBoundedIndexAcrossReaderLifetimes) { + detail::ParquetPageCacheRangeDirectory directory(2); + auto first_reader_index = directory.get_or_create("file-a"); + first_reader_index->insert({100, 100}); + first_reader_index.reset(); + + // The directory owns the per-file index, so reader B still discovers reader A's wider cache + // entry and can plan a subset hit after A closes. + auto second_reader_index = directory.get_or_create("file-a"); + const auto plan = detail::plan_page_cache_range_read(120, 30, second_reader_index->ranges()); + ASSERT_EQ(plan.size(), 1); + expect_plan_entry(plan[0], {100, 100}, 20, 0, 30); + + directory.get_or_create("file-b"); + directory.get_or_create("file-c"); + EXPECT_EQ(directory.size(), 2); +} + TEST(ParquetPageCacheRangeTest, ValidPrefetchRangesSkipInvalidAndOverflowRanges) { const std::vector ranges = { {100, 50}, diff --git a/be/test/format_v2/parquet/parquet_reader_control_test.cpp b/be/test/format_v2/parquet/parquet_reader_control_test.cpp index 2fd85cefd49ac9..36b7cebdaa9cb9 100644 --- a/be/test/format_v2/parquet/parquet_reader_control_test.cpp +++ b/be/test/format_v2/parquet/parquet_reader_control_test.cpp @@ -1206,6 +1206,20 @@ TEST(ParquetColumnReaderFactoryTest, RejectsInvalidLeafIdBeforeCreatingRecordRea EXPECT_NE(status.to_string().find("Invalid parquet leaf column id"), std::string::npos); } +TEST(ParquetColumnReaderFactoryTest, RejectsProjectedUnsupportedLogicalType) { + ParquetColumnSchema schema = int64_schema("unsupported_time"); + schema.kind = ParquetColumnSchemaKind::PRIMITIVE; + schema.type_descriptor.unsupported_reason = + "Parquet TIME with isAdjustedToUTC=true is not supported"; + + ParquetColumnReaderFactory factory(nullptr, 1); + std::unique_ptr reader; + const auto status = factory.create(schema, &reader); + EXPECT_FALSE(status.ok()); + EXPECT_NE(status.to_string().find(schema.type_descriptor.unsupported_reason), + std::string::npos); +} + TEST(ParquetColumnReaderFactoryTest, RejectsStructInvalidAndEmptyProjection) { auto schema = struct_schema_for_projection(); ParquetColumnReaderFactory factory(nullptr, 0); diff --git a/be/test/format_v2/parquet/parquet_scan_test.cpp b/be/test/format_v2/parquet/parquet_scan_test.cpp index 55387d2c1ed380..36cc99ebf106c0 100644 --- a/be/test/format_v2/parquet/parquet_scan_test.cpp +++ b/be/test/format_v2/parquet/parquet_scan_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -45,6 +46,8 @@ #include "core/field.h" #include "exprs/vexpr.h" #include "exprs/vexpr_context.h" +#include "exprs/vslot_ref.h" +#include "format_v2/expr/delete_predicate.h" #include "format_v2/file_reader.h" #include "format_v2/parquet/parquet_column_schema.h" #include "format_v2/parquet/parquet_reader.h" @@ -304,6 +307,32 @@ void write_table(const std::string& file_path, const std::shared_ptr out = *file_result; + + // Arrow intentionally writes time32 as local time (isAdjustedToUTC=false), so use Parquet's + // low-level writer to produce the unsupported required logical type needed by this aggregate + // regression. Required is important: Nereids may push COUNT(col) because NULL filtering cannot + // change its value, which is precisely the path where an empty aggregate request lost `col`. + const auto adjusted_time = ::parquet::schema::PrimitiveNode::Make( + "unsupported_time", ::parquet::Repetition::REQUIRED, + ::parquet::LogicalType::Time(true, ::parquet::LogicalType::TimeUnit::MILLIS), + ::parquet::Type::INT32); + const auto schema_node = ::parquet::schema::GroupNode::Make( + "schema", ::parquet::Repetition::REQUIRED, {adjusted_time}); + const auto schema = std::static_pointer_cast<::parquet::schema::GroupNode>(schema_node); + auto writer = ::parquet::ParquetFileWriter::Open(out, schema); + auto* row_group = writer->AppendRowGroup(); + auto* time_writer = static_cast<::parquet::Int32Writer*>(row_group->NextColumn()); + const int32_t values[] = {1000, 2000}; + EXPECT_EQ(time_writer->WriteBatch(2, nullptr, nullptr, values), 2); + time_writer->Close(); + row_group->Close(); + writer->Close(); +} + void write_int_pair_parquet_file(const std::string& file_path, int64_t row_group_size = 2, bool enable_statistics = true) { auto schema = arrow::schema({ @@ -737,6 +766,15 @@ TEST_F(ParquetScanTest, AggregateCountAndMinMaxUseAllSelectedRowGroups) { EXPECT_EQ(count_result.count, 6); EXPECT_TRUE(count_result.columns.empty()); + format::FileAggregateResult required_count_result; + format::FileAggregateRequest required_count_request; + required_count_request.agg_type = TPushAggOp::COUNT; + required_count_request.columns.push_back({.projection = field_projection(0)}); + ASSERT_TRUE(reader->get_aggregate_result(required_count_request, &required_count_result).ok()); + // The required scalar projection is retained for logical-type validation, but after that + // validation COUNT(id) can reuse the same selected-row-group footer count as COUNT(*). + EXPECT_EQ(required_count_result.count, 6); + format::FileAggregateResult minmax_result; format::FileAggregateRequest minmax_request; minmax_request.agg_type = TPushAggOp::MINMAX; @@ -753,6 +791,96 @@ TEST_F(ParquetScanTest, AggregateCountAndMinMaxUseAllSelectedRowGroups) { EXPECT_EQ(minmax_result.columns[1].max_value.get(), 60); } +TEST_F(ParquetScanTest, AggregateCountRejectsRequiredUnsupportedScalarProjection) { + write_required_adjusted_time_parquet_file(_file_path); + auto reader = create_reader(); + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + ASSERT_TRUE(reader->init(&state).ok()); + open_all_row_groups(reader.get()); + + format::FileAggregateRequest request; + request.agg_type = TPushAggOp::COUNT; + request.columns.push_back({.projection = field_projection(0)}); + format::FileAggregateResult result; + const auto status = reader->get_aggregate_result(request, &result); + EXPECT_TRUE(status.is()) << status; + EXPECT_NE(status.to_string().find("Parquet TIME with isAdjustedToUTC=true is not supported"), + std::string::npos); +} + +TEST_F(ParquetScanTest, CountStarIgnoresUnsupportedPlannerPlaceholder) { + write_required_adjusted_time_parquet_file(_file_path); + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + + // A normal projection still requests the TIME_MILLIS value and must fail at open, before + // row-group pruning or physical INT32 statistics can hide the unsupported logical type. + auto projected_reader = create_reader(); + ASSERT_TRUE(projected_reader->init(&state).ok()); + auto projected_request = std::make_shared(); + format::FileScanRequestBuilder projected_builder(projected_request.get()); + ASSERT_TRUE(projected_builder.add_non_predicate_column(format::LocalColumnId(0)).ok()); + const auto projected_status = projected_reader->open(projected_request); + EXPECT_TRUE(projected_status.is()) << projected_status; + + // Metadata COUNT(*) carries the same retained scan slot, but its aggregate request has no + // semantic column. The placeholder must not make open fail before footer row counts are used. + auto metadata_count_reader = create_reader(); + ASSERT_TRUE(metadata_count_reader->init(&state).ok()); + auto metadata_count_request = std::make_shared(); + format::FileScanRequestBuilder metadata_count_builder(metadata_count_request.get()); + ASSERT_TRUE(metadata_count_builder.add_non_predicate_column(format::LocalColumnId(0)).ok()); + metadata_count_request->count_star_placeholder_columns.push_back(format::LocalColumnId(0)); + ASSERT_TRUE(metadata_count_reader->open(metadata_count_request).ok()); + + format::FileAggregateRequest aggregate_request; + aggregate_request.agg_type = TPushAggOp::COUNT; + format::FileAggregateResult aggregate_result; + ASSERT_TRUE( + metadata_count_reader->get_aggregate_result(aggregate_request, &aggregate_result).ok()); + EXPECT_EQ(aggregate_result.count, 2); + + // The marker is independent of aggregate eligibility. Simulate a position delete, + // which disables metadata COUNT and requires the reader to produce surviving rows. Parquet + // reads only the virtual row position, filters row 1, and fills a default placeholder for row 0 + // without validating or decoding either unsupported TIME_MILLIS value. + auto count_star_reader = create_reader(); + ASSERT_TRUE(count_star_reader->init(&state).ok()); + auto count_star_request = std::make_shared(); + format::FileScanRequestBuilder count_star_builder(count_star_request.get()); + ASSERT_TRUE(count_star_builder.add_non_predicate_column(format::LocalColumnId(0)).ok()); + ASSERT_TRUE(count_star_builder + .add_predicate_column(format::LocalColumnId(format::ROW_POSITION_COLUMN_ID)) + .ok()); + count_star_request->count_star_placeholder_columns.push_back(format::LocalColumnId(0)); + + const std::vector deleted_rows {1}; + auto delete_predicate = std::make_shared(deleted_rows); + const auto row_position = count_star_request->local_positions.at( + format::LocalColumnId(format::ROW_POSITION_COLUMN_ID)); + delete_predicate->add_child(VSlotRef::create_shared( + cast_set(row_position.value()), cast_set(row_position.value()), -1, + std::make_shared(), format::ROW_POSITION_COLUMN_NAME)); + auto delete_context = VExprContext::create_shared(std::move(delete_predicate)); + ASSERT_TRUE(delete_context->prepare(&state, RowDescriptor()).ok()); + ASSERT_TRUE(delete_context->open(&state).ok()); + count_star_request->delete_conjuncts.push_back(std::move(delete_context)); + ASSERT_TRUE(count_star_reader->open(count_star_request).ok()); + + std::vector file_schema; + ASSERT_TRUE(count_star_reader->get_schema(&file_schema).ok()); + file_schema.push_back(format::row_position_column_definition()); + auto block = build_file_block(file_schema); + size_t rows = 0; + bool eof = false; + ASSERT_TRUE(count_star_reader->get_block(&block, &rows, &eof).ok()); + EXPECT_EQ(rows, 1); + EXPECT_EQ(block.get_by_position(0).column->size(), 1); + const auto& positions = + assert_cast(*block.get_by_position(row_position.value()).column); + ASSERT_EQ(positions.size(), 1); + EXPECT_EQ(positions.get_element(0), 0); +} + TEST_F(ParquetScanTest, AggregateMinMaxRejectsInexactBinaryStatistics) { write_binary_minmax_parquet_file(_file_path); auto reader = create_reader(); diff --git a/be/test/format_v2/parquet/parquet_schema_test.cpp b/be/test/format_v2/parquet/parquet_schema_test.cpp index e620ed718efbf2..9f78735bfb560a 100644 --- a/be/test/format_v2/parquet/parquet_schema_test.cpp +++ b/be/test/format_v2/parquet/parquet_schema_test.cpp @@ -414,7 +414,7 @@ TEST(ParquetSchemaTest, BuildEntryValidatesNullPointerAndEmptyRoot) { EXPECT_TRUE(fields.empty()); } -TEST(ParquetSchemaTest, RejectInvalidListMapAndUnsupportedTime) { +TEST(ParquetSchemaTest, RejectInvalidListMapAndPreserveUnsupportedTime) { const auto bad_list = ::parquet::schema::GroupNode::Make( "bad_list", ::parquet::Repetition::OPTIONAL, {::parquet::schema::PrimitiveNode::Make("item", ::parquet::Repetition::OPTIONAL, @@ -432,9 +432,11 @@ TEST(ParquetSchemaTest, RejectInvalidListMapAndUnsupportedTime) { const auto converted_time = ::parquet::schema::PrimitiveNode::Make( "time_ms", ::parquet::Repetition::REQUIRED, ::parquet::Type::INT32, ::parquet::ConvertedType::TIME_MILLIS); - const auto status = build_status({converted_time}); - EXPECT_FALSE(status.ok()); - EXPECT_NE(status.to_string().find("Parquet TIME with isAdjustedToUTC=true is not supported"), + const auto fields = build_fields({converted_time}); + ASSERT_EQ(fields.size(), 1); + EXPECT_EQ(remove_nullable(fields[0]->type)->get_primitive_type(), TYPE_INT); + EXPECT_NE(fields[0]->type_descriptor.unsupported_reason.find( + "Parquet TIME with isAdjustedToUTC=true is not supported"), std::string::npos); } @@ -513,15 +515,21 @@ TEST(ParquetSchemaTest, RejectAdditionalInvalidListAndMapLayouts) { EXPECT_FALSE(build_status({repeated_map}).ok()); } -TEST(ParquetSchemaTest, LogicalUtcTimeIsRejected) { +TEST(ParquetSchemaTest, LogicalUtcTimeIsPreservedForProjection) { const auto adjusted_time = ::parquet::schema::PrimitiveNode::Make( "time_ms", ::parquet::Repetition::REQUIRED, ::parquet::LogicalType::Time(true, ::parquet::LogicalType::TimeUnit::MILLIS), ::parquet::Type::INT32); - const auto status = build_status({adjusted_time}); - EXPECT_FALSE(status.ok()); - EXPECT_NE(status.to_string().find("Parquet TIME with isAdjustedToUTC=true is not supported"), - std::string::npos); + const auto supported_value = ::parquet::schema::PrimitiveNode::Make( + "value", ::parquet::Repetition::REQUIRED, ::parquet::Type::INT64); + const auto row = ::parquet::schema::GroupNode::Make("row", ::parquet::Repetition::OPTIONAL, + {adjusted_time, supported_value}); + const auto fields = build_fields({row}); + ASSERT_EQ(fields.size(), 1); + ASSERT_EQ(fields[0]->children.size(), 2); + EXPECT_EQ(remove_nullable(fields[0]->children[0]->type)->get_primitive_type(), TYPE_INT); + EXPECT_FALSE(fields[0]->children[0]->type_descriptor.unsupported_reason.empty()); + EXPECT_EQ(remove_nullable(fields[0]->children[1]->type)->get_primitive_type(), TYPE_BIGINT); } } // namespace doris::format::parquet diff --git a/be/test/format_v2/table/hudi_reader_test.cpp b/be/test/format_v2/table/hudi_reader_test.cpp index 28c396371e4559..2dd001469d8a66 100644 --- a/be/test/format_v2/table/hudi_reader_test.cpp +++ b/be/test/format_v2/table/hudi_reader_test.cpp @@ -17,14 +17,20 @@ #include "format_v2/table/hudi_reader.h" +#include +#include #include +#include +#include +#include #include #include #include #include #include +#include "core/data_type/data_type_nullable.h" #include "core/data_type/data_type_number.h" #include "core/data_type/data_type_string.h" #include "core/data_type/data_type_struct.h" @@ -32,6 +38,9 @@ #include "format_v2/column_data.h" #include "gen_cpp/ExternalTableSchema_types.h" #include "gen_cpp/PlanNodes_types.h" +#include "io/io_common.h" +#include "runtime/runtime_profile.h" +#include "runtime/runtime_state.h" namespace doris::format { namespace { @@ -69,6 +78,39 @@ ColumnDefinition make_file_column(int32_t id, const std::string& name, const Dat return field; } +ColumnDefinition make_table_column(int32_t id, const std::string& name, const DataTypePtr& type) { + ColumnDefinition column; + column.identifier = Field::create_field(id); + column.name = name; + column.type = make_nullable(type); + return column; +} + +Block build_table_block(const std::vector& columns) { + Block block; + for (const auto& column : columns) { + block.insert({column.type->create_column(), column.type, column.name}); + } + return block; +} + +void write_int_parquet_file(const std::string& file_path, const std::vector& values) { + arrow::Int32Builder value_builder; + for (const auto value : values) { + ASSERT_TRUE(value_builder.Append(value).ok()); + } + std::shared_ptr value_array; + ASSERT_TRUE(value_builder.Finish(&value_array).ok()); + const auto table = arrow::Table::Make( + arrow::schema({arrow::field("id", arrow::int32(), false)}), {value_array}); + + auto file_result = arrow::io::FileOutputStream::Open(file_path); + ASSERT_TRUE(file_result.ok()) << file_result.status(); + std::shared_ptr output = *file_result; + PARQUET_THROW_NOT_OK(::parquet::arrow::WriteTable(*table, arrow::default_memory_pool(), output, + static_cast(values.size()))); +} + TTableFormatFileDesc hudi_table_format_desc(std::optional schema_id) { TTableFormatFileDesc table_format_params; table_format_params.__set_table_format_type("hudi"); @@ -201,5 +243,62 @@ TEST(HudiHybridReaderTest, AdaptiveBatchSizeReachesBothChildReaders) { EXPECT_EQ(child_batch_sizes.second, 123); } +TEST(HudiHybridReaderTest, NativeCountStarReportsMetadataRowsThroughHybridReader) { + const auto test_dir = + std::filesystem::temp_directory_path() / "doris_hudi_hybrid_count_star_test"; + std::filesystem::remove_all(test_dir); + std::filesystem::create_directories(test_dir); + const auto file_path = (test_dir / "base-file.parquet").string(); + write_int_parquet_file(file_path, {1, 2, 3}); + + const std::vector projected_columns { + make_table_column(0, "id", std::make_shared()), + }; + RuntimeProfile profile("test_profile"); + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + TFileScanRangeParams scan_params; + scan_params.__set_file_type(TFileType::FILE_LOCAL); + scan_params.__set_format_type(TFileFormatType::FORMAT_PARQUET); + io::FileReaderStats file_reader_stats; + io::FileCacheStatistics file_cache_stats; + auto io_ctx = std::make_shared(); + io_ctx->file_reader_stats = &file_reader_stats; + io_ctx->file_cache_stats = &file_cache_stats; + ShardedKVCache cache(1); + + hudi::HudiHybridReader reader; + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = &scan_params, + .io_ctx = io_ctx, + .runtime_state = &state, + .scanner_profile = &profile, + .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, + }) + .ok()); + + SplitReadOptions split_options; + split_options.cache = &cache; + split_options.current_split_format = FileFormat::PARQUET; + split_options.current_range.__set_path(file_path); + split_options.current_range.__set_file_size( + static_cast(std::filesystem::file_size(file_path))); + split_options.current_range.__set_format_type(TFileFormatType::FORMAT_PARQUET); + split_options.current_range.__set_table_format_params(hudi_table_format_desc(std::nullopt)); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_EQ(block.rows(), 3); + EXPECT_TRUE(reader.current_split_uses_metadata_count()); + + ASSERT_TRUE(reader.close().ok()); + std::filesystem::remove_all(test_dir); +} + } // namespace } // namespace doris::format diff --git a/be/test/format_v2/table/iceberg_reader_test.cpp b/be/test/format_v2/table/iceberg_reader_test.cpp index 0831c98750ecbf..cbcf6d1725676d 100644 --- a/be/test/format_v2/table/iceberg_reader_test.cpp +++ b/be/test/format_v2/table/iceberg_reader_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -462,6 +463,39 @@ void write_single_int_parquet_file(const std::string& file_path, const std::stri builder.build())); } +void write_unsupported_time_first_int_parquet_file(const std::string& file_path, + const std::vector& ids) { + auto file_result = arrow::io::FileOutputStream::Open(file_path); + ASSERT_TRUE(file_result.ok()) << file_result.status(); + std::shared_ptr out = *file_result; + + // Arrow writes time32 as isAdjustedToUTC=false, which Doris supports. Use Parquet's low-level + // writer so the first physical column is the deliberately unsupported adjusted TIME_MILLIS + // shape from the review report. The query will project only the following supported `id`. + const auto unsupported_time = ::parquet::schema::PrimitiveNode::Make( + "unsupported_time", ::parquet::Repetition::REQUIRED, + ::parquet::LogicalType::Time(true, ::parquet::LogicalType::TimeUnit::MILLIS), + ::parquet::Type::INT32); + const auto id = ::parquet::schema::PrimitiveNode::Make("id", ::parquet::Repetition::REQUIRED, + ::parquet::Type::INT32); + const auto schema_node = ::parquet::schema::GroupNode::Make( + "schema", ::parquet::Repetition::REQUIRED, {unsupported_time, id}); + const auto schema = std::static_pointer_cast<::parquet::schema::GroupNode>(schema_node); + + auto writer = ::parquet::ParquetFileWriter::Open(out, schema); + auto* row_group = writer->AppendRowGroup(); + auto* time_writer = static_cast<::parquet::Int32Writer*>(row_group->NextColumn()); + std::vector times(ids.size(), 1000); + const auto row_count = cast_set(ids.size()); + EXPECT_EQ(time_writer->WriteBatch(row_count, nullptr, nullptr, times.data()), row_count); + time_writer->Close(); + auto* id_writer = static_cast<::parquet::Int32Writer*>(row_group->NextColumn()); + EXPECT_EQ(id_writer->WriteBatch(row_count, nullptr, nullptr, ids.data()), row_count); + id_writer->Close(); + row_group->Close(); + writer->Close(); +} + void write_two_int_parquet_file(const std::string& file_path, const std::string& first_name, const std::vector& first_values, std::optional first_field_id, @@ -1938,6 +1972,9 @@ TEST(IcebergV2ReaderTest, IcebergTableLevelCountUsesAssignedRowCountWithPosition .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + // An explicit empty argument list is the new FE marker for + // COUNT(*)/COUNT(1); nullopt intentionally exercises fallback. + .push_down_count_columns = std::vector {}, }) .ok()); @@ -2147,6 +2184,45 @@ TEST(IcebergV2ReaderTest, IcebergEqualityDeleteMatchesNullForMissingDataColumn) std::filesystem::remove_all(test_dir); } +TEST(IcebergV2ReaderTest, IcebergEqualityDeleteMissingKeyDoesNotReadUnsupportedUnprojectedCarrier) { + const auto test_dir = std::filesystem::temp_directory_path() / + "doris_iceberg_equality_delete_virtual_carrier_test"; + std::filesystem::remove_all(test_dir); + std::filesystem::create_directories(test_dir); + + const auto file_path = (test_dir / "split.parquet").string(); + const auto delete_file_path = (test_dir / "equality-delete.parquet").string(); + write_unsupported_time_first_int_parquet_file(file_path, {1, 2, 3}); + write_iceberg_equality_delete_parquet_file(delete_file_path, 1, 7, "added_column"); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + + RuntimeProfile profile("test_profile"); + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto scan_params = make_local_parquet_scan_params(); + io::FileReaderStats file_reader_stats; + io::FileCacheStatistics file_cache_stats; + auto io_ctx = make_io_context(&file_reader_stats, &file_cache_stats); + ShardedKVCache cache(1); + doris::format::iceberg::IcebergTableReader reader; + init_iceberg_reader(&reader, projected_columns, &scan_params, io_ctx, &state, &profile); + + auto split_options = build_split_options(file_path); + split_options.cache = &cache; + split_options.current_range.__set_table_format_params(make_iceberg_table_format_desc( + file_path, {make_iceberg_equality_delete_file(delete_file_path, {1})})); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + // The missing data key is NULL, so it does not match delete key 7. More importantly, the + // hidden row-count dependency must use virtual row position instead of the first physical + // TIME_MILLIS column, which is unsupported and was not requested by this `id` projection. + EXPECT_EQ(read_iceberg_ids(&reader, projected_columns), std::vector({1, 2, 3})); + + ASSERT_TRUE(reader.close().ok()); + std::filesystem::remove_all(test_dir); +} + TEST(IcebergV2ReaderTest, IcebergEqualityDeleteMatchesInitialDefaultForMissingDataColumn) { const auto test_dir = std::filesystem::temp_directory_path() / "doris_iceberg_equality_delete_missing_default_test"; diff --git a/be/test/format_v2/table/paimon_reader_test.cpp b/be/test/format_v2/table/paimon_reader_test.cpp index 6a3f20fac52726..ae1691a774f899 100644 --- a/be/test/format_v2/table/paimon_reader_test.cpp +++ b/be/test/format_v2/table/paimon_reader_test.cpp @@ -661,6 +661,59 @@ TEST(PaimonHybridReaderTest, AdaptiveBatchSizeReachesBothChildReaders) { EXPECT_EQ(child_batch_sizes.second, 321); } +TEST(PaimonHybridReaderTest, NativeCountColumnReportsMetadataRowsThroughHybridReader) { + const auto test_dir = + std::filesystem::temp_directory_path() / "doris_paimon_hybrid_count_column_test"; + std::filesystem::remove_all(test_dir); + std::filesystem::create_directories(test_dir); + const auto file_path = (test_dir / "data-file.parquet").string(); + write_int_pair_parquet_file(file_path, {1, 2, 3}, {10, 20, 30}, {"one", "two", "three"}); + + const std::vector projected_columns { + make_table_column(0, "id", std::make_shared()), + }; + RuntimeProfile profile("test_profile"); + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto scan_params = make_local_parquet_scan_params(); + io::FileReaderStats file_reader_stats; + io::FileCacheStatistics file_cache_stats; + auto io_ctx = make_io_context(&file_reader_stats, &file_cache_stats); + ShardedKVCache cache(1); + + paimon::PaimonHybridReader reader; + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = &scan_params, + .io_ctx = io_ctx, + .runtime_state = &state, + .scanner_profile = &profile, + .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = + std::vector {GlobalIndex(0)}, + }) + .ok()); + + SplitReadOptions split_options; + split_options.cache = &cache; + split_options.current_split_format = FileFormat::PARQUET; + split_options.current_range = make_paimon_native_range(TFileFormatType::FORMAT_PARQUET); + split_options.current_range.__set_path(file_path); + split_options.current_range.__set_file_size( + static_cast(std::filesystem::file_size(file_path))); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_EQ(block.rows(), 3); + EXPECT_TRUE(reader.current_split_uses_metadata_count()); + + ASSERT_TRUE(reader.close().ok()); + std::filesystem::remove_all(test_dir); +} + TEST(PaimonHybridReaderTest, DispatchesNativeThenJniSplitToMatchingReader) { RuntimeProfile profile("test_profile"); RuntimeState state {TQueryOptions(), TQueryGlobals()}; diff --git a/be/test/format_v2/table_reader_test.cpp b/be/test/format_v2/table_reader_test.cpp index 05ef805ae71267..d0096990357935 100644 --- a/be/test/format_v2/table_reader_test.cpp +++ b/be/test/format_v2/table_reader_test.cpp @@ -1012,6 +1012,7 @@ struct FakeFileReaderState { bool stop_during_read = false; bool not_found_during_init = false; std::shared_ptr last_request; + std::optional last_aggregate_request; std::shared_ptr condition_cache_ctx; std::shared_ptr io_ctx; }; @@ -1127,6 +1128,7 @@ class FakeFileReader final : public FileReader { if (request.agg_type != TPushAggOp::type::COUNT) { return Status::NotSupported("fake reader only supports COUNT aggregate pushdown"); } + _state->last_aggregate_request = request; if (_state->stop_during_aggregate) { DORIS_CHECK(_state->io_ctx != nullptr); _state->io_ctx->should_stop = true; @@ -1510,6 +1512,7 @@ TEST(TableReaderTest, RefreshedConjunctDisablesTableLevelCount) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); @@ -1552,6 +1555,7 @@ TEST(TableReaderTest, PendingRuntimeFilterDisablesTableLevelCount) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); @@ -1569,6 +1573,10 @@ TEST(TableReaderTest, PendingRuntimeFilterDisablesTableLevelCount) { ASSERT_TRUE(reader.get_block(&block, &eos).ok()); EXPECT_EQ(fake_state->open_count, 1); EXPECT_EQ(block.rows(), 2); + ASSERT_NE(fake_state->last_request, nullptr); + // Aggregate pushdown is disabled while a runtime filter is pending, but COUNT(*) semantics do + // not change. The retained output slot remains a value-less placeholder during row fallback. + EXPECT_TRUE(fake_state->last_request->is_count_star_placeholder(LocalColumnId(0))); ASSERT_TRUE(reader.close().ok()); } @@ -1652,6 +1660,7 @@ TEST(TableReaderTest, SlotlessConjunctDisablesAggregatePushdown) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); @@ -1714,11 +1723,12 @@ TEST(TableReaderTest, AbortSplitClearsReaderAfterIgnorableNotFound) { } TEST(TableReaderTest, PushDownCountRecordsReaderRowsBeforeClosingReader) { + const auto nullable_int_type = make_nullable(std::make_shared()); std::vector file_schema; - file_schema.push_back(make_file_column(0, "id", std::make_shared())); + file_schema.push_back(make_file_column(0, "id", nullable_int_type)); std::vector projected_columns; - projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + projected_columns.push_back(make_table_column(0, "id", nullable_int_type)); set_name_identifiers(&projected_columns); io::FileReaderStats file_reader_stats; @@ -1739,6 +1749,8 @@ TEST(TableReaderTest, PushDownCountRecordsReaderRowsBeforeClosingReader) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = + std::vector {GlobalIndex(0)}, }) .ok()); @@ -1753,6 +1765,230 @@ TEST(TableReaderTest, PushDownCountRecordsReaderRowsBeforeClosingReader) { EXPECT_EQ(block.rows(), 3); EXPECT_EQ(file_reader_stats.read_rows, 3); EXPECT_EQ(fake_state->close_count, 1); + EXPECT_TRUE(reader.current_split_uses_metadata_count()); + ASSERT_TRUE(fake_state->last_request != nullptr); + EXPECT_TRUE(fake_state->last_request->count_star_placeholder_columns.empty()); + ASSERT_TRUE(fake_state->last_aggregate_request.has_value()); + ASSERT_EQ(fake_state->last_aggregate_request->columns.size(), 1); + // A primitive COUNT(col) projection must reach the file reader just like a complex one. + EXPECT_EQ(fake_state->last_aggregate_request->columns[0].projection.local_id(), 0); +} + +TEST(TableReaderTest, PushDownCountStarIgnoresProjectedPlaceholderColumn) { + const auto nullable_int_type = make_nullable(std::make_shared()); + std::vector file_schema; + file_schema.push_back(make_file_column(0, "nullable_id", nullable_int_type)); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "nullable_id", nullable_int_type)); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + fake_state->aggregate_count = 3; + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE( + reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + // COUNT(*) deliberately has no explicit count columns. The + // nullable_id projection is only the planner's scan placeholder. + .push_down_count_columns = std::vector {}, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_FALSE(eos); + EXPECT_EQ(block.rows(), 3); + ASSERT_TRUE(fake_state->last_request != nullptr); + ASSERT_EQ(fake_state->last_request->count_star_placeholder_columns.size(), 1); + EXPECT_TRUE(fake_state->last_request->is_count_star_placeholder(LocalColumnId(0))); + ASSERT_TRUE(fake_state->last_aggregate_request.has_value()); + // Passing nullable_id here would implement COUNT(nullable_id) and reproduce the external ORC + // and Parquet failures where footer row counts were reduced by null values. + EXPECT_TRUE(fake_state->last_aggregate_request->columns.empty()); +} + +TEST(TableReaderTest, PushDownCountFallsBackWhenSemanticArgumentsAreAbsent) { + const auto nullable_int_type = make_nullable(std::make_shared()); + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", nullable_int_type)); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", nullable_int_type)); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + fake_state->aggregate_count = 3; + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + // Simulate an old FE: it can request COUNT pushdown but cannot + // serialize push_down_count_slot_ids. + .push_down_count_columns = std::nullopt, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_FALSE(eos); + EXPECT_EQ(block.rows(), 2); + // The explicit aggregate_count=3 would be returned if absence were confused with COUNT(*). + EXPECT_FALSE(fake_state->last_aggregate_request.has_value()); + EXPECT_FALSE(reader.current_split_uses_metadata_count()); +} + +TEST(TableReaderTest, PushDownCountFallsBackForMultipleArguments) { + const auto nullable_int_type = make_nullable(std::make_shared()); + const auto nullable_string_type = make_nullable(std::make_shared()); + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", nullable_int_type)); + file_schema.push_back(make_file_column(1, "name", nullable_string_type)); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", nullable_int_type)); + projected_columns.push_back(make_table_column(1, "name", nullable_string_type)); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + fake_state->aggregate_count = 3; + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE( + reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = + std::vector {GlobalIndex(0), GlobalIndex(1)}, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_FALSE(eos); + EXPECT_EQ(block.rows(), 2); + EXPECT_FALSE(fake_state->last_aggregate_request.has_value()); + EXPECT_FALSE(reader.current_split_uses_metadata_count()); +} + +TEST(TableReaderTest, PushDownCountFallsBackForNullableToRequiredMapping) { + const auto nullable_int_type = make_nullable(std::make_shared()); + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", nullable_int_type)); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + // make_table_column models the usual nullable external-table descriptor. Override it here to + // reproduce an evolved table contract that declares the mapped physical column required. + projected_columns[0].type = std::make_shared(); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + fake_state->aggregate_count = 3; + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = + std::vector {GlobalIndex(0)}, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + // The normal scan rejects the nullable physical column because it cannot satisfy the required + // table contract. Footer COUNT would bypass that validation and incorrectly return 3. + EXPECT_FALSE(reader.get_block(&block, &eos).ok()); + EXPECT_FALSE(fake_state->last_aggregate_request.has_value()); +} + +TEST(TableReaderTest, PushDownCountFallsBackForCastMapping) { + const auto nullable_int_type = make_nullable(std::make_shared()); + const auto nullable_bigint_type = make_nullable(std::make_shared()); + std::vector file_schema; + file_schema.push_back(make_file_column(0, "id", nullable_int_type)); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", nullable_bigint_type)); + set_name_identifiers(&projected_columns); + + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + auto fake_state = std::make_shared(); + fake_state->aggregate_count = 3; + FakeTableReader reader(file_schema, fake_state); + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = + std::vector {GlobalIndex(0)}, + }) + .ok()); + + SplitReadOptions split_options; + split_options.current_range.__set_path("fake-table-reader-input"); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + Block block = build_table_block(projected_columns); + bool eos = false; + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + EXPECT_FALSE(eos); + EXPECT_EQ(block.rows(), 2); + EXPECT_TRUE(block.get_by_position(0).type->equals(*nullable_bigint_type)); + // INT->BIGINT is a non-trivial mapping, so COUNT must not skip the cast/materialization path. + EXPECT_FALSE(fake_state->last_aggregate_request.has_value()); } TEST(TableReaderTest, PushDownCountStopConvertsAggregateEndOfFileToEos) { @@ -1780,6 +2016,7 @@ TEST(TableReaderTest, PushDownCountStopConvertsAggregateEndOfFileToEos) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); @@ -1820,6 +2057,7 @@ TEST(TableReaderTest, DebugStringCoversReaderStateAndEnumNames) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); @@ -2466,6 +2704,7 @@ TEST(TableReaderTest, PushDownCountFromNewParquetReader) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); ASSERT_TRUE(reader.prepare_split(build_split_options(file_path)).ok()); @@ -2507,6 +2746,7 @@ TEST(TableReaderTest, TableLevelCountUsesAssignedRowCount) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); auto split_options = build_split_options(file_path); @@ -2538,6 +2778,56 @@ TEST(TableReaderTest, TableLevelCountUsesAssignedRowCount) { std::filesystem::remove_all(test_dir); } +TEST(TableReaderTest, TableLevelCountRequiresExplicitCountStarArguments) { + const auto test_dir = std::filesystem::temp_directory_path() / + "doris_table_reader_table_count_arguments_test"; + std::filesystem::remove_all(test_dir); + std::filesystem::create_directories(test_dir); + + const auto file_path = (test_dir / "split.parquet").string(); + write_int_pair_parquet_file(file_path, {1, 2, 3}, {10, 20, 30}, {"one", "two", "three"}); + + std::vector projected_columns; + projected_columns.push_back(make_table_column(0, "id", std::make_shared())); + set_name_identifiers(&projected_columns); + + const std::vector>> unsafe_count_arguments { + std::nullopt, std::vector {GlobalIndex(0)}}; + for (const auto& count_arguments : unsafe_count_arguments) { + RuntimeState state {TQueryOptions(), TQueryGlobals()}; + TableReader reader; + ASSERT_TRUE(reader.init({ + .projected_columns = projected_columns, + .conjuncts = {}, + .format = FileFormat::PARQUET, + .scan_params = nullptr, + .io_ctx = nullptr, + .runtime_state = &state, + .scanner_profile = nullptr, + .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = count_arguments, + }) + .ok()); + auto split_options = build_split_options(file_path); + // Five metadata rows deliberately disagree with the three physical rows. nullopt models an + // old FE, while the non-empty vector models COUNT(id); neither may be treated as COUNT(*). + set_table_level_row_count(&split_options, 5); + ASSERT_TRUE(reader.prepare_split(split_options).ok()); + + size_t total_rows = 0; + bool eos = false; + while (!eos) { + Block block = build_table_block(projected_columns); + ASSERT_TRUE(reader.get_block(&block, &eos).ok()); + total_rows += block.rows(); + } + EXPECT_EQ(3, total_rows); + ASSERT_TRUE(reader.close().ok()); + } + + std::filesystem::remove_all(test_dir); +} + TEST(TableReaderTest, PushDownMinMaxFromNewParquetReader) { const auto test_dir = std::filesystem::temp_directory_path() / "doris_table_reader_minmax_test"; std::filesystem::remove_all(test_dir); @@ -3354,6 +3644,7 @@ TEST(TableReaderTest, PushDownCountOnlyUsesSelectedRowGroupInFileRange) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); ASSERT_TRUE(reader.prepare_split(build_split_options_for_row_group_mid(file_path, 2)).ok()); @@ -3393,6 +3684,7 @@ TEST(TableReaderTest, PushDownCountFallsBackWithTableConjunct) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); ASSERT_TRUE(reader.prepare_split(build_split_options(file_path)).ok()); @@ -3434,6 +3726,7 @@ TEST(TableReaderTest, PushDownCountFallsBackWithFilter) { .runtime_state = &state, .scanner_profile = nullptr, .push_down_agg_type = TPushAggOp::type::COUNT, + .push_down_count_columns = std::vector {}, }) .ok()); ASSERT_TRUE(reader.prepare_split(build_split_options(file_path)).ok()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/FileScanNode.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/FileScanNode.java index 466dad11d5dc49..53b0d11c5c7283 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/FileScanNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/FileScanNode.java @@ -88,6 +88,8 @@ public FileScanNode(PlanNodeId id, TupleDescriptor desc, String planNodeName, @Override protected void toThrift(TPlanNode planNode) { planNode.setPushDownAggTypeOpt(pushDownAggNoGroupingOp); + planNode.setPushDownCountSlotIds( + pushDownCountSlotIds.stream().map(id -> id.asInt()).collect(Collectors.toList())); planNode.setNodeType(TPlanNodeType.FILE_SCAN_NODE); TFileScanNode fileScanNode = new TFileScanNode(); @@ -103,6 +105,21 @@ protected void setPushDownCount(long count) { tableLevelRowCount = count; } + /** + * Return whether FE may replace real table-format splits with metadata COUNT splits. + * + *

The aggregate opcode alone is insufficient because both {@code COUNT(*)} and + * {@code COUNT(col)} use {@link TPushAggOp#COUNT}. The semantic argument list distinguishes + * them: it is empty only for {@code COUNT(*)}/{@code COUNT(1)}. For example, if an Iceberg + * table has 100 data files, retaining one representative split is correct for a snapshot + * {@code COUNT(*)}. Doing that for {@code COUNT(required_col)} is unsafe: BE deliberately + * falls back to reading the column, but it would then see only the representative file and + * undercount the table. + */ + protected boolean isTableLevelCountStarPushdown() { + return pushDownAggNoGroupingOp == TPushAggOp.COUNT && pushDownCountSlotIds.isEmpty(); + } + private long getPushDownCount() { return tableLevelRowCount; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/source/HiveScanNode.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/source/HiveScanNode.java index 3ea39987d021f7..cae6728c5f467f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/source/HiveScanNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/source/HiveScanNode.java @@ -60,7 +60,6 @@ import org.apache.doris.thrift.TFileRangeDesc; import org.apache.doris.thrift.TFileScanRangeParams; import org.apache.doris.thrift.TFileTextScanRangeParams; -import org.apache.doris.thrift.TPushAggOp; import org.apache.doris.thrift.TTableFormatFileDesc; import org.apache.doris.thrift.TTransactionalHiveDeleteDeltaDesc; import org.apache.doris.thrift.TTransactionalHiveDesc; @@ -339,7 +338,7 @@ private void getFileSplitByPartitions(HiveExternalMetaCache cache, List getSplits(int numBackends) throws UserException { ++paimonSplitNum; } - boolean applyCountPushdown = getPushDownAggNoGroupingOp() == TPushAggOp.COUNT; + // Merged row counts contain only COUNT(*) semantics. COUNT(col) must keep every DataSplit + // because BE will read the argument column to account for NULL and schema-mapping rules. + boolean applyCountPushdown = isTableLevelCountStarPushdown(); // Used to avoid repeatedly calculating partition info map for the same // partition data. // And for counting the number of selected partitions for this paimon table. diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/tvf/source/TVFScanNode.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/tvf/source/TVFScanNode.java index ce22e96c80b58e..2124318c5a6fbc 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/tvf/source/TVFScanNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/tvf/source/TVFScanNode.java @@ -47,7 +47,6 @@ import org.apache.doris.thrift.TFileFormatType; import org.apache.doris.thrift.TFileRangeDesc; import org.apache.doris.thrift.TFileType; -import org.apache.doris.thrift.TPushAggOp; import org.apache.doris.thrift.TTableFormatFileDesc; import com.google.common.collect.Lists; @@ -143,9 +142,9 @@ public List getSplits(int numBackends) throws UserException { List fileStatuses = tableValuedFunction.getFileStatuses(); - // Push down count optimization. + // Avoid splitting only for table-level COUNT(*). COUNT(column) still reads column data. boolean needSplit = true; - if (getPushDownAggNoGroupingOp() == TPushAggOp.COUNT) { + if (isTableLevelCountStarPushdown()) { int parallelNum = sessionVariable.getParallelExecInstanceNum(scanContext.getClusterName()); int totalFileNum = fileStatuses.size(); needSplit = FileSplitter.needSplitForCountPushdown(parallelNum, numBackends, totalFileNum); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java index 62e35f744732ad..f42112d1e11d28 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java @@ -850,6 +850,11 @@ private PlanFragment getPlanFragmentForPhysicalFileScan(PhysicalFileScan fileSca scanNode.setNereidsId(fileScan.getId()); context.getNereidsIdToPlanNodeIdMap().put(fileScan.getId(), scanNode.getId()); scanNode.setPushDownAggNoGrouping(context.getRelationPushAggOp(fileScan.getRelationId())); + scanNode.setPushDownCountSlotIds(context.getRelationPushCountArgumentExprIds(fileScan.getRelationId()) + .stream() + .map(exprId -> Objects.requireNonNull(context.findSlotRef(exprId), + "missing slot for pushed-down COUNT argument " + exprId).getSlotId()) + .collect(Collectors.toList())); scanNode.setHasPartitionPredicate(fileScan.hasPartitionPredicate()); if (fileScan.getStats() != null) { @@ -1401,6 +1406,10 @@ public PlanFragment visitPhysicalStorageLayerAggregate( context.setRelationPushAggOp( storageLayerAggregate.getRelation().getRelationId(), pushAggOp); + context.setRelationPushCountArgumentExprIds( + storageLayerAggregate.getRelation().getRelationId(), + pushAggOp == TPushAggOp.COUNT + ? storageLayerAggregate.getCountArgumentExprIds() : ImmutableList.of()); PlanFragment planFragment = storageLayerAggregate.getRelation().accept(this, context); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java index f680936b5e5ef4..936f4cbe1a9235 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java @@ -52,6 +52,7 @@ import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; @@ -106,6 +107,7 @@ public class PlanTranslatorContext { private final Map cteScanNodeMap = Maps.newHashMap(); private final Map tablePushAggOp = Maps.newHashMap(); + private final Map> tablePushCountArgumentExprIds = Maps.newHashMap(); private final Map> statsUnknownColumnsMap = Maps.newHashMap(); @@ -430,6 +432,14 @@ public TPushAggOp getRelationPushAggOp(RelationId relationId) { return tablePushAggOp.getOrDefault(relationId, TPushAggOp.NONE); } + public void setRelationPushCountArgumentExprIds(RelationId relationId, List exprIds) { + tablePushCountArgumentExprIds.put(relationId, Lists.newArrayList(exprIds)); + } + + public List getRelationPushCountArgumentExprIds(RelationId relationId) { + return tablePushCountArgumentExprIds.getOrDefault(relationId, Collections.emptyList()); + } + public boolean isTopMaterializeNode() { return isTopMaterializeNode; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java index 975cfaf973c408..e91d458a098c81 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java @@ -33,6 +33,7 @@ import org.apache.doris.nereids.rules.expression.rules.FoldConstantRuleOnFE; import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.Cast; +import org.apache.doris.nereids.trees.expressions.ExprId; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.IsNull; import org.apache.doris.nereids.trees.expressions.Or; @@ -570,6 +571,7 @@ private LogicalAggregate storageLayerAggregate( Map, PushDownAggOp> supportedAgg = PushDownAggOp.supportedFunctions(); boolean containsCount = false; + boolean countHasCastArgument = false; Set checkNullSlots = new HashSet<>(); Set expressionAfterProject = new HashSet<>(); @@ -591,6 +593,7 @@ private LogicalAggregate storageLayerAggregate( checkNullSlots.add((SlotReference) arg0); expressionAfterProject.add(arg0); } else if (arg0 instanceof Cast) { + countHasCastArgument = true; Expression child0 = arg0.child(0); if (child0 instanceof SlotReference) { checkNullSlots.add((SlotReference) child0); @@ -667,6 +670,7 @@ private LogicalAggregate storageLayerAggregate( return canNotPush; } else { if (needCheckSlotNull) { + countHasCastArgument = true; checkNullSlots.add((SlotReference) argument.child(0)); } } @@ -677,6 +681,16 @@ private LogicalAggregate storageLayerAggregate( argumentsOfAggregateFunction = processedExpressions; } + // File aggregate metadata can describe COUNT(*) or COUNT(file_column), but it cannot + // describe the CAST wrapped around a COUNT argument. Dropping that CAST is incorrect even + // when the source column is NOT NULL. For example, a non-null DOUBLE value outside the INT + // range becomes NULL for CAST(double_col AS INT), so COUNT(CAST(double_col AS INT)) must + // exclude it while a footer-level COUNT(double_col) would include it. Keep OLAP's existing + // storage-layer behavior unchanged, and make external files evaluate the CAST normally. + if (logicalScan instanceof LogicalFileScan && countHasCastArgument) { + return canNotPush; + } + Set pushDownAggOps = functionClasses.stream() .map(supportedAgg::get) .collect(Collectors.toSet()); @@ -690,6 +704,12 @@ private LogicalAggregate storageLayerAggregate( List usedSlotInTable = (List) Project.findProject(aggUsedSlots, logicalScan.getOutput()); + // COUNT(*) has no aggregate arguments, even though later column pruning retains one + // arbitrary scan slot. Preserve the semantic arguments here so the BE never needs to infer + // COUNT(col) from the post-pruning scan shape. + List countArgumentExprIds = mergeOp == PushDownAggOp.COUNT + ? usedSlotInTable.stream().map(SlotReference::getExprId).collect(Collectors.toList()) + : ImmutableList.of(); for (SlotReference slot : usedSlotInTable) { Optional optionalColumn = slot.getOriginalColumn(); @@ -738,11 +758,12 @@ private LogicalAggregate storageLayerAggregate( if (project != null) { return aggregate.withChildren(ImmutableList.of( project.withChildren( - ImmutableList.of(new PhysicalStorageLayerAggregate(physicalScan, mergeOp))) + ImmutableList.of(new PhysicalStorageLayerAggregate( + physicalScan, mergeOp, countArgumentExprIds))) )); } else { return aggregate.withChildren(ImmutableList.of( - new PhysicalStorageLayerAggregate(physicalScan, mergeOp) + new PhysicalStorageLayerAggregate(physicalScan, mergeOp, countArgumentExprIds) )); } @@ -754,11 +775,12 @@ private LogicalAggregate storageLayerAggregate( if (project != null) { return aggregate.withChildren(ImmutableList.of( project.withChildren( - ImmutableList.of(new PhysicalStorageLayerAggregate(physicalScan, mergeOp))) + ImmutableList.of(new PhysicalStorageLayerAggregate( + physicalScan, mergeOp, countArgumentExprIds))) )); } else { return aggregate.withChildren(ImmutableList.of( - new PhysicalStorageLayerAggregate(physicalScan, mergeOp) + new PhysicalStorageLayerAggregate(physicalScan, mergeOp, countArgumentExprIds) )); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalStorageLayerAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalStorageLayerAggregate.java index 2db9c1afc27492..1d4255b6df98ef 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalStorageLayerAggregate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalStorageLayerAggregate.java @@ -20,6 +20,7 @@ import org.apache.doris.nereids.memo.GroupExpression; import org.apache.doris.nereids.properties.LogicalProperties; import org.apache.doris.nereids.properties.PhysicalProperties; +import org.apache.doris.nereids.trees.expressions.ExprId; import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction; import org.apache.doris.nereids.trees.expressions.functions.agg.Count; import org.apache.doris.nereids.trees.expressions.functions.agg.Max; @@ -43,21 +44,30 @@ public class PhysicalStorageLayerAggregate extends PhysicalCatalogRelation { private final PhysicalCatalogRelation relation; private final PushDownAggOp aggOp; + private final List countArgumentExprIds; public PhysicalStorageLayerAggregate(PhysicalCatalogRelation relation, PushDownAggOp aggOp) { + this(relation, aggOp, ImmutableList.of()); + } + + public PhysicalStorageLayerAggregate(PhysicalCatalogRelation relation, PushDownAggOp aggOp, + List countArgumentExprIds) { super(relation.getRelationId(), relation.getType(), relation.getTable(), relation.getQualifier(), Optional.empty(), relation.getLogicalProperties(), ImmutableList.of()); this.relation = Objects.requireNonNull(relation, "relation cannot be null"); this.aggOp = Objects.requireNonNull(aggOp, "aggOp cannot be null"); + this.countArgumentExprIds = ImmutableList.copyOf(countArgumentExprIds); } public PhysicalStorageLayerAggregate(PhysicalCatalogRelation relation, PushDownAggOp aggOp, + List countArgumentExprIds, Optional groupExpression, LogicalProperties logicalProperties, PhysicalProperties physicalProperties, Statistics statistics) { super(relation.getRelationId(), relation.getType(), relation.getTable(), relation.getQualifier(), groupExpression, logicalProperties, physicalProperties, statistics, ImmutableList.of()); this.relation = Objects.requireNonNull(relation, "relation cannot be null"); this.aggOp = Objects.requireNonNull(aggOp, "aggOp cannot be null"); + this.countArgumentExprIds = ImmutableList.copyOf(countArgumentExprIds); } public PhysicalRelation getRelation() { @@ -68,6 +78,10 @@ public PushDownAggOp getAggOp() { return aggOp; } + public List getCountArgumentExprIds() { + return countArgumentExprIds; + } + @Override public R accept(PlanVisitor visitor, C context) { return visitor.visitPhysicalStorageLayerAggregate(this, context); @@ -83,20 +97,21 @@ public String toString() { } public PhysicalStorageLayerAggregate withPhysicalOlapScan(PhysicalOlapScan physicalOlapScan) { - return AbstractPlan.copyWithSameId(this, () -> new PhysicalStorageLayerAggregate(physicalOlapScan, aggOp)); + return AbstractPlan.copyWithSameId(this, () -> new PhysicalStorageLayerAggregate( + physicalOlapScan, aggOp, countArgumentExprIds)); } @Override public PhysicalStorageLayerAggregate withGroupExpression(Optional groupExpression) { return AbstractPlan.copyWithSameId(this, () -> new PhysicalStorageLayerAggregate(relation, aggOp, - groupExpression, getLogicalProperties(), physicalProperties, statistics)); + countArgumentExprIds, groupExpression, getLogicalProperties(), physicalProperties, statistics)); } @Override public Plan withGroupExprLogicalPropChildren(Optional groupExpression, Optional logicalProperties, List children) { return AbstractPlan.copyWithSameId(this, () -> new PhysicalStorageLayerAggregate(relation, aggOp, - groupExpression, logicalProperties.get(), physicalProperties, statistics)); + countArgumentExprIds, groupExpression, logicalProperties.get(), physicalProperties, statistics)); } @Override @@ -104,7 +119,7 @@ public PhysicalPlan withPhysicalPropertiesAndStats(PhysicalProperties physicalPr Statistics statistics) { return AbstractPlan.copyWithSameId(this, () -> new PhysicalStorageLayerAggregate( (PhysicalCatalogRelation) relation.withPhysicalPropertiesAndStats(null, statistics), - aggOp, groupExpression, + aggOp, countArgumentExprIds, groupExpression, getLogicalProperties(), physicalProperties, statistics)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java index c1f97fdeb96e20..1fc55fe8f5e1c0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java @@ -771,11 +771,18 @@ public void setCardinalityAfterFilter(long cardinalityAfterFilter) { } protected TPushAggOp pushDownAggNoGroupingOp = TPushAggOp.NONE; + // Explicit COUNT arguments. COUNT(*)/COUNT(1) intentionally keep this empty even though + // column pruning retains one placeholder scan slot. + protected List pushDownCountSlotIds = Collections.emptyList(); public void setPushDownAggNoGrouping(TPushAggOp pushDownAggNoGroupingOp) { this.pushDownAggNoGroupingOp = pushDownAggNoGroupingOp; } + public void setPushDownCountSlotIds(List pushDownCountSlotIds) { + this.pushDownCountSlotIds = Lists.newArrayList(pushDownCountSlotIds); + } + public void setChildrenDistributeExprLists(List> childrenDistributeExprLists) { this.childrenDistributeExprLists = childrenDistributeExprLists; } diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java index d4273f6faa4cc5..3505a5c5d11e42 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/source/IcebergScanNodeTest.java @@ -32,6 +32,7 @@ import org.apache.doris.system.Backend; import org.apache.doris.thrift.TFileRangeDesc; import org.apache.doris.thrift.TIcebergDeleteFileDesc; +import org.apache.doris.thrift.TPushAggOp; import org.apache.iceberg.DataFile; import org.apache.iceberg.FileFormat; @@ -154,6 +155,56 @@ public void testSystemTableProjectionRejectsUnmaterializedFilterColumn() throws } } + private static class CountPlanningIcebergScanNode extends IcebergScanNode { + private final TableScan tableScan; + private final long snapshotCount; + private int snapshotCountCalls; + + CountPlanningIcebergScanNode(SessionVariable sv, TableScan tableScan, long snapshotCount) { + super(new PlanNodeId(0), new TupleDescriptor(new TupleId(0)), sv, ScanContext.EMPTY); + this.tableScan = tableScan; + this.snapshotCount = snapshotCount; + } + + @Override + public TableScan createTableScan() { + return tableScan; + } + + @Override + public long getCountFromSnapshot() { + ++snapshotCountCalls; + return snapshotCount; + } + } + + @Test + public void testTableLevelCountSplitPlanningRequiresCountStar() { + SessionVariable sv = Mockito.mock(SessionVariable.class); + Mockito.when(sv.getEnableExternalTableBatchMode()).thenReturn(false); + TableScan tableScan = Mockito.mock(TableScan.class); + Mockito.when(tableScan.snapshot()).thenReturn(Mockito.mock(Snapshot.class)); + + // COUNT(required_col) carries a non-empty semantic argument list. Even though its result + // equals COUNT(*) for valid data, BE intentionally reads the column to enforce schema + // contracts. FE must therefore leave all real file tasks available to that fallback. + CountPlanningIcebergScanNode countColumnNode = + new CountPlanningIcebergScanNode(sv, tableScan, 30_000); + countColumnNode.setPushDownAggNoGrouping(TPushAggOp.COUNT); + countColumnNode.setPushDownCountSlotIds(Collections.singletonList(new SlotId(7))); + Assert.assertFalse(countColumnNode.isBatchMode()); + Assert.assertEquals(0, countColumnNode.snapshotCountCalls); + + // COUNT(*) has an explicitly empty argument list, so snapshot row count remains eligible + // and doGetSplits may retain only representative tasks for parallel materialization. + CountPlanningIcebergScanNode countStarNode = + new CountPlanningIcebergScanNode(sv, tableScan, 30_000); + countStarNode.setPushDownAggNoGrouping(TPushAggOp.COUNT); + countStarNode.setPushDownCountSlotIds(Collections.emptyList()); + Assert.assertFalse(countStarNode.isBatchMode()); + Assert.assertEquals(1, countStarNode.snapshotCountCalls); + } + @Test public void testInitialDefaultMetadataUsesSnapshotSchema() throws Exception { Schema snapshotSchema = new Schema(Types.NestedField.optional("historical_binary") diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/paimon/source/PaimonScanNodeTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/paimon/source/PaimonScanNodeTest.java index 034be2185e20f9..9f8583ed1bc860 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/paimon/source/PaimonScanNodeTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/paimon/source/PaimonScanNodeTest.java @@ -17,6 +17,7 @@ package org.apache.doris.datasource.paimon.source; +import org.apache.doris.analysis.SlotId; import org.apache.doris.analysis.TupleDescriptor; import org.apache.doris.analysis.TupleId; import org.apache.doris.common.ExceptionChecker; @@ -35,6 +36,7 @@ import org.apache.doris.qe.SessionVariable; import org.apache.doris.thrift.TFileRangeDesc; import org.apache.doris.thrift.TFileScanRangeParams; +import org.apache.doris.thrift.TPushAggOp; import org.apache.paimon.data.BinaryRow; import org.apache.paimon.io.DataFileMeta; @@ -68,6 +70,38 @@ public class PaimonScanNodeTest { @Mock private PaimonFileExternalCatalog paimonFileExternalCatalog; + @Test + public void testCountColumnKeepsAllSplitsWhileCountStarUsesMergedRowCount() throws UserException { + PaimonScanNode node = Mockito.spy(newTestNode(new PlanNodeId(1), new TupleId(3), sv)); + node.setSource(mockPaimonSourceWithPartitionKeys(Collections.emptyList())); + List dataSplits = Arrays.asList( + mockCountDataSplit("f1.parquet", 4_000), + mockCountDataSplit("f2.parquet", 5_000), + mockCountDataSplit("f3.parquet", 6_000)); + Mockito.doReturn(dataSplits).when(node).getPaimonSplitFromAPI(); + Mockito.when(sv.isForceJniScanner()).thenReturn(true); + Mockito.when(sv.getIgnoreSplitType()).thenReturn("NONE"); + Mockito.when(sv.getParallelExecInstanceNum(ArgumentMatchers.nullable(String.class))).thenReturn(1); + + // Before the fix, the raw COUNT opcode made this path keep only parallel representative + // splits and attach the 15,000 metadata rows. BE rejects that shortcut for COUNT(col), so + // it would scan only those representatives and silently miss the discarded DataSplits. + node.setPushDownAggNoGrouping(TPushAggOp.COUNT); + node.setPushDownCountSlotIds(Collections.singletonList(new SlotId(7))); + List countColumnSplits = node.getSplits(1); + Assert.assertEquals(3, countColumnSplits.size()); + for (org.apache.doris.spi.Split split : countColumnSplits) { + Assert.assertFalse(((PaimonSplit) split).getRowCount().isPresent()); + } + + // COUNT(*) remains metadata-only. The 15,000 rows exceed the parallel threshold, so one + // configured execution instance retains one representative split carrying the full sum. + node.setPushDownCountSlotIds(Collections.emptyList()); + List countStarSplits = node.getSplits(1); + Assert.assertEquals(1, countStarSplits.size()); + Assert.assertEquals(Optional.of(15_000L), ((PaimonSplit) countStarSplits.get(0)).getRowCount()); + } + @Test public void testSplitWeight() throws UserException { @@ -692,4 +726,20 @@ private DataSplit createDataSplit(String fileName) { .withDataFiles(Collections.singletonList(dataFileMeta)) .build(); } + + private DataSplit mockCountDataSplit(String fileName, long rowCount) { + DataFileMeta dataFileMeta = DataFileMeta.forAppend(fileName, 64L * 1024 * 1024, rowCount, + SimpleStats.EMPTY_STATS, 1L, 1L, 1L, Collections.emptyList(), null, + FileSource.APPEND, Collections.emptyList(), null, null, + Collections.emptyList()); + DataSplit dataSplit = Mockito.mock(DataSplit.class); + Mockito.when(dataSplit.rowCount()).thenReturn(rowCount); + Mockito.when(dataSplit.mergedRowCountAvailable()).thenReturn(true); + Mockito.when(dataSplit.mergedRowCount()).thenReturn(rowCount); + Mockito.when(dataSplit.partition()).thenReturn(BinaryRow.singleColumn(1)); + Mockito.when(dataSplit.dataFiles()).thenReturn(Collections.singletonList(dataFileMeta)); + Mockito.when(dataSplit.convertToRawFiles()).thenReturn(Optional.empty()); + Mockito.when(dataSplit.deletionFiles()).thenReturn(Optional.empty()); + return dataSplit; + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/tvf/source/TVFScanNodeTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/tvf/source/TVFScanNodeTest.java index 8cf98daea94c59..8ea9041480dadd 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/tvf/source/TVFScanNodeTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/tvf/source/TVFScanNodeTest.java @@ -17,14 +17,20 @@ package org.apache.doris.datasource.tvf.source; +import org.apache.doris.analysis.SlotId; import org.apache.doris.analysis.TupleDescriptor; import org.apache.doris.analysis.TupleId; import org.apache.doris.catalog.FunctionGenTable; +import org.apache.doris.datasource.FileQueryScanNode; +import org.apache.doris.datasource.FileSplitter; import org.apache.doris.planner.PlanNodeId; import org.apache.doris.planner.ScanContext; import org.apache.doris.qe.SessionVariable; +import org.apache.doris.spi.Split; import org.apache.doris.tablefunction.ExternalFileTableValuedFunction; import org.apache.doris.thrift.TBrokerFileStatus; +import org.apache.doris.thrift.TFileType; +import org.apache.doris.thrift.TPushAggOp; import org.junit.Assert; import org.junit.Test; @@ -37,6 +43,40 @@ public class TVFScanNodeTest { private static final long MB = 1024L * 1024L; + @Test + public void testCountColumnKeepsNormalFileSplitting() throws Exception { + SessionVariable sv = new SessionVariable(); + sv.parallelExecInstanceNum = 1; + sv.setFileSplitSize(32 * MB); + TupleDescriptor desc = new TupleDescriptor(new TupleId(0)); + FunctionGenTable table = Mockito.mock(FunctionGenTable.class); + ExternalFileTableValuedFunction tvf = Mockito.mock(ExternalFileTableValuedFunction.class); + Mockito.when(table.getTvf()).thenReturn(tvf); + Mockito.when(tvf.getTFileType()).thenReturn(TFileType.FILE_LOCAL); + Mockito.when(tvf.getFileStatuses()).thenReturn(List.of( + splittableFile("file:///tmp/count_col_1.parquet", 128 * MB), + splittableFile("file:///tmp/count_col_2.parquet", 128 * MB))); + desc.setTable(table); + + TVFScanNode countColumnNode = new TVFScanNode( + new PlanNodeId(0), desc, false, sv, ScanContext.EMPTY); + setFileSplitter(countColumnNode, new FileSplitter(32 * MB, 32 * MB, 0)); + countColumnNode.setPushDownAggNoGrouping(TPushAggOp.COUNT); + countColumnNode.setPushDownCountSlotIds(Collections.singletonList(new SlotId(7))); + + List countColumnSplits = countColumnNode.getSplits(1); + Assert.assertEquals(8, countColumnSplits.size()); + + TVFScanNode countStarNode = new TVFScanNode( + new PlanNodeId(1), desc, false, sv, ScanContext.EMPTY); + setFileSplitter(countStarNode, new FileSplitter(32 * MB, 32 * MB, 0)); + countStarNode.setPushDownAggNoGrouping(TPushAggOp.COUNT); + countStarNode.setPushDownCountSlotIds(Collections.emptyList()); + + List countStarSplits = countStarNode.getSplits(1); + Assert.assertEquals(2, countStarSplits.size()); + } + @Test public void testDetermineTargetFileSplitSizeHonorsMaxFileSplitNum() throws Exception { SessionVariable sv = new SessionVariable(); @@ -57,4 +97,19 @@ public void testDetermineTargetFileSplitSizeHonorsMaxFileSplitNum() throws Excep long target = (long) method.invoke(node, statuses); Assert.assertEquals(100 * MB, target); } + + private static TBrokerFileStatus splittableFile(String path, long size) { + TBrokerFileStatus status = new TBrokerFileStatus(); + status.setPath(path); + status.setSize(size); + status.setModificationTime(0); + status.setIsSplitable(true); + return status; + } + + private static void setFileSplitter(TVFScanNode node, FileSplitter splitter) throws Exception { + java.lang.reflect.Field field = FileQueryScanNode.class.getDeclaredField("fileSplitter"); + field.setAccessible(true); + field.set(node, splitter); + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java index bf5b74f2ea6f25..eb3a42c3868610 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PhysicalStorageLayerAggregateTest.java @@ -75,7 +75,26 @@ public void testWithoutProject() { .applyImplementation(storageLayerAggregateWithoutProject()) .matches( logicalAggregate( - physicalStorageLayerAggregate().when(agg -> agg.getAggOp() == PushDownAggOp.COUNT) + physicalStorageLayerAggregate().when(agg -> agg.getAggOp() == PushDownAggOp.COUNT + && agg.getCountArgumentExprIds().equals( + ImmutableList.of(olapScan.getOutput().get(0).getExprId()))) + ) + ); + + // COUNT(*) still keeps a placeholder scan slot after column pruning, so its semantic + // argument list must remain empty instead of being inferred from the physical scan shape. + aggregate = new LogicalAggregate<>( + Collections.emptyList(), + ImmutableList.of(new Alias(new Count(), "count_star")), + true, Optional.empty(), olapScan); + context = MemoTestUtils.createCascadesContext(aggregate); + + PlanChecker.from(context) + .applyImplementation(storageLayerAggregateWithoutProject()) + .matches( + logicalAggregate( + physicalStorageLayerAggregate().when(agg -> agg.getAggOp() == PushDownAggOp.COUNT + && agg.getCountArgumentExprIds().isEmpty()) ) ); @@ -138,7 +157,9 @@ public void testWithProject() { .matches( logicalAggregate( logicalProject( - physicalStorageLayerAggregate().when(agg -> agg.getAggOp() == PushDownAggOp.COUNT) + physicalStorageLayerAggregate().when(agg -> agg.getAggOp() == PushDownAggOp.COUNT + && agg.getCountArgumentExprIds().equals( + ImmutableList.of(olapScan.getOutput().get(0).getExprId()))) ) ) ); diff --git a/gensrc/thrift/PlanNodes.thrift b/gensrc/thrift/PlanNodes.thrift index b546649933c239..dddc2d17c3215c 100644 --- a/gensrc/thrift/PlanNodes.thrift +++ b/gensrc/thrift/PlanNodes.thrift @@ -1713,6 +1713,10 @@ struct TPlanNode { 52: optional TRecCTEScanNode rec_cte_scan_node 53: optional TBucketedAggregationNode bucketed_agg_node 54: optional TLocalExchangeNode local_exchange_node + // COUNT(*) and COUNT(col) share push_down_agg_type_opt=COUNT, but file readers need to know + // whether a projected scan slot is the aggregate argument or merely the placeholder retained by + // column pruning. Empty means row-count semantics; non-empty identifies explicit COUNT columns. + 55: optional list push_down_count_slot_ids // projections is final projections, which means projecting into results and materializing them into the output block. 101: optional list projections diff --git a/regression-test/data/external_table_p0/tvf/file_scanner_v2_empty.csv b/regression-test/data/external_table_p0/tvf/file_scanner_v2_empty.csv new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/regression-test/data/external_table_p0/tvf/file_scanner_v2_struct_0_bigint.parquet b/regression-test/data/external_table_p0/tvf/file_scanner_v2_struct_0_bigint.parquet new file mode 100644 index 00000000000000..22eff58b7a2389 Binary files /dev/null and b/regression-test/data/external_table_p0/tvf/file_scanner_v2_struct_0_bigint.parquet differ diff --git a/regression-test/data/external_table_p0/tvf/file_scanner_v2_struct_1_int.parquet b/regression-test/data/external_table_p0/tvf/file_scanner_v2_struct_1_int.parquet new file mode 100644 index 00000000000000..5a9ca0baa47e89 Binary files /dev/null and b/regression-test/data/external_table_p0/tvf/file_scanner_v2_struct_1_int.parquet differ diff --git a/regression-test/data/external_table_p0/tvf/file_scanner_v2_unsupported_time.parquet b/regression-test/data/external_table_p0/tvf/file_scanner_v2_unsupported_time.parquet new file mode 100644 index 00000000000000..5b973181a56148 Binary files /dev/null and b/regression-test/data/external_table_p0/tvf/file_scanner_v2_unsupported_time.parquet differ diff --git a/regression-test/data/external_table_p0/tvf/test_file_scanner_v2_review_fixes.out b/regression-test/data/external_table_p0/tvf/test_file_scanner_v2_review_fixes.out new file mode 100644 index 00000000000000..c8312571c56e36 --- /dev/null +++ b/regression-test/data/external_table_p0/tvf/test_file_scanner_v2_review_fixes.out @@ -0,0 +1,17 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !empty_csv -- + +-- !struct_leaf_promotion_cold -- +1 10 100 +3 10 300 + +-- !struct_leaf_promotion_warm -- +1 10 100 +3 10 300 + +-- !count_evolved_struct -- +4 + +-- !unprojected_unsupported_time -- +1 +2 diff --git a/regression-test/suites/external_table_p0/tvf/test_file_scanner_v2_review_fixes.groovy b/regression-test/suites/external_table_p0/tvf/test_file_scanner_v2_review_fixes.groovy new file mode 100644 index 00000000000000..6fa29c321c8e72 --- /dev/null +++ b/regression-test/suites/external_table_p0/tvf/test_file_scanner_v2_review_fixes.groovy @@ -0,0 +1,113 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_file_scanner_v2_review_fixes", "p0,external") { + def backends = sql "show backends" + assertTrue(backends.size() > 0) + def backendId = backends[0][0] + def dataPath = context.config.dataPath + "/external_table_p0/tvf" + // A developer may point this at fixtures already visible to every BE (for example, a shared + // checkout mounted at /tmp). CI leaves it unset and exercises the normal per-BE distribution. + def predeployedFixturePath = context.config.otherConfigs.get("fileScannerV2FixturePath") + def remotePath = predeployedFixturePath ?: "/tmp/file_scanner_v2_review_fixes" + def fixtureNames = [ + "file_scanner_v2_empty.csv", + "file_scanner_v2_struct_0_bigint.parquet", + "file_scanner_v2_struct_1_int.parquet", + "file_scanner_v2_unsupported_time.parquet" + ] + + if (predeployedFixturePath == null) { + mkdirRemotePathOnAllBE("root", remotePath) + for (def backend : backends) { + for (def fixtureName : fixtureNames) { + scpFiles("root", backend[1], "${dataPath}/${fixtureName}", remotePath, false) + } + } + } + + sql "set enable_file_scanner_v2 = true" + + // A zero-byte CSV is a valid zero-row split when the schema is supplied explicitly. The EOF + // raised during reader preparation must skip this split instead of failing the query. + order_qt_empty_csv """ + SELECT id, name + FROM local( + "file_path" = "${remotePath}/file_scanner_v2_empty.csv", + "backend_id" = "${backendId}", + "format" = "csv", + "csv_schema" = "id:int;name:string") + ORDER BY id + """ + + // The first file defines STRUCT, while the second stores a as INT. Localization is + // split-specific: the BIGINT file compares BIGINT values, while the old INT file safely rewrites + // the exactly representable BIGINT literal 10 to INT and compares in the physical file type. + // If a literal cannot round-trip through INT, the mapper instead casts the INT data to BIGINT. + // Repeating the multi-file read also verifies that neither predicate rewrites nor page-cache + // range state leak from one file schema into the next file or the warm scan. + def evolvedStructQuery = """ + SELECT id, col.a, col.b + FROM local( + "file_path" = "${remotePath}/file_scanner_v2_struct_*.parquet", + "backend_id" = "${backendId}", + "format" = "parquet") + WHERE col.a = CAST(10 AS BIGINT) + ORDER BY id + """ + order_qt_struct_leaf_promotion_cold evolvedStructQuery + order_qt_struct_leaf_promotion_warm evolvedStructQuery + + // COUNT(col) sees a non-trivial mapping for the file whose STRUCT leaf is INT while the merged + // table type is BIGINT. It must fall back to the normal scan so schema-evolution casts and + // nullability checks run instead of counting footer values directly. + order_qt_count_evolved_struct """ + SELECT COUNT(col.a) + FROM local( + "file_path" = "${remotePath}/file_scanner_v2_struct_*.parquet", + "backend_id" = "${backendId}", + "format" = "parquet") + """ + + // TIME_MILLIS is intentionally unsupported by the record reader. It must not prevent schema + // construction when only the supported id column is projected. + order_qt_unprojected_unsupported_time """ + SELECT id + FROM local( + "file_path" = "${remotePath}/file_scanner_v2_unsupported_time.parquet", + "backend_id" = "${backendId}", + "format" = "parquet") + ORDER BY id + """ + + // A requested unsupported leaf must fail before metadata pruning. The fixture stores positive + // TIME_MILLIS values, so `unsupported_time < 0` can be eliminated from INT32 row-group + // statistics. Without the early validation, this query incorrectly returns an empty result + // instead of preserving the unsupported logical-type contract. + test { + sql """ + SELECT unsupported_time + FROM local( + "file_path" = "${remotePath}/file_scanner_v2_unsupported_time.parquet", + "backend_id" = "${backendId}", + "format" = "parquet") + WHERE unsupported_time < 0 + """ + exception "Parquet TIME with isAdjustedToUTC=true is not supported" + } + +}