morningman commented on code in PR #69581: URL: https://github.com/apache/doris/pull/69581#discussion_r4430765577
########## be/src/format_v2/lance/lance_runtime_filter_helper.cpp: ########## @@ -0,0 +1,550 @@ +// 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. + +#include "format_v2/lance/lance_runtime_filter_helper.h" + +#include <arrow/type.h> +#include <cctz/time_zone.h> + +#include <algorithm> +#include <cctype> +#include <charconv> +#include <limits> +#include <optional> +#include <set> +#include <string_view> +#include <system_error> + +#include "common/logging.h" +#include "core/data_type/data_type_nullable.h" +#include "core/field.h" +#include "exprs/hybrid_set.h" +#include "exprs/runtime_filter_expr.h" +#include "exprs/vdirect_in_predicate.h" +#include "exprs/vexpr_context.h" +#include "exprs/vliteral.h" +#include "exprs/vslot_ref.h" +#include "format/format_common.h" +#include "runtime/runtime_profile.h" +#include "util/utf8_check.h" + +namespace doris::format::lance { +namespace { + +constexpr std::string_view LANCE_RUNTIME_FILTER_CACHE_KEY_PREFIX = "lance-runtime-filter-sql:"; + +std::string format_filter_ids(const std::vector<int>& filter_ids) { + std::string result; + for (const auto filter_id : filter_ids) { + if (!result.empty()) { + result.append(","); + } + result.append(std::to_string(filter_id)); + } + return result; +} + +std::string quote_sql_identifier(std::string_view identifier) { + // Lance SQL uses backticks for delimited identifiers. Escape an embedded backtick by doubling + // it, matching the SQL parser's quoted-identifier syntax. + std::string quoted("`"); + quoted.reserve(identifier.size() + 2); + for (const char ch : identifier) { + if (ch == '`') { + quoted.append("``"); + } else { + quoted.push_back(ch); + } + } + quoted.push_back('`'); + return quoted; +} + +const RuntimeFilterExpr* get_runtime_filter(const VExprContextSPtr& conjunct) { + if (conjunct == nullptr || conjunct->root() == nullptr) { + return nullptr; + } + return dynamic_cast<const RuntimeFilterExpr*>(conjunct->root().get()); +} + +std::string lowercase_ascii(std::string value) { + std::ranges::transform(value, value.begin(), [](const unsigned char ch) { + return static_cast<char>(std::tolower(ch)); + }); + return value; +} + +std::string quote_string_value(std::string_view value) { + std::string quoted("'"); + quoted.reserve(value.size() + 2); + for (const char ch : value) { + if (ch == '\'') { + quoted.append("''"); + } else { + quoted.push_back(ch); + } + } + quoted.push_back('\''); + return quoted; +} + +std::shared_ptr<arrow::DataType> find_unique_physical_type(const arrow::Schema& schema, + std::string_view column_name) { + const auto fields = schema.GetAllFieldsByName(column_name); + return fields.size() == 1 ? fields.front()->type() : nullptr; +} + +std::optional<int64_t> parse_int64(std::string_view value) { + int64_t parsed = 0; + const auto* begin = value.data(); + const auto* end = begin + value.size(); + const auto result = std::from_chars(begin, end, parsed); + if (result.ec != std::errc() || result.ptr != end) { + return std::nullopt; + } + return parsed; +} + +bool integer_literal_fits_decimal_precision(std::string_view value, int32_t precision) { + if (!value.empty() && (value.front() == '-' || value.front() == '+')) { + value.remove_prefix(1); + } + const auto first_nonzero = value.find_first_not_of('0'); + if (first_nonzero == std::string_view::npos) { + return true; + } + return value.size() - first_nonzero <= static_cast<size_t>(precision); +} + +// This checks Lance planner coercion, not whether stored Lance values fit their own Arrow type. +// Unsigned Arrow columns map to wider signed Doris types (for example UInt8 -> SMALLINT), so an RF +// produced from another input may contain a valid Doris value that cannot be coerced back to the +// narrower physical type. +bool integer_literal_fits_physical_type(std::string_view value, + const arrow::DataType& physical_type) { + const auto parsed = parse_int64(value); + if (!parsed.has_value()) { + return false; + } + const auto literal = *parsed; + switch (physical_type.id()) { + case arrow::Type::INT8: + return literal >= std::numeric_limits<int8_t>::min() && + literal <= std::numeric_limits<int8_t>::max(); + case arrow::Type::UINT8: + return literal >= 0 && literal <= std::numeric_limits<uint8_t>::max(); + case arrow::Type::INT16: + return literal >= std::numeric_limits<int16_t>::min() && + literal <= std::numeric_limits<int16_t>::max(); + case arrow::Type::UINT16: + return literal >= 0 && literal <= std::numeric_limits<uint16_t>::max(); + case arrow::Type::INT32: + return literal >= std::numeric_limits<int32_t>::min() && + literal <= std::numeric_limits<int32_t>::max(); + case arrow::Type::UINT32: + return literal >= 0 && literal <= std::numeric_limits<uint32_t>::max(); + case arrow::Type::INT64: + return true; Review Comment: Fixed in 3c278213385. The finding holds: `integer_literal_fits_physical_type()` admitted every i64 for an Int64 column, and the scale-zero decimal check admitted every value that parses as an i64, so a runtime filter on INT64_MIN reached Lance as `-9223372036854775808`. The pinned parser negates a magnitude that it first parses as i64; 9223372036854775808 overflows into a Float64, which the scalar coercion cannot turn into Int64 or Decimal, so the first `lance_scanner_next()` failed planning. IN, `>=` and `<=` all reached this through `to_lance_sql_literal()`. `parse_lance_int64_literal()` now parses an integer literal as the pinned parser does and rejects INT64_MIN, and both checks use it. A filter that mentions INT64_MIN stays residual-only and is reported under `LanceRuntimeFilterSkippedIds`; an IN set with that member is rejected as a unit, as for any member Lance cannot take. INT64_MIN + 1 and every other i64 are still pushed. Tests: - `LanceRuntimeFilterHelperTest.KeepsInt64MinFiltersResidualOnly`: ordinary (not null-aware) IN with an INT64_MIN member, `>=` and `<=` on INT64_MIN, and a DECIMAL(38, 0) IN on INT64_MIN are skipped; the same filters on INT64_MIN + 1 are pushed with the expected SQL. - `LanceTableReaderFilterTest.KeepsInt64MinRuntimeFiltersOutOfLanceScanner`: a Lance dataset holding INT64_MIN is read under an INT64 IN, an INT64 `<=` and a DECIMAL(38, 0) IN on INT64_MIN. The scan succeeds, reports all three as skipped, and returns the matching row. `run-be-ut.sh --run --filter='Lance*'` (ASAN, macOS arm64): all 106 Lance cases pass. Restoring `parse_int64()` in either check makes exactly these two tests fail; the reader case then fails with Lance's own planning error ("Received literal Float64(-9223372036854776000) and could not convert to literal of type ..."), on the INT64 column for the integer check and on the DECIMAL column for the decimal one. -- 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]
