[fix](be) Backport selected query engine fixes to branch-4.1#64537
Open
Mryange wants to merge 10 commits into
Open
[fix](be) Backport selected query engine fixes to branch-4.1#64537Mryange wants to merge 10 commits into
Mryange wants to merge 10 commits into
Conversation
…nView (apache#63076) What problem does this PR solve? Issue Number: N/A Problem Summary: The uniform function takes three arguments: min, max, and seed. Only the first two (min, max) are truly "always constant" — the seed column should be treated as a regular column, not a constant. Without overriding get_arguments_that_are_always_constant(), when a user passes a constant value as the third argument (seed), the default framework logic treats it as a constant column, leading to incorrect results. Root cause: the base class default get_arguments_that_are_always_constant() does not distinguish between the seed argument and the min/max arguments, so a constant seed would be folded by the constant-handling path rather than being treated as a per-row value. Fix: - Override get_arguments_that_are_always_constant() to return {0, 1}, explicitly marking only min and max as always-constant arguments. - Refactor seed column access to use ColumnView<TYPE_BIGINT> for safer and more idiomatic typed column iteration. (cherry picked from commit a70c212)
…he#63810) The mixed const execution probe exposed several constant-handling problems in BE vectorized functions. - ColumnConst::clone_resized reused the original nested column, so cloned const columns could still alias the source data. - quantile_percent requires its percentile argument to stay constant, but the all-const probe path unpacked it and triggered a false constant-check failure. - regexp_count accessed string columns directly and did not handle mixed const inputs correctly. - uniform still went through the default constant implementation even though its result depends on per-row seed values. This change fixes those behaviors and adds focused unit tests for the uncovered cases. (cherry picked from commit 905c804)
…ache#63174) ### What problem does this PR solve? Issue Number: N/A Problem Summary: Aggregate batch deserialization creates aggregate states with placement new before deserializing or merging serialized input. If deserialization or merge throws after `create()` succeeds, the previous cleanup only destroyed states from earlier rows and skipped the current already-created state. This can leak resources owned by aggregate state objects, such as hash sets or bitmap internals. Root cause: the exception cleanup destroyed only states from previous rows. If the current row's state was created successfully and deserialization failed afterward, that current state was excluded from cleanup. This PR tracks the number of successfully created aggregate states and destroys exactly that range on exception. It preserves the successful-path ownership model: `deserialize_vec()` leaves created states to its caller, while merge helpers still release temporary rhs states with `destroy_vec()` after successful merge. This PR also switches aggregate-local `phmap::flat_hash_map` and `phmap::flat_hash_set` usages to Doris wrapper aliases so they use Doris' default equality and allocator definitions consistently. (cherry picked from commit 4483daf)
Fix collect_list/group_array on nested TIMESTAMPTZ values when complex aggregate state is serialized through JSON. This keeps the existing state format for compatibility, provides a UTC timezone during serde, and adds regression coverage for the nested group_array case. (cherry picked from commit 8731600)
…ted wrappers for thread safety analysis (apache#63109) After apache#63070 replaced `std::mutex`/`std::lock_guard` with annotated wrappers (`AnnotatedMutex`/`LockGuard`), shared mutex usage still relied on raw `std::shared_mutex` and `std::shared_lock`/`std::unique_lock`, which are invisible to Clang's thread safety analysis. This leaves shared-lock sites unverified — `GUARDED_BY`, `REQUIRES_SHARED`, and other annotations cannot be enforced. - Add `AnnotatedSharedMutex` (wrapping `std::shared_mutex` with `CAPABILITY`/`ACQUIRE`/`RELEASE`/`ACQUIRE_SHARED`/`RELEASE_SHARED` annotations) and `SharedLockGuard` (RAII `SCOPED_CAPABILITY` with `ACQUIRE_SHARED`/`RELEASE`) in `thread_safety_annotations.h`. - Migrate `VDataStreamMgr` and `RuntimeFilterMergeControllerEntity` from `std::shared_mutex` to `AnnotatedSharedMutex`, and from `std::unique_lock`/`std::shared_lock` to `LockGuard`/`SharedLockGuard`. - Add `GUARDED_BY` annotations to the protected maps. - Extract `_find_recvr` as a private helper annotated with `REQUIRES_SHARED(_lock)`, eliminating the `bool acquire_lock` parameter that previously bypassed lock tracking. (cherry picked from commit ab1a4dd)
### What problem does this PR solve?
Issue Number: N/A
Related PR: None
Problem Summary:
When dry_run_query is enabled, FE only needs the returned row count, but
BE still spends most of PhysicalResultSink time serializing MySQL result
rows In a local dry-run case against numbers("number"="1000000"), the
profile showed AppendBatchTime = 77.689ms, TupleConvertTime = 68.650ms,
and ResultSendTime = 2.702us, which means the dry-run path was still
paying almost the full result sink conversion cost.
This change keeps output expr evaluation intact, but returns early in
the MySQL result writers once the output block is produced in dry-run
mode. That preserves returned row accounting while skipping result
serialization, block copy, and sink enqueue work that dry-run queries
never consume.
(cherry picked from commit 4938d63)
Problem Summary: `MergeSorterState` used the generic copy-based merge path even when the current top sorted run could return its whole remaining block before any other run. This adds a direct whole-block fast path guarded by a total-order check, avoiding unnecessary `insert_range_from` work in inner merge. ### What is changed? - Add `MergeSortCursor::totally_less_or_equals()` to detect when the current run is wholly before the next child. - Return the current block directly from `MergeSorterState::_merge_sort_read_impl()` when the whole-block condition is satisfied. - Add focused BE unit tests for exact-batch and smaller-than-batch whole-block fast-path cases. (cherry picked from commit 974f9bd)
… is map key or value (apache#63124) ### What problem does this PR solve? Issue Number: N/A Problem Summary: `map_contains_entry` throws a `RUNTIME_ERROR` at BE execution time when the MAP column has `TIMESTAMPTZ` as its key or value type. Root cause: `FunctionMapContainsEntry::is_equality_comparison_supported` hard-coded a list of accepted primitive types (`is_date_type`, `is_time_type`, `is_number`, `is_string_type`, `is_ip`) but omitted `TYPE_TIMESTAMPTZ`. As a result, the pre-execution type guard always rejected TIMESTAMPTZ even though the underlying `dispatch_switch_all` + `ColumnVector::compare_at` path supports it correctly. The fix replaces the hand-maintained list with a direct call to `dispatch_switch_all`, which already covers TIMESTAMPTZ in its `DATETIME` branch, making the guard consistent with the actual dispatch layer. (cherry picked from commit 60b0d46)
ANN query vector extraction returned a generic `IColumn::Ptr`, so the TopN and range search paths had to downcast the column again before reading float data. This made the code more indirect and delayed type validation. This PR changes the helper and runtime state to keep the query vector as `ColumnFloat32::Ptr`, validates the concrete type at extraction time, and removes redundant casts from the ANN execution path. (cherry picked from commit 99691f6)
…63365) ### What problem does this PR solve? Nereids rejects `TIMESTAMPDIFF(MICROSECOND, ...)` during analysis. The executable path already exists through `MicroSecondsDiff`, but the FE binder cannot reach it because: - `Interval.TimeUnit` does not define a standalone `MICROSECOND` - `DatetimeFunctionBinder` only routes `TIMESTAMPDIFF` up to `SECOND` As a result, queries such as `TIMESTAMPDIFF(MICROSECOND, MIN(t), MAX(t))` fail even for valid `DATETIMEV2(6)` inputs. (cherry picked from commit 4ab7cc0)
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related PR:
Problem Summary:
This PR backports a clean batch of branch-4.1-compatible fixes and refinements from master. The picked changes cover constant argument handling in scalar functions, mixed const-column regressions, aggregate state cleanup, TIMESTAMPTZ group_array serde, annotated mutex wrappers, dry-run result serialization, sort block copying, TIMESTAMPTZ map_contains_entry, typed ANN query vectors, and TIMESTAMPDIFF MICROSECOND support in Nereids.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)