Gabriel39 commented on code in PR #65674: URL: https://github.com/apache/doris/pull/65674#discussion_r3613488376
########## be/src/format_v2/parquet/reader/plain_fixed_predicate.h: ########## @@ -0,0 +1,125 @@ +// 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. + +#pragma once + +#include <array> +#include <cstddef> +#include <cstdint> +#include <cstring> + +#include "common/status.h" +#include "util/unaligned.h" + +namespace doris::format::parquet { + +enum class PlainFixedPredicateOp : uint8_t { EQ, NE, LT, LE, GT, GE }; +enum class PlainFixedPredicateType : uint8_t { INT32, INT64, FLOAT, DOUBLE }; + +// A compiled predicate over the physical little-endian values of a fixed-width PLAIN page. +// Compilation is deliberately restricted to expressions whose Doris value and Parquet physical +// value have identical comparison semantics; casts and logical-type conversions stay on the +// ordinary materialization path. +class PlainFixedPredicate { +public: + template <typename T> + static PlainFixedPredicate create(PlainFixedPredicateType type, PlainFixedPredicateOp op, + T literal) { + PlainFixedPredicate predicate; + predicate._type = type; + predicate._op = op; + static_assert(sizeof(T) <= sizeof(predicate._literal)); + memcpy(predicate._literal.data(), &literal, sizeof(T)); + return predicate; + } + + size_t value_width() const { + switch (_type) { + case PlainFixedPredicateType::INT32: + case PlainFixedPredicateType::FLOAT: + return sizeof(uint32_t); + case PlainFixedPredicateType::INT64: + case PlainFixedPredicateType::DOUBLE: + return sizeof(uint64_t); + } + __builtin_unreachable(); + } + + PlainFixedPredicateType type() const { return _type; } + + // AND this predicate into matches. The caller owns NULL handling because Parquet omits NULLs + // from the physical value stream. + Status evaluate(const uint8_t* values, size_t num_values, size_t value_width, + uint8_t* matches) const { + if (UNLIKELY(value_width != this->value_width())) { + return Status::Corruption("PLAIN predicate width {} does not match expected {}", + value_width, this->value_width()); + } + switch (_type) { + case PlainFixedPredicateType::INT32: + return _evaluate<int32_t>(values, num_values, matches); + case PlainFixedPredicateType::INT64: + return _evaluate<int64_t>(values, num_values, matches); + case PlainFixedPredicateType::FLOAT: + return _evaluate<float>(values, num_values, matches); + case PlainFixedPredicateType::DOUBLE: + return _evaluate<double>(values, num_values, matches); + } + __builtin_unreachable(); + } + +private: + template <typename T> + Status _evaluate(const uint8_t* values, size_t num_values, uint8_t* matches) const { + const T literal = unaligned_load<T>(_literal.data()); + for (size_t row = 0; row < num_values; ++row) { + if (matches[row] == 0) { + continue; + } + const T value = unaligned_load<T>(values + row * sizeof(T)); + bool keep = false; + switch (_op) { + case PlainFixedPredicateOp::EQ: + keep = value == literal; + break; + case PlainFixedPredicateOp::NE: + keep = value != literal; + break; + case PlainFixedPredicateOp::LT: + keep = value < literal; + break; + case PlainFixedPredicateOp::LE: + keep = value <= literal; + break; + case PlainFixedPredicateOp::GT: Review Comment: Fixed in 2524c7836e8. The raw fixed-value Expr evaluator now uses Doris Compare for every operator, preserving FLOAT/DOUBLE NaN ordering and equality. Added direct-path coverage for NaN > finite and NaN == NaN; the private PlainFixedPredicate layer was removed. ########## be/src/format_v2/parquet/parquet_scan.cpp: ########## @@ -1130,22 +1649,61 @@ Status ParquetScanScheduler::read_filter_columns(int64_t batch_rows, update_counter_if_not_null(_scan_profile.rows_filtered_by_dict_filter, filtered_rows); if (new_selected_rows != selected_rows_before) { - // The dictionary reader has already appended only surviving values for the - // current column. Apply the compact row filter only to columns read before this - // one, then update the shared selection for later predicate/lazy columns. - RETURN_IF_ERROR(filter_read_predicate_columns(file_block, read_column_positions, - compact_filter)); + // The dictionary reader already appended only survivors for this column. Keep + // older predicate columns in their original coordinate spaces and compact all + // of them once at the expression/output boundary below. *selected_rows = apply_compact_filter_to_selection(compact_filter, selection, selected_rows_before); - *predicate_columns_filtered = true; } file_block->replace_by_position(block_position, std::move(column)); read_column_positions.push_back(cast_set<uint32_t>(block_position)); + remember_column_selection(cast_set<uint32_t>(block_position)); *used_dictionary_filter = true; return Status::OK(); } } + if (single_column_conjuncts != nullptr && + request.is_predicate_only(format::LocalColumnId(cast_set<int32_t>(local_id)))) { + auto predicates = compile_plain_fixed_predicates( + *single_column_conjuncts, file_block->get_by_position(block_position).type, + block_position); + if (predicates.has_value()) { + const uint16_t selected_rows_before = *selected_rows; + IColumn::Filter compact_filter; + bool used_filter = false; + RETURN_IF_ERROR(column_reader->select_with_plain_filter( + *selection, *selected_rows, batch_rows, *predicates, &compact_filter, + &used_filter)); + if (used_filter) { + DORIS_CHECK_EQ(compact_filter.size(), selected_rows_before); + update_counter_if_not_null(_scan_profile.plain_predicate_direct_batches, 1); + update_counter_if_not_null(_scan_profile.plain_predicate_direct_rows, + selected_rows_before); + const uint16_t new_selected_rows = count_selected_rows(compact_filter); + const auto filtered_rows = static_cast<int64_t>(selected_rows_before) - + static_cast<int64_t>(new_selected_rows); + if (conjunct_filtered_rows != nullptr) { + *conjunct_filtered_rows += filtered_rows; + } + if (new_selected_rows != selected_rows_before) { + *selected_rows = apply_compact_filter_to_selection( + compact_filter, selection, selected_rows_before); + } + // Predicate-only values are dead after this exact comparison. Preserve only a + // row-shaped placeholder so later block positions keep their stable identity. Review Comment: Fixed in 2524c7836e8. Direct no-materialization filtering is now disabled whenever a residual or delete conjunct still references the hidden slot, preserving its payload until those expressions finish. Added a single-column plus multi-column residual regression test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
