## What problem does this PR solve?
An async dictionary load task writes the INC version journal, then DROP
deletes the dictionary, then a failed BE commit writes a DEC journal for
the already dropped dictionary. Followers exit while replaying the DEC
journal because the dictionary cannot be found by name anymore, and the
cluster cannot recover since the bad journal is persistent.
Root cause: the commit failure rollback runs outside the manager lock
and does not check whether the dictionary is still current, so DROP can
interleave between the INC journal and the rollback DEC journal. Replay
of the DEC journal looks the dictionary up by name and throws when it is
gone, which makes EditLog.loadJournal exit the FE.
Fix: on commit failure, only persist the DEC journal while the
dictionary is still the current one (identity check under the manager
read lock), so the journal stream becomes CREATE -> INC -> DROP instead
of CREATE -> INC -> DROP -> DEC. Replay of increase/decrease version
journals is made idempotent and keyed by dictionary id, so a DEC journal
of an already dropped or recreated dictionary is a no-op instead of a
fatal error.
## Release note
None
## Check List (For Author)
- Test: Unit test DictionaryManagerTest (replay idempotency, journal
order CREATE -> INC -> DROP -> DEC, ABA recreated same-name dictionary)
and docker regression test test_dictionary_drop_while_load_commit_fail
(deterministic race reproduction via debug points).
- Behavior changed: No
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Cloud table ALTER accepts `partition.retention_count`
during analysis, but the cloud schema-change handler did not dispatch
the property. Valid ALTER statements therefore failed as `invalid
properties` before table-property persistence and retention scheduler
registration. The fix treats the property as an FE-only metadata update,
while shared validation rejects it atomically when dynamic partitioning
is enabled so the two scheduler modes cannot coexist.
### Release note
Cloud AUTO RANGE tables can update `partition.retention_count` through
`ALTER TABLE` when dynamic partitioning is not enabled.
- Behavior changed:
- [x] Yes. Valid cloud-mode retention ALTER succeeds, while
retention_count is rejected when dynamic partitioning is enabled.
A dictionary refresh can retain a `Dictionary` object while `DROP
DATABASE` removes its owner database. The old path repeatedly recovered
the database through `dictionary.getDatabase()` and checked existence by
database/name, so it could either dereference a null database or accept
a newly created dictionary with the same name.
This change establishes one insert-target invariant: resolve the
database and table/dictionary together, then carry that exact pair
through authorization, sink binding, retry validation, and executor
construction. Dictionary loads additionally validate canonical
dictionary identity by ID before planning and after execution. Database
drop now removes dictionary IDs from the canonical map, matching replay
behavior and preventing same-name ABA.
Ordinary internal-table inserts keep their existing table
read-lock/write-lock and transaction ordering. The paired target only
closes the pre-lock metadata lookup window and adds database ID to retry
validation.
Problem Summary: Auto partition creation can race with dynamic partition
retention. After new partitions are added, the response builder reads
the table partition map and partition info without holding the table
metadata lock. If a retention drop removes one of those partitions in
the same window, the response builder can observe a missing partition or
partition item and hit a null pointer while constructing partition and
tablet metadata. This change snapshots the required partition metadata
under the table read lock, returns a normal error when the partition was
already dropped before the snapshot, and keeps tablet
location/cache/backend resolution work outside the table lock.
The regression case can overlap with a dynamic-partition scheduler run
that was started while the global interval was one second. Restoring the
interval to 600 seconds does not cancel that in-flight run. If it drops
the out-of-range `p1900` partition while the multi-row INSERT is
committing, the INSERT can fail even though the scheduler reached its
intended final state.
The prior workaround depended on an exact exception message and issued
`show partitions` inside a `TestAction` check closure. That nested DSL
call changes the action current SQL, making the failure report and its
string-based classification brittle.
This change drains the likely preceding short-interval run before
INSERT, captures the INSERT exception without nested DSL calls, and
tolerates only the known terminal state: `p1900` is absent while the
`2024`, `2900`, and `3000` auto-created partitions remain. All other
failures still fail the case. The intentional one-second cleanup
interval is restored to 600 seconds in `finally`.
An empty or too-short libunwind stack trace underflows the
remaining-frame count in `StackTrace::toString()` after it skips its
internal frames. Formatting an error `Status` then reads beyond the
fixed frame array and can crash the BE.
Treat non-positive stack-capture results as empty, render a trace with
fewer frames than the requested skip as the existing `<Empty trace>`,
and preserve signal-context offsets after slicing. A debug-point BEUT
injects an empty trace into the real `Status::NotFound` capture/logging
path.
### What problem does this PR solve?
Issue Number: None
Related PR: #65178
The review runner previously obtained the aggregate pull-request diff
through the GitHub API. GitHub rejects aggregate diffs larger than
20,000 lines, causing large pull requests to fail before Codex could
review them. The runner now checks out complete history and locally
generates the changed-file list and the verified three-dot diff from the
live base and head SHAs. The review prompt also states those diff
semantics, and context-preparation failures are reported separately from
Codex failures.
Each run now randomly selects
`oss://doris-community-ci/codex/auth.json.1` or
`oss://doris-community-ci/codex/auth.json.2`, downloads it locally as
`auth.json`, and syncs refreshed credentials back to that same selected
object.
The Codex GitHub Actions review pipeline needed stronger guarantees that
agents review the intended PR snapshot, use one authoritative diff,
pursue high-risk cross-module mechanisms, and expose the work of spawned
subagents in Litefuse.
This PR:
- verifies the checked-out PR head and rechecks the live base/head after
context preparation;
- prepares an authoritative aggregate PR patch and changed-file list,
with atomic bounded retry (`2s`, `4s`, `8s`) for transient GitHub API
failures;
- directs the main agent to perform an initial risk scan, dispatch
focused subagents for suspicious mechanisms, maintain a shared
append-only ledger, validate/deduplicate every candidate, and converge
for at most three rounds;
- requires paths outside the authoritative diff/changed-file context to
be confirmed with `rg --files` before reading;
- records separate Litefuse traces for each Codex subagent session,
including command/tool I/O and non-empty encrypted-reasoning status,
instead of mixing subagent activity into the main trace;
- makes Litefuse ingestion more resilient to transport timeouts while
keeping telemetry upload non-blocking for the review outcome.
The repository review skill is kept focused on review invariants;
orchestration and completion policy live in the workflow prompt, where
they are available to the GitHub Actions invocation.
### Validation and experiment material
- Static checks: `python3 -m py_compile
.github/scripts/emit_litefuse_otel_io.py` and `git diff --check`.
- Litefuse uploader validation: real/synthetic subagent session
conversion, payload-limit coverage, and readback of independent subagent
traces.
- End-to-end replayed review experiments against fresh fork PRs built
from the original PR base/head commits:
| Replay PR | GitHub Actions run | Result |
| --- | --- | --- |
| [#136](https://github.com/zclllyybb/doris/pull/136) |
[29112823062](https://github.com/zclllyybb/doris/actions/runs/29112823062)
| Success; 2 inline comments |
| [#137](https://github.com/zclllyybb/doris/pull/137) |
[29114260631](https://github.com/zclllyybb/doris/actions/runs/29114260631)
| Success; 3 inline comments |
| [#138](https://github.com/zclllyybb/doris/pull/138) |
[29115946034](https://github.com/zclllyybb/doris/actions/runs/29115946034)
| Success; 1 inline comment |
| [#139](https://github.com/zclllyybb/doris/pull/139) |
[29150372758](https://github.com/zclllyybb/doris/actions/runs/29150372758)
| Success; 1 inline comment |
| [#140](https://github.com/zclllyybb/doris/pull/140) |
[29151057520](https://github.com/zclllyybb/doris/actions/runs/29151057520)
| Success; 1 inline comment |
The latest replay trace, `86be226391a40e5992bb8d3b4e125eca`, confirms
one authoritative `pr.diff` read, two changed-file-list reads, no `gh pr
diff` or `git diff` calls from the reviewing agent, and separate
subagent sessions. It also exposed one remaining prompt-compliance gap:
a guessed adjacent `.cpp` path was read without a preceding `rg --files`
lookup; this is documented test evidence rather than hidden by the
validation.
Remove stale CODEOWNERS entries that only assign ownership to
`zclllyybb` and `BiteTheDDDDt`, so those accounts are no longer
requested as code owners for the affected paths.
### What problem does this PR solve?
Problem Summary: I modified the codex to support the `codex exec --goal`
mode. The Codex review workflow now enters goal mode through the
runner's --goal invocation, so the review instructions should describe
the completion criteria and convergence process instead of telling the
model to create the goal itself.
see modification in
https://github.com/zclllyybb/codex/tree/codex-exec-goal
Verification:
- Before:
https://github.com/apache/doris/pull/63690 got 1 comment when `/review`
- Now:
https://github.com/zclllyybb/doris/pull/49 replicated the same PR,
receiving 8 comments in a single `/review`
This PR adds a BE-side diagnostic HTTP API for collecting thread stack
traces from a live BE process:
```text
GET /api/stack_trace
GET /api/stack_trace?thread_id=<tid>[,<tid>...]
GET /api/stack_trace?mode=<DISABLED|FAST|FULL|FULL_WITH_INLINE>
GET /api/stack_trace?dwarf_location_info_mode=<DISABLED|FAST|FULL|FULL_WITH_INLINE>
GET /api/stack_trace?timeout_ms=<ms>
GET /api/stack_trace?skip_blocking_syscalls=<true|false>
```
`thread_id` is the preferred explicit selector. `tid` is kept only as a
legacy alias.
The default full-process collection samples all BE threads without a
signal-attempt cap and does not skip blocking syscalls. This is
intentional: during real incidents, blocked worker stacks are often the
useful part of the dump. `skip_blocking_syscalls=true` remains available
as an explicit conservative mode for isolating EINTR-sensitive paths.
The output is plain text and includes:
- BE pid, diagnostic signal number, thread count, timeout, DWARF mode,
and syscall-skip mode.
- One section per thread, with TID, thread name, capture status, capture
method, frame count, frame-pointer status, and stack bounds.
- A summary line with captured, skipped, timed-out, signal-error, and
exited counts.
Example header:
```text
BE thread stack traces
pid: 96543
service_signal: 40
thread_count: 1168
timeout_ms_per_thread: 3000
dwarf_location_info_mode: fast
skip_blocking_syscalls: false
signal_handler_unwinder: frame_pointer_with_coordinator_signal_context_libunwind_fallback
```
Example execution stack captured under load:
```text
----- thread 12438 (p_normal_simple) status=ok capture_method=frame_pointer frames=32 fp_status=end_of_chain stack_bounds=... -----
doris::CastToStringFunction::execute_impl(...)
doris::VectorizedFnCall::execute_column_impl(...)
doris::AggSinkLocalState::_execute_without_key(...)
doris::AggSinkOperatorX::sink_impl(...)
doris::DataSinkOperatorXBase::sink(...)
doris::PipelineTask::execute(bool*)
doris::TaskScheduler::_do_work(int)
```
Implementation notes:
- The remote-thread capture path uses a signal handler to copy only
lightweight frame-pointer/context data from the target thread.
Symbolization and fallback unwinding run outside the handler.
- The handler publishes through one process-wide slot, so HTTP
collection is serialized. The coordinator waits until the previous
handler has fully released its latch before signaling the next TID;
otherwise back-to-back captures can drop the next signal.
- Stack bounds are collected before sending any signal, because the
handler must not open `/proc`, allocate memory, or take locks while
interrupting an arbitrary BE thread.
- Frame-pointer walking validates stack-range bounds and pointer
alignment before reading frame records.
- The same PCs can be rendered with different DWARF modes, so the
StackTrace cache key includes `dwarf_location_info_mode`. This prevents
a `DISABLED` rendering from poisoning a later `FULL` request and
prevents full file/line detail from leaking into a disabled request.
- Instruction addresses are compared via `uintptr_t` instead of pointer
arithmetic on non-object instruction addresses.
- Threads that block the diagnostic signal are reported as
`status=signal_blocked` instead of being treated as ambiguous timeouts.
- This is not a global stop-the-world snapshot. Each thread is sampled
independently as the coordinator walks the TID list, so stacks are close
in time but not guaranteed to be from the exact same instant.
EINTR handling:
- Sending a diagnostic signal can interrupt blocking syscalls even when
`SA_RESTART` is used, especially for poll/select/accept-style waits.
- Doris Thrift servers previously could treat `Interrupted system call`
from `TServerSocket::acceptImpl()` as a fatal server-loop exception.
- This PR wraps BE Thrift server sockets with `ImprovedServerSocket`,
which retries only the narrow EINTR/`Interrupted system call` case for
accept/read/peek. Other transport exceptions keep the original behavior.
Problem Summary: Automated code review runs relied on the reviewer to
infer which module-specific AGENTS.md files apply to a pull request.
Recent Litefuse traces showed that reviews often skipped ancestor guides
such as the repository root AGENTS.md, fe/AGENTS.md, and
be/test/AGENTS.md even when the changed files made them applicable. This
change fetches the PR changed file list, derives every existing
AGENTS.md file from the changed file ancestor directories, records the
list in the review context, and injects the required guide paths
directly into the first Codex review prompt.
We tested 5 PRs that did not read the AGENTS.md file during previous
pipeline runs, and the new results are as follows:
| Origin PR | Test PR | Run | Litefuse Trace | Result |
|---|---:|---:|---|---|
| apache/doris#64478 | zclllyybb/doris#36 | `27536249602` |
`d76b77ac4e0d52d96f413b154c9f2571` | Read All |
| apache/doris#64458 | zclllyybb/doris#37 | `27536258049` |
`8db34388a8379acdcd9e48720f11225f` | Read All |
| apache/doris#64392 | zclllyybb/doris#38 | `27536266784` |
`a5e37fba03375a9517f7a67e33f70d39` | Read All |
| apache/doris#64489 | zclllyybb/doris#39 | `27536275680` |
`22ace18d70ece96b0ca7fa73123b62da` | Read All |
| apache/doris#64419 | zclllyybb/doris#41 | `27538239601` |
`1803c040e4140d61e840e4e1526190e7` | Read All |
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: `IColumn::is_nullable()` only identified physical
`ColumnNullable`, while callers also need nullable semantics for
`ColumnConst(ColumnNullable)`. The old split between semantic nullable
checks and physical nullable checks made call sites ambiguous. This
change removes `is_concrete_nullable()`, makes `ColumnConst` report
nullable when its nested data column is nullable, and migrates
physical-column branches to `is_column_nullable()` or
`check_and_get_column<ColumnNullable>()`. Const nullable join keys and
row-copy paths are materialized or unwrapped explicitly so null maps are
propagated correctly.
Remove the obsolete bitmap runtime filter planning and execution path
while preserving thrift compatibility entries as Deprecated.
Root cause:
BITMAP_FILTER still had a dedicated FE planning branch for
bitmap-containing nested-loop join predicates and matching BE
runtime/filter predicate implementations. Even though the path is no
longer intended to be planned, helper objects and runtime mappings kept
it reachable.
Fix:
- Keep existing thrift ids and fields for BITMAP_PRED,
TRuntimeFilterType.BITMAP, bitmap_target_expr, bitmap_filter_not_in, and
is_output_left_side_only, but mark them Deprecated.
- Remove FE bitmap runtime-filter generation, pushdown, translation,
session-variable acceptance, and nested-loop join probe-only special
handling.
- Delete VBitmapPredicate, BitmapFilterFunc, and bitmap column predicate
execution objects.
- Adjust runtime-filter tests and regressions to verify bitmap runtime
filters are not planned.
## Proposed changes
Follow-up to the merged Litefuse payload-size fix in #64328.
This patch keeps the previous Litefuse 413/pagination/trace-shape fixes
and adds the latest CI hardening:
- Renames the automated review workflows from `opencode-*` to
`code-review-*`, and removes the remaining `opencode` wording from
workflow-local paths.
- Removes the temporary gh-only / MCP-forbid prompt restrictions that
were added while diagnosing the GitHub MCP failure path. The workflow
still verifies the real outcome by reading GitHub Reviews for the
current head SHA after Codex exits.
- Enables Codex memories in CI and syncs `$CODEX_HOME/memories` with
`oss://doris-community-ci/codex/memories.tar.gz` before and after
review. Memory sync is best-effort and does not block review completion.
- Keeps Litefuse recording best-effort so a successful code review is
not failed only because trace upload or verification fails.
The original payload-limit fixes are still included:
- removes the workflow's 4KB context override;
- makes `--max-context-json-chars <= 0` inherit `--max-json-chars`;
- paginates legacy Litefuse observation readback during verification;
- adaptively retries Litefuse 413 responses without creating extra
traces;
- preserves one trace per run and original observation ordering across
ingestion chunks.
## Commits
- `500ebc6319b` `[chore](ci) Rename code review workflows`
- `57a0dd223e3` `[fix](ci) Relax code review submission tooling`
- `746e46d0bbb` `[feat](ci) Sync Codex memories for reviews`
## Validation
Fork workflow validation:
- Pushed the same workflow head to `origin/master` first.
- Opened temporary fork PR https://github.com/zclllyybb/doris/pull/33
from `codex-review-validation-20260610-memory-rename` and triggered
`/review`.
- Code review run succeeded:
https://github.com/zclllyybb/doris/actions/runs/27249379410/job/80470334233
- GitHub review was submitted by `github-actions` on commit
`b14d07483c2d4e2c3dd00a9e8da00e52a9fff4ac`.
- Litefuse trace `0a88ea57df463c2f975fc0081a612f02` was uploaded and
verified with trace input/output present, all agent-message context
windows present, 60 events, 35 observations, no 413 retries, and one
ingestion request under the configured payload cap.
- First memory download saw the expected initial `NoSuchKey` for
`codex/memories.tar.gz` and continued without failing the review; memory
upload then succeeded with a 7,735-byte archive.
- Temporary validation PR and branch were closed/deleted after
validation.
Earlier payload-limit validation retained from the previous iteration:
- Unit-style chunking check: splitting into multiple chunks preserved
event bodies, event order, and trace id.
- Simulated Litefuse 413 on multi-event batches: uploader split/retried,
preserved event order, and uploaded all 16 events successfully.
- Simulated Litefuse 413 on one oversized observation: uploader shrank
the single observation and retried successfully.
- Synthetic dry-run with 267 Codex events and default context
truncation: split into 24 requests, max request size 395324 bytes under
a 400000-byte test cap.
- Synthetic dry-run with a single huge observation and default context
truncation: max request size 341060 bytes under a 400000-byte test cap.
- Verified the failed trace from
https://github.com/apache/doris/actions/runs/27213447314/job/80351165150
using paginated legacy readback: trace
`45202ab41ddbfd099877fa7be5cc1256` has 105 observations across pages, 21
agent messages, one turn input, and zero observations missing I/O.
- Diagnosed
https://github.com/apache/doris/actions/runs/27213705642/job/80352669355
as a live Litefuse 413 from the old master helper
`f48efb28db486837db5375e6fd0bcfc95ea67a05` despite `--max-payload-bytes
4000000`.
## Proposed changes
The Codex review workflow can produce large JSONL traces. Sending every
Litefuse observation in a single legacy ingestion request can exceed
Langfuse Cloud's per-request payload limit and fail with HTTP 413.
This patch:
- splits Litefuse ingestion into size-bounded requests;
- uses compact JSON encoding for ingestion payloads;
- adds an explicit smaller cap for agent-message context event details;
- shrinks any oversized single ingestion event before posting;
- passes explicit 4MB payload and 4KB context caps from the workflow.
GitHub hosted runners can reject the bubblewrap uid-map setup used by
Codex workspace-write mode, which prevents even local gh commands from
running inside the automated review step.
Switch the Codex invocation to danger-full-access for the ephemeral
runner, keep xhigh reasoning and the Codex OSS auth sync path intact,
and make the prompt require GH_TOKEN-backed gh/gh api review submission
instead of GitHub MCP connector write tools.
After Codex exits, reject runs that attempted github_add_review_to_pr
and verify through the GitHub Reviews API that a new review was actually
submitted for the current head SHA, so a completed Codex turn cannot
silently pass without posting a review.
Run the AI review workflow through Codex, persist the review
prompt/output even when review fails, and emit Codex JSONL steps to
Litefuse using the public ingestion API.
The runner now uses a Codex-native auth.json stored at
oss://doris-community-ci/codex/auth.json. It downloads that file
directly into CODEX_HOME and syncs the refreshed Codex auth.json back to
the same object, without converting to or from the old opencode auth
format.
Codex execution uses workspace-write sandboxing with
approval_policy=never instead of dangerous bypass mode. The workflow
does not add any writable directory outside the checked-out repository,
while keeping network access available for GitHub review operations.
The Litefuse emitter creates a trace, root review span, turn generation,
and per-step observations with non-empty input/output. Agent messages
use the event window since the previous agent message as input,
preserving command/tool/reasoning context as a structured list.
Readback verification uses Litefuse public observation APIs to assert
trace I/O, step observation counts, missing I/O, and agent-message
context structure without logging prompt text or command output.
Problem Summary: Runtime profiles had a CPU-time counter named
MaxFindRecvrTime(NS), while the naming convention for CPU-time counters
is to include CpuTime directly in the counter name. This makes it hard
to distinguish CPU-time counters from wall-time counters by name. Rename
the exchange receiver counter to MaxFindRecvrCpuTime(NS), keep existing
TaskCpuTime and ScannerCpuTime names without an extra suffix, and cover
the expected profile names in BE unit tests and the existing scanner
profile regression test.
Problem Summary: Some BE operator paths could keep doing unnecessary
work after query cancellation had already been observed by RuntimeState.
This covers three focused cases:
- Spill revoke and recovery paths could still enter spill or repartition
work after cancellation.
- Nested loop join build close could still process cross runtime filters
after cancellation, scanning build blocks and evaluating filter
expressions.
- Analytic sink could advance multiple buffered output blocks in one
sink call without checking cancellation between blocks.
This PR adds cancellation checks at the retained public entry points or
block boundaries. The analytic sink change is intentionally
conservative: it stops before advancing to another buffered output
block, while preserving current-block processing semantics.
Count aggregation without GROUP BY reaches
AggFnEvaluator::execute_single_add(), which calls
add_batch_single_place(). AggregateFunctionCount and
AggregateFunctionCountNotNullUnary previously inherited the row-by-row
helper there, so count(*) and count(nullable_expr) paid per-row
add/is_null_at costs even when all rows were aggregated into one state.
This patch adds batch implementations: count(*) increments the state
once by batch_size, while unary count(nullable_expr) checks the nullable
null map once and fast-paths the no-NULL case to count += batch_size.
When NULLs exist it uses simd::count_zero_num() over the null map to
count non-NULL rows. The nullable class name is kept because SQL
count(expr) counts non-NULL values, not NULL values.
Performance:
test with sql
```sql
select count(nullable(number)) from numbers("number"="1000000000");
select count(nullable(if(number >= 0, null, number))) from numbers("number"="1000000000");
select count(nullable(if(number % 2 = 0, number, null))) from numbers("number"="1000000000");
```
get result
```
Scenario before median / mean after median / mean median diff
━━━━━━━━━━━ ━━━━━━━━━━━━━━━━━━━━━━ ━━━━━━━━━━━━━━━━━━━━━ ━━━━━━━━━━━━━
non NULL 645 / 648.6 ms 555 / 556.4 ms -14.0%
─────────── ────────────────────── ───────────────────── ─────────────
all NULL 1541 / 1539.6 ms 1448 / 1450.6 ms -6.0%
─────────── ────────────────────── ───────────────────── ─────────────
half NULL 4256 / 4261.2 ms 4192 / 4232.2 ms -1.5%
```
Root cause: clang lowers Doris floating mod/fmod to libm std::fmod for
every non-null row. On the arithmetic operator benchmark distribution,
especially large lhs with sub-unit or small rhs and float inputs
promoted to double, that libm path dominates expression execution while
SR/GCC-like code uses x87 fprem plus cheap finite and magnitude fast
paths.
Fix: add fmod_fast helpers for double/float on x86_64 that inline the
x87 fprem sequence, keep std::fmod fallback for zero divisor and
non-finite inputs, and preserve Doris null semantics by still writing
the null map for vector-vector and constant/vector shapes. Wire only
floating mod/fmod paths to the helpers; integer, decimal, and pmod
behavior stay unchanged.
```
case baseline mean patched mean speedup
━━━━━━━━━━━━━━━━━━━━ ━━━━━━━━━━━━━━━ ━━━━━━━━━━━━━━ ━━━━━━━━━
Double_DB_DB 91.2 ms 76.1 ms 1.20x
──────────────────── ─────────────── ────────────── ─────────
Double_IN_ONE_DB 102.2 ms 87.1 ms 1.17x
──────────────────── ─────────────── ────────────── ─────────
Double_DB_IN_ONE 473.7 ms 120.7 ms 3.92x
──────────────────── ─────────────── ────────────── ─────────
Double_DB_IN_TEN 205.4 ms 108.5 ms 1.89x
──────────────────── ─────────────── ────────────── ─────────
Double_MIXED_ZERO 302.1 ms 141.7 ms 2.13x
──────────────────── ─────────────── ────────────── ─────────
Double_VectorConst 413.6 ms 102.5 ms 4.03x
──────────────────── ─────────────── ────────────── ─────────
Double_ConstVector 80.0 ms 79.1 ms 1.01x
──────────────────── ─────────────── ────────────── ─────────
Double_Nullable 488.4 ms 153.2 ms 3.19x
──────────────────── ─────────────── ────────────── ─────────
Float_DB_DB 75.2 ms 70.7 ms 1.06x
──────────────────── ─────────────── ────────────── ─────────
Float_DB_IN_ONE 525.7 ms 114.7 ms 4.58x
──────────────────── ─────────────── ────────────── ─────────
Float_VectorConst 414.2 ms 106.4 ms 3.89x
```
Problem Summary: Add the
[apache/doris-skills](https://github.com/apache/doris-skills) repository
as a root-level submodule and configure it to track the main branch so
maintainers can refresh it with git submodule update --remote
doris-skills.
Add script to extract Doris profile operator/counter names and update
AGENTS.md
- Introduced a new Python script `extract_source_profile_inventory.py`
that extracts operator and counter names from the Doris source tree,
generating a markdown inventory.
- Updated AGENTS.md to include a requirement for formatting BE code with
the correct skill before committing if modifications are made.
Related PR: #60386
Problem Summary: MultiCastDataStreamer already shares pulled blocks
through the column copy-on-write contract. The previous pull path still
cloned every column for each consumer through _copy_block(), which added
unnecessary per-consumer allocation and copy work. The copy-on-write
assertion changes and regression coverage from the original attempt are
already present on current master, so this change keeps the pull
completion accounting in _finish_pull() and returns the shared block
directly.
test performance with:
```sql
SET enable_profile = true;
SET profile_level = 2;
SET inline_cte_referenced_threshold = 0;
SET parallel_pipeline_task_num = 16;
WITH base AS (
SELECT id, k1, ..., k8, v01, ..., v24, s1, s2, s3, s4
FROM bench_cte_multicast_wide
WHERE id >= 0
)
SELECT 0 AS branch, COUNT(*),
SUM(id + k1 + ... + k8 + v01 + ... + v24),
SUM(LENGTH(s1) + LENGTH(s2) + LENGTH(s3) + LENGTH(s4))
FROM base WHERE k1 % 16 = 0
UNION ALL
...
UNION ALL
SELECT 15 AS branch, COUNT(*),
SUM(id + k1 + ... + k8 + v01 + ... + v24),
SUM(LENGTH(s1) + LENGTH(s2) + LENGTH(s3) + LENGTH(s4))
FROM base WHERE k1 % 16 = 15
ORDER BY branch;
```
result:
| version | min | median | avg | max |
|---|---:|---:|---:|---:|
| before | 1.450s | 1.470s | 1.472s | 1.500s |
| after | 0.720s | 0.735s | 0.737s | 0.760s |
Problem Summary: This PR changes the BE COW contract around
`assume_mutable`: callers may only use it when they already own the
column exclusively, and the helper now behaves as an ownership assertion
instead of a silent mutable borrow. Shared `ColumnPtr` / `Block` data
must go through `mutate()` or one of the scoped owner-slot mutation
helpers before being modified. This lets blocks and columns rely on real
COW semantics and avoids both unsafe in-place mutation of shared data
and unnecessary cross-operator copies.
The main changes are:
- Make `assume_mutable` / `assume_mutable_ref` validate exclusive
ownership and document audited usage in
`docs/dev/be-cow-assume-mutable-audit.md`.
- Add scoped COW mutation APIs for common owner-slot patterns, including
whole-block scoped mutation, single-column scoped mutation, and
rvalue-only stealing mutation for `Block::mutate_columns()`.
- Migrate BE paths that previously assumed mutable access to explicit
COW ownership transfer, scoped restore, or `MutableBlock` /
`MutableColumns` usage in hot paths.
- Fix affected storage, scanner, external reader,
parquet/orc/json/table-format, variant, aggregation, local exchange, and
schema scanner paths so errors restore moved-out columns and nested
subcolumns are written back through their owner slots.
- Add focused BE UT coverage for the COW contract: shared-column detach,
scoped restore on early error, block schema access through scoped
guards, LocalExchanger restore-on-error, and table-format
partition/missing-column COW behavior.
Root cause: md5/md5sum evaluated every row through Md5Digest and
OpenSSL, which leaves the vectorized string function path dominated by
per-row scalar digest setup and hex materialization.
Fix: add an AVX2 multi-buffer MD5 helper with scalar fallback, expose a
batch hex API, and route single-argument md5/md5sum over
ColumnString/ColumnVarbinary through the batch path while keeping
multi-argument md5sum and sm3 on the existing digest implementation.
test with sql:
```sql
SET parallel_pipeline_task_num=1;
SET enable_query_cache=false;
SELECT SUM(ASCII(SUBSTRING(MD5(CAST(number AS STRING)), 1, 1)))
FROM numbers("number" = "50000000");
```
result:
| version | times | avg | median |
|---|---:|---:|---:|
| upstream/master baseline | 8.59, 10.21, 9.52, 9.93, 8.85s | 9.42s |
9.52s |
| after AVX2 batch | 2.83, 2.84, 2.82, 2.79, 2.82s | 2.82s | 2.82s |
The OpenAI with OAuth authentication will expire but has the ability to
refresh automatically. Therefore, we ensure the refresh by storing and
accessing credentials through OSS.
Remove the mutable BE config used to force string overflow behavior and
keep the production string column limit as a named constant with the
original uint32 max value. The conversion helper now reads a scoped
debug point override only when debug points are enabled, so tests can
exercise the ColumnString64 conversion path without changing global BE
config.
The fault-injection regression now validates through qt_sql baselines
before and after enabling the debug point. Its query builds the string
join key from convert_tz output, which drives the same string-column
conversion path while keeping the expected results deterministic.
Also keep the existing conversion-focused BE unit coverage on debug
point injection rather than global config mutation, and preserve the
overflow conversion loop guard so forced small limits do not read before
the first offset.
Problem Summary:
The real queue mutation paths all re-check _is_cancelled before touching
_block_queue, but cancel() still used a two-lock pattern and close() ran
delayed RPC done callbacks while holding the queue mutex. That made
ownership of queued BlockItem callbacks harder to reason about and left
some debug reads of queue state outside the mutex.
This change makes cancel() and close() move queued BlockItems into a
local list while holding _lock, then runs call_done() after the member
queue is fully detached and the lock is released. It also locks
get_batch()'s debug-only invariant read and debug_string()'s queue-state
read.
Related PR: https://github.com/apache/doris/pull/63183
Problem Summary:
don't fuzzy it since the overflow may occurs in loading path which has
no proper exception deal.
TIMESTAMPTZ result rows are serialized as timezone-aware strings in the
MySQL binary protocol. FE metadata used to advertise them as
MYSQL_TYPE_DATETIME, so Connector/J decoded the length-encoded string
bytes as a MySQL datetime binary payload and failed with Invalid length
(32) for type TIMESTAMP when ResultSet.getString read a server-prepared
result.
Change PrimitiveType.toMysqlType() to return MYSQL_TYPE_STRING for
TIMESTAMPTZ, adjust the field packet length and decimals to string
semantics, and make the FE local binary result path write TIMESTAMPTZ
through the string fallback instead of the datetime binary layout. BE
serialization already uses push_timestamptz -> push_string, so no BE
payload change is needed.
Add a field-packet unit test and a regression suite that forces
ServerPreparedStatement and compares direct ResultSet.getString(ts) with
CAST(ts AS STRING).
String column overflow can be triggered when ordinary vectorized string
output grows past the 32-bit `ColumnString` offset limit. The previous
widening-to-`ColumnString64` approach is unsafe for ordinary
operator/function outputs because many downstream STRING paths still
assume `ColumnString` and are not prepared for `DataTypeString` backed
by `ColumnString64`.
This PR changes the boundary:
- Checked `ColumnString` append paths keep 32-bit `ColumnString`. When
the effective string byte limit would be exceeded, they throw
`STRING_OVERFLOW_IN_VEC_ENGINE` with size/limit details.
- `string_overflow_size` keeps the production default `4294967295`, so
the normal limit remains 4GB. Tests lower it dynamically to exercise the
overflow branch without allocating multi-GB columns.
- Existing owned accumulator paths that deliberately call
`insert_range_from_ignore_overflow()` and later
`convert_column_if_overflow()` are left intact, because those are the
safe places to replace owned mutable columns.
## Summary
- Switch OpenCode review runner auth provider from `github-copilot` to
`openai`
- Update model parameter from `github-copilot/gpt-5.5` to
`openai/gpt-5.5`
- Use separate secrets for `access`, `refresh`, and `accountId` fields
instead of a single shared secret
Release note: None
Problem Summary: TIMESTAMPTZ values in a DST fold hour could be
converted through an ambiguous local time, making explicit post-fold
instants use the earlier branch during FE literal handling and BE
floor/ceil/date_trunc execution.
The old code would treat DST repeated hours like `2024-11-03 01:xx` as
local civil time without offset when parsing TIMESTAMPTZ with explicit
offset in FE, and when BE converts local time to UTC using
date_trunc/floor/ceil, it would lose the `-04/-05` offset branch
information and default to the earlier branch. After the fix, FE
directly constructs TimestampTzLiteral from timezone-aware strings, and
BE uses the original UTC instant's offset to select pre/post branches
when converting repeated hours back to UTC, thus preserving the correct
fold branch.
when `time_zone = 'America/New_York'`:
```sql
SELECT
CAST(CAST('2024-11-03 01:05:00 -05:00' AS TIMESTAMPTZ(6)) AS VARCHAR(64)) AS ts,
CAST(date_trunc(CAST('2024-11-03 01:05:30 -05:00' AS TIMESTAMPTZ(6)), 'hour') AS VARCHAR(64)) AS hour_trunc;
```
before:
```
ts hour_trunc
2024-11-03 01:05:00.000000-04:00 2024-11-03 01:00:00.000000-04:00
```
now:
```
ts hour_trunc
2024-11-03 01:05:00.000000-05:00 2024-11-03 01:00:00.000000-05:00
```
Problem Summary: SerDe from_olap_string, from_fe_string, and
from_zonemap_string handled invalid scalar strings inconsistently: some
supported types returned errors while others filled default fields. This
change makes supported string deserialization paths fill type defaults
consistently on parse failure.
ATTN: We used to write some values in zonemap that were considered
unreasonable after upgrading to later versions, as these values were
actually not used (mostly just placeholders in partial updates or
something). The only reasonable approach was to ignore them.
Problem Summary: Remove the remaining Pipeline Tracing packaging
artifacts and repository tools so the codebase no longer ships dead
tracing directories or conversion scripts after the runtime feature
deletion.
before we often meet error like
```
[ERROR] Required array size too large -> [Help 1]
[ERROR] java.lang.OutOfMemoryError: Required array size too large
```
when compile FE.
This is because Maven loads all content that constitutes the signature
into memory when checking whether it can reuse the cache. Since we
incorrectly passed `DORIS_HOME`, any file within the project becomes
part of the checksum for the cache. When loading into memory, large
files (such as some core dumps or compressed packages) can easily lead
to an OOM error.
This is also why disabling the cache previously could avoid this issue.
The problem has now been thoroughly fixed by removing the `DORIS_HOME`
directory from the checksum cache. This does not affect the
participation of `gensrc/` content in the cache identification, as they
are explicitly declared through other means.
Problem Summary: Pass existing PR review threads and user-provided
/review focus text into the AI review workflow so the reviewer can avoid
repeating already-raised issues and respond to requested focus areas.
validated by https://github.com/zclllyybb/doris/pull/26
if a sql raise an error, it should be:
```sql
Operation unix_timestamp of 2024-01-01, %Y%m%d is invalid
```
but before it is:
```sql
Conversion = 'Y'
```
now fixed.
The callback's call() method may reuse the callback object (e.g., in
vdata_stream_sender.h get_send_callback()), triggering a new RPC that
mutates response_ and cntl_. If AutoReleaseClosure::Run() invokes call()
before checking cntl_->Failed() or response_->status(), it reads the NEW
RPC's state instead of the ORIGINAL RPC's result, causing:
```
*** SIGSEGV address not mapped to object (@0x0) received by PID 238162 (TID 240463 OR 0xfffa2c9898e0) from PID 0; stack trace: ***
0# doris::signal::(anonymous namespace)::FailureSignalHandler(int, siginfo_t*, void*) at /home/zcp/repo_center/doris_release/doris/be/src/common/signal_handler.h:421
1# os::Linux::chained_handler(int, siginfo_t*, void*) in /opt/module/doris/java8/jre/lib/aarch64/server/libjvm.so
2# JVM_handle_linux_signal in /opt/module/doris/java8/jre/lib/aarch64/server/libjvm.so
3# signalHandler(int, siginfo_t*, void*) in /opt/module/doris/java8/jre/lib/aarch64/server/libjvm.so
4# 0x0000FFFF0AB107C0 in linux-vdso.so.1
5# doris::Status doris::Status::create<true>(doris::PStatus const&) at /home/zcp/repo_center/doris_release/doris/be/src/common/status.h:398
6# void doris::AutoReleaseClosure<doris::PTransmitDataParams, doris::pipeline::ExchangeSendCallback<doris::PTransmitDataResult> >::_process_status<doris::PTransmitDataResult>(doris::PTransmitDataResult*) at /home/zcp/repo_center/doris_release/doris/be/src/util/ref_count_closure.h:128
7# doris::AutoReleaseClosure<doris::PTransmitDataParams, doris::pipeline::ExchangeSendCallback<doris::PTransmitDataResult> >::Run() at /home/zcp/repo_center/doris_release/doris/be/src/util/ref_count_closure.h:102
8# brpc::Controller::EndRPC(brpc::Controller::CompletionInfo const&) in /opt/module/doris/be/lib/doris_be
9# brpc::policy::ProcessRpcResponse(brpc::InputMessageBase*) in /opt/module/doris/be/lib/doris_be
10# brpc::ProcessInputMessage(void*) in /opt/module/doris/be/lib/doris_be
11# bthread::TaskGroup::task_runner(long) in /opt/module/doris/be/lib/doris_be
12# bthread_make_fcontext in /opt/module/doris/be/lib/doris_be
```
we have confirmed the data race is real existing with temporary LOGs
which has been removed:
```
F20260325 21:46:58.465230 3453395 brpc_closure.h:116] Check failed: _debug_generation_at_construction == current_gen (2 vs. 3) RACE DETECTED: AutoReleaseClosure response_ was reused by a new RPC (generation changed from 2 to 3) while still in Run(). The old closure is about to read response_->status() but the new RPC may be concurrently writing to the same response_ object.
```
and we add some be-ut which could only pass WITH this patch.
before we fix:
```
[----------] 7 tests from ExchangeSinkTest (5 ms total)
[----------] Global test environment tear-down
[==========] 7 tests from 1 test suite ran. (5 ms total)
[ PASSED ] 4 tests.
[ FAILED ] 3 tests, listed below:
[ FAILED ] ExchangeSinkTest.test_closure_call_must_not_corrupt_status_check
[ FAILED ] ExchangeSinkTest.test_closure_call_must_not_hide_error_status
[ FAILED ] ExchangeSinkTest.test_closure_call_must_not_hide_rpc_failure
```
after:
```
[----------] 7 tests from ExchangeSinkTest (4 ms total)
[----------] Global test environment tear-down
[==========] 7 tests from 1 test suite ran. (5 ms total)
[ PASSED ] 7 tests.
```
Problem Summary: Remove the legacy io_helper wrapper layer and move its
parsing and formatting entry points onto the existing cast
implementations so serde and conversion code depends on the real owners
of that behavior.
Related PR: https://github.com/apache/doris/pull/61954
Problem Summary:
Extract the dynamic partition table check logic into a private helper
method `isDynamicScheduleTable()` and add documentation explaining that
DynamicPartitionScheduler tracks both dynamic partition tables with
scheduling enabled and auto partition tables using
`partition.retention_count` for periodic cleanup.
Replace GitHub PR review approval/request-changes mechanism with commit
status tracking for code-review workflow. The workflow now:
- Posts "pending" status on PR open/sync/reopen events
- Updates to "success" status when /review comment triggers automated
review
- Removes decision-based approval logic and RESULT_START/RESULT_END
parsing
- Changes gh pr review --approve to --comment for final summary
make AI review to be official access control workflow.
Split code review workflow into two parts:
1. Main workflow (opencode-review.yml) now triggers on PR events
(opened/synchronize/reopened/ready_for_review) as a required check, but
only runs actual review when called via workflow_call
2. New comment dispatcher (opencode-review-comment.yml) triggers on
/review comments and invokes the main workflow with PR context
and remove `Need 2 Approval`.
tested in https://github.com/zclllyybb/doris/pull/24 and
https://github.com/zclllyybb/doris/pull/25
still need to enable `Allow GitHub Actions to create and approve pull
requests` and set workflow permissions to `Read and write permissions`
Remove the Need_2_Approval check from the required GitHub status checks
list in .asf.yaml, as this approval requirement is no longer needed in
the CI workflow.
Remove obsolete compile_check_begin/end headers and all paired includes
now that the compile option is enabled globally, while keeping minimal
compile_check_avoid guards only around legacy code that still fails
under global conversion checks.
Remove the author association restriction (MEMBER/OWNER/COLLABORATOR)
from the opencode-review workflow trigger conditions, allowing any user
to trigger code review via `/review` comment.
Auto-partition tables with `partition.retention_count` set were not
being registered in DynamicPartitionScheduler during FE restart, causing
retention policies to stop working until manual re-registration.
Modified `initDynamicPartitionTable()` to check for both dynamic
partition tables and OlapTables with retention_count > 0, ensuring these
tables are properly tracked after restart
- Change opencode review model from claude-opus-4.6 to gpt-5.4
- Add instructions for using GitHub suggested changes blocks in inline
comments
- Clarify when to prefer suggested changes (small, self-contained fixes)
vs normal explanations (broad/architectural issues)
and split individual `CastToTimestampTz` class
and Replace `IS_STRICT` and `IS_DATETIME` boolean template parameters
with strongly-typed enums `DatelikeParseMode` and `DatelikeTargetType`
across all datetime cast operations. This improves code readability and
type safety by making template parameters self-documenting.
- Restrict `-fsanitize-ignorelist` flag to Clang only in ASAN build
flags, as GCC does not support this option
- Add explicit `static_cast<uint16_t>` for bitwise NOT operations to fix
implicit conversion warnings in memcmp_small.h, ip_address_cidr.h, and
find_symbols.h
- Add explicit cast in ipv6_to_binary.h mask generation and simplify
template usage
- Remove unnecessary `virtual` keywords
- Change `_try_close` from `bool` to `std::atomic<bool>` with
acquire/release memory ordering
- Use `memory_order_acquire` when reading in `_send_batch_process()`
(bthread)
- Use `memory_order_release` when writing in `_do_try_close()` (pthread)
- Remove unused `_close_wait` field
This fixes a potential race condition where `_try_close` is written by
pthread and read by bthread without proper synchronization.
It's ok on x86 so no test added.
Normal Hit Rate:
```
cache hit (direct) 1303
cache hit (preprocessed) 20
cache miss 219
cache hit rate 85.80 %
```
Previously, when we opened a worktree, the first compilation:
```
cache hit (direct) 0
cache hit (preprocessed) 0
cache miss 1404
cache hit rate 0.00 %
```
Now (using another different directory name, plus the current patch):
```
cache hit (direct) 4665
cache hit (preprocessed) 2517
cache miss 0
cache hit rate 100.00 %
```
see more in
https://ccache.dev/manual/4.7.4.html#_compiling_in_different_directories
docs: https://github.com/apache/doris-website/pull/3452
Replace LOG_INFO with VLOG_CRITICAL for profile send and registration
success messages to reduce log noise in production environments. Profile
is not a critical part of SQL execution, so it can be adjusted like
this. It's not too late to open the verbose log if any issues are found.
When filtering or clearing block columns, the code incorrectly assumed
exclusive ownership and directly mutated shared columns via
`assume_mutable()`. This violates Copy-On-Write (COW) semantics and can
cause data corruption when columns are shared across multiple operators
(e.g., in CTE MultiCast scenarios).
Introduce DORIS_CHECK macro to enforce preconditions with fail-fast
behavior:
- Throws FatalError exception when assertion fails
- Replaces defensive if-checks with explicit precondition validation
- Follows guideline: crash on unexpected conditions rather than continue
execution
Related PR: https://github.com/apache/doris/pull/59990
Problem Summary:
Move setTransactionStatus(COMMITTED) to after idToTableCommitInfos
population in both commitTransaction methods. This prevents
PublishVersionDaemon from seeing COMMITTED status with empty commit
info, as it checks status without synchronized(transactionState) lock.
Related PR: https://github.com/apache/doris/pull/59262
Problem Summary:
During BE graceful shutdown, cancel_worker cancels running queries via
_get_all_running_queries_from_fe, so clearing
_query_ctx_map_delay_delete here won't cause RF consumer to hang.
Without this clear, QueryContext destruction in ~FragmentMgr() may
attempt to remove itself from _query_ctx_map_delay_delete while the map
is being destructed, causing undefined behavior.
stack:
```
~FragmentMgr
~_query_ctx_map_delay_delete
~QueryContext
FragmentMgr::remove_query_context
_query_ctx_map_delay_delete.erase() ⚠️call function of object that is being destructed
```
Dictionary refresh tasks build load command internally, which didn't set
`originSql` of it. this may cause some usage fail because of dependency
of not null `originSql`.
1. Change construct method to fix null `originSql` of dictionary refresh
tasks.
2. add defensive check in `checkBlockSql` to avoid failure.
zclllyybb