mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
bdb90856e6e468eb495e5bfd67010de90570bfbc
32960
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  zhaorongshengandzhaorongsheng
|
bdb90856e6 |
[improvement](hive) Push partition filters to HMS (#67725)
## What problem does this PR solve? Push eligible Hive partition equality/IN predicates into bounded HMS filtering without eagerly materializing every partition. Preserve conservative fallback for unsupported predicates, types, identifiers and unavailable views. This update addresses the current review findings: - Reuse filtered handles without losing stronger backend residual predicates. - Prepare complete partition views for MV creation and eligible refresh/rewrite consumers. - Distinguish unavailable coverage from native/all coverage during MV compensation. - Fence ROOT/ADMIN selective subsets against schema and connector changes. - Restrict filtered, unfiltered and empty refresh reads to their materialized pin. - Preserve connector-retained schema through deferred pin hydration. - Keep INSERT preloading outside the outer target lock; retry schema or target replacement before binding with refreshed table caches. - Avoid pre-binding constant folding of unbound CAST predicates. - Handle reserved Hive partition keys `in` and `between` through local fallback. - Display unknown full partition totals as `?` without enumerating solely for EXPLAIN. - Resolve the latest target integration conflict by preserving both partition-pruning and write-only-default capabilities and advancing the connector API to 28.0. ## Release note Improve selective Hive partition planning while preserving schema, snapshot and materialized-view consistency. Selective EXPLAIN output may show an unknown full partition total as `?`. ## Validation - Published head: `ae2f21cbb30469408970cfb81709d8bbe27a8998`. - Target snapshot integrated without rewriting published history: `0976b2cd5020607205969bf7986dcb551b6a7325`. - Before the final target integration, canonical `./run-fe-ut.sh --coverage --run ...` runs verify 39 selected classes: 945 tests, zero failures/errors, one existing disabled test. - After the final target-replacement correction, a fresh targeted canonical run passes 33 tests, zero failures/errors/skips, and 89 actual zero-violation Maven Checkstyle executions. - After the final target integration, fresh canonical reports verify 36 classes and 642 tests, zero failures/errors/skips. This includes SPI's own full suite, Hive/Iceberg, view/MVCC, planning and partition compensation, INSERT/defaults, MTMV task fixtures and SQL-cache version tests. - The first integration run passes 641 tests but exits 1 when the final mock-FE class hits an occupied ephemeral edit-log port before executing its test. The isolated single-method rerun passes the remaining test, exits 0 with `BUILD SUCCESS`, and performs 89 actual zero-violation Checkstyle executions. These results are aggregated from fresh reports, not a claim that the all-class reactor succeeded in one attempt. - Coverage includes connector/HMS/SPI, MVCC schema and partition views, sync/stream/batch handoffs, MV compensation, policy/replan boundaries, INSERT lock windows and real target replacement in both execution and planning-only modes. Previously failing CAST/shape classes are included. - Connector-import, metadata-funnel, build-hygiene and authored whitespace checks pass. - Local mock-FE port conflicts required targeted reruns. A temporary test-only non-ephemeral port selection was used and restored before committing. - Standard local `./build.sh --fe` packaging remains blocked before Maven by missing genuine native thirdparty prerequisites. No successful full packaging, live HMS/BE regression or complete refresh-scheduler run is claimed. - Extended review rounds were stopped at the author's request. All independently confirmed remaining findings are repaired and tested; no pipeline PASS receipt or new remote `/review` request is issued by this update. --------- Co-authored-by: zhaorongsheng <zhaorongsheng@corp.netease.com> |
||
 Jack
|
7cca01bc6c |
[fix](be) Score MATCH on keyword inverted indexes like SEARCH (#69790)
### What problem does this PR solve? Issue Number: None Related PR: #69051 Problem Summary: score() of a MATCH on an untokenized (keyword) inverted index is always 0, while SEARCH on the same field scores BM25: INDEX idx_redirect (redirect) USING INVERTED -- one row ('BP') SELECT score() FROM t WHERE redirect MATCH 'BP'; -- 0 SELECT score() FROM t WHERE SEARCH('redirect:BP'); -- 0.2876821 Elasticsearch scores a match, term or match_phrase query on a keyword field with BM25 in query context: no norms and no frequencies, so freq = dl = avgdl = 1 and the score is the idf of the whole value (0.2876821 above, 0.6931471 for N=4, n=2). Only a bool filter scores 0. SEARCH already gives exactly these values. MATCH on keyword behaved like a filter, so `body MATCH 'apple' AND tag MATCH 'x'` ranked as if the tag clause were not there. Root cause: #69051 made the engine score keyword postings (unit frequency and norm) but kept MATCH out on purpose: can_score_match required an analyzed index, and MatchPredicateCollector returned early for indexes without an analyzer, so no statistics were collected for them. Fix: a keyword (non-analyzed) index is eligible for MATCH scoring regardless of support_phrase, and MatchPredicateCollector collects its whole value as one term (case and spaces kept), the same term the reader looks up and the same as SearchPredicateCollector collects. Analyzed indexes keep the existing support_phrase / lowercase-normalizer rule. Equality and IN stay pure filters. A keyword MATCH_PHRASE_PREFIX scores the constant 1 of a single-term prefix; MATCH_REGEXP is unchanged. |
||
 Mryange
|
4bc7f42682 |
[fix](be) Fix TIMESTAMPTZ schema change defaults (#69611)
### What problem does this PR solve?
Issue Number: N/A
Problem Summary:
Adding a TIMESTAMPTZ column with DEFAULT CURRENT_TIMESTAMP could finish
schema change but leave historical rows unreadable. Root cause: the
backfill default was materialized without timezone information, and the
batch ADD COLUMN and Protobuf paths did not consistently use the
materialized value.
Materialize TIMESTAMPTZ backfill defaults as UTC literals, use the
schema-change conversion only for TIMESTAMPTZ/TIMESTAMP_NS in batch ADD
COLUMN, preserving other types' existing conversion, and preserve the
default expression in both Protobuf and Thrift. Partial updates evaluate
the preserved expression for new rows instead of reusing the fixed
backfill timestamp.
### Release note
Fix unreadable historical TIMESTAMPTZ values after adding a column with
DEFAULT CURRENT_TIMESTAMP.
### Check List (For Author)
- Test:
- [x] Regression test: test_timestamptz_default_schema_change passed
with -forceGenOut and again normally (1 suite, 0 failures each),
including TIMESTAMP_NS backfill and TIMESTAMP_NS/DATETIME partial-insert
checks.
- [ ] Unit Test: FE coverage added and compiled, not executed
separately.
- BE ASAN build passed for the BE changes. FE rebuilt successfully after
the follow-up. `git diff --check` passed. C++ formatting checks passed.
Clang-tidy reported existing diagnostics outside modified lines; the
full scan was not clean.
- Behavior changed:
- [x] Yes. Historical rows receive readable TIMESTAMPTZ defaults;
subsequent writes retain dynamic CURRENT_TIMESTAMP evaluation.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
 yujun
|
c9b73d2947 |
[fix](ivm) Plan EXPLAIN REFRESH COMPLETE with the read mode a refresh reads (#69851)
`EXPLAIN REFRESH MATERIALIZED VIEW mv COMPLETE` for an IVM materialized view did not show the plan the refresh actually runs. The explain path installed `IvmRewriteContext.fullExplain`, which carried no stream read mode, so `hasFullRefreshStreamScans()` was false and `IvmFullRefreshMTMV` never rewrote the base-table scans. The statement therefore printed the normalized MV query over ordinary base-table scans, while a real COMPLETE refresh reads its base tables through IVM table streams in RESET mode (the first batch of a whole-MV refresh, and every table it does not partition by). That rewrite reshapes the plan, so the explained plan and the executed plan differed in exactly the part EXPLAIN is used to debug. Problem Summary: - Root cause: `IvmRewriteContext.fullExplain` passed an empty reset partition map and an empty non-PCT read mode, so the full-refresh stream rewrite rule was gated off during EXPLAIN. - Fix: the explain context now carries `StreamReadMode.RESET`, and the full-refresh rewrite reads every stream-scanned base table — including the tables the MV partitions by — in that mode under EXPLAIN, over the partitions the plan selects. - A base table without a usable IVM stream now fails the statement instead of being explained as a plain scan. EXPLAIN plans the same read the refresh runs, and the refresh fails the same way (a real COMPLETE refresh reconciles the stream before planning; EXPLAIN must not create one). Notes: - A complete refresh is not one plan: it may be split into batches, and the later batches read non-PCT tables at the stream offset (SNAPSHOT) and read tables the MV partitions by as ordinary scans. One EXPLAIN statement stands for the refresh as a whole, so it shows the read the refresh starts with — RESET for every stream read — and no reset partition scope is resolved for the explain plan. - No execution state is touched. EXPLAIN never builds an insert transaction, so no stream offset advances; the only added work at plan time is reading stream metadata. |
||
 Calvin Kirs
|
0976b2cd50 |
[opt](sql-cache) Preserve SQL cache across unrelated partition writes (#69883)
### What problem does this PR solve? Issue Number: None Problem Summary: SQL cache currently invalidates an OLAP query whenever the table visible version changes. For example, a cached query reading historical partition p1 is discarded after an insert into unrelated partition p2, even though its result is unchanged. Record the partition versions actually assigned to the scan, including selected empty partitions removed during planning. When the table version advances, reuse the cache only if those dependencies and the schema/partition topology still match. Use the same stable dependency identity for the BE cache key. Keep table-wide validation for metadata aggregates, variable-dependent keys and materialized-view rewrites whose physical scans can hide source dependencies. The change is FE-only and preserves the existing BE cache protocol. Regression cases use only 3–5 source rows and cover unrelated/relevant writes, empty partitions and branches, self joins, partition DDL, partial MV rewrites and eliminated empty MVs. |
||
 shuke
|
38a38dda22 |
[fix](ci) Use GPT-6 Sol for review capacity fallback (#69988)
### What problem does this PR solve? Related PR: #69953 When GPT-6.1 Sol encounters five capacity failures, review recovery currently switches to GPT-6 Astra, which has continued to encounter capacity failures in observed runs. Change the capacity fallback to `gpt-6-sol` and update the existing test fixtures and assertions to exercise that target. The production change is one workflow argument. The primary model remains `gpt-6.1-sol` with `xhigh`; the five-failure threshold, seven total capacity attempts, 300-second backoff, same-session recovery, shared budget, ledger and round preservation, head-change checks, and submission verification are unchanged. |
||
 Mingyu Chen (Rayner)
|
7e6f6cb3a3 |
[fix](fe) Resolve DEFAULT(column) on Iceberg tables from the write schema (#69488)
### What problem does this PR solve? Issue Number: None Related PR: #66538, #57531, #65851 Problem Summary: **In short.** On an Iceberg table, `DEFAULT(column)` anywhere except `UPDATE ... SET` and `MERGE ... VALUES` reads the default from the read schema, which carries no Iceberg write-default. This PR resolves it against the write schema that the statement pinned for its target, as branch-4.1 and branch-4.2 do. **Background.** An Iceberg `write-default` applies to rows written after it is set. So it lives in the writer schema that a write statement pins (`IcebergWriteSchemaContext`), not in the columns of the read schema. #65851 resolves `DEFAULT(column)` against that pinned schema in `UPDATE ... SET`, `MERGE ... UPDATE SET` and `MERGE ... INSERT VALUES` (`ConnectorWriteSchemaUtils`). Every other `DEFAULT(column)` either fails at binding or reaches `RewriteDefaultExpression` (#57531), which reads the default from the bound column. branch-4.1 #66538, also in branch-4.2, resolves these against the pinned write target too. It rejects `DEFAULT(column)` on an Iceberg column in a statement that does not write the table. **The problem, and what it cost.** Reproduced on a local cluster with a `file://` format-version 3 table: `CREATE TABLE t (id INT, c INT)`, `ALTER TABLE t ADD COLUMN d INT DEFAULT 42`, one row `(0, 0)`, so `d = 42`. | Statement | Master today | After this PR (same as branch-4.2) | |---|---|---| | `INSERT INTO t (id, c, d) VALUES (1, 1, DEFAULT(d))` | `Unknown column 'd' in 'table list'` | writes 42 | | `INSERT INTO t (id, c, d) SELECT 2, 2, DEFAULT(d) FROM t WHERE id = 0` | writes NULL | writes 42 | | `INSERT INTO t (id, c, d) SELECT 3, 3, DEFAULT(d)` | `Unknown column 'd'` | writes 42 | | `UPDATE t SET c = 100 WHERE d = DEFAULT(d)` | updates nothing: `d = NULL` makes the plan an empty set | updates every row, `d = 42` is pushed down to the scan | | `MERGE INTO t USING (SELECT 0 AS id) s ON t.id = s.id AND t.d = DEFAULT(t.d) WHEN MATCHED THEN UPDATE SET c = 300` | matches nothing | updates row 0 | | `SELECT DEFAULT(d) FROM t` | NULL | `DEFAULT(d) must reference a column of the table that this statement writes` | The `UPDATE` and `MERGE` cases fail silently: the statement succeeds and changes nothing. **How this PR fixes it.** - fe-core cannot tell from a column whether its connector keeps defaults in the read schema. Hive does: an HMS default shows up in both schemas. Iceberg does not. The new connector capability `SUPPORTS_WRITE_ONLY_DEFAULTS` says which kind a table is. The Iceberg connector declares it. The Hive connector passes it on to iceberg-on-HMS tables, like the other per-table capabilities, but only through the two-argument `getTableSchema`. Since #69299, a statement that pins the table at its latest snapshot reads the schema from the snapshot overload, which reflects none of them, so on an iceberg-on-HMS table such a statement keeps master's results (for example, `UPDATE t SET c = 100 WHERE d = DEFAULT(d)` still changes no row). Reflecting the sibling's capabilities there changes planning for all of them and is left to a follow-up PR. - For a target that declares it, `InsertUtils.normalizePlan` resolves `DEFAULT(column)` in `INSERT ... VALUES` and in the select lists of `INSERT ... SELECT`, including the replacements of `SELECT * REPLACE(...)`, against the pinned write schema. It does this by name, before the source relations are bound, so the name always refers to a target column, as in branch-4.2. In MySQL, `DEFAULT(column)` in a `SELECT` refers to a source column instead. A derived table in `FROM` is a query of its own and is left alone: `DEFAULT(column)` in it still names a column of its own `FROM` clause. - `RewriteDefaultExpression` takes the default for a `DEFAULT(column)` bound to such a table from the pinned write column with the same field id. That covers the `WHERE` of `UPDATE` and the `ON` of `MERGE`. A MERGE resolves it whatever its WHEN branches, also when they all delete, as branch-4.2 does. If the statement pinned no write schema for that table, it fails. A `DELETE` pins the writer schema too, for its delete sink, but it writes no row values, so `RowLevelDmlCommand` marks its target and `DEFAULT(column)` on it fails with `DEFAULT(d) cannot be used in a DELETE: the defaults of this table apply only to the rows that a statement writes`, as in branch-4.2, whose DELETE installs no write schema context. Master reads NULL there, so `DELETE FROM t WHERE d = DEFAULT(d)` deletes nothing; resolved to the write-default it would delete every row with `d = 42`. - A resolved default is cast to the type of its writer column before the expression around it is bound, as `RewriteDefaultExpression` already does for a bound `DEFAULT(column)`. The parsed literal `-1` is a TINYINT, so without the cast `bit_count(DEFAULT(c))` on an INT column would count 8 bits instead of 32. The external `UPDATE`, `MERGE` and changelog paths use the same helper. `RewriteDefaultExpression` rejects a bound `DEFAULT(column)` whose writer column has another type than the read column the statement was bound with, which happens only when the table changed outside Doris after Doris cached it, and asks to refresh the table: the expressions around it were bound against the read type. - Hive, Paimon and internal tables do not declare the capability, so they behave as before. - The new enum constant changes the connector plugin API surface, so `connector.plugin.api.version` goes from 26.0 to 27.0, as for the other recent changes to that surface (#69299, #69588, #69298, #69683 and #69835 took 22.0 to 26.0). **Where `DEFAULT(column)` resolves on an Iceberg target (direct Iceberg catalogs).** | Statement | Position of `DEFAULT(column)` | Result | branch-4.2 | |---|---|---|---| | `INSERT` / `INSERT OVERWRITE` | `VALUES` rows | the target column's write-default, typed as the column | same | | | the INSERT's own select lists (with or without `FROM`, set operation branches, `SELECT * REPLACE`) | the target column's write-default, by name, before binding | same, except `* REPLACE`, which branch-4.2 binds to the source | | | `GROUP BY` / `ROLLUP` outputs, a derived table in `FROM`, `WITH` definitions, subqueries, `WHERE` / `ON` | bound like any `DEFAULT(column)` (last row) | same, except derived tables, which branch-4.2 resolves against the target | | | a `VALUES` branch of a set operation | fails with an unknown column, as on master | same | | | a qualifier other than the target (`DEFAULT(src.c)`, `DEFAULT(ctl.db.t.c)`) in the positions above | fails: `column in assignment list is invalid` | same | | `UPDATE` | `SET` / `WHERE` | write-default | same | | `MERGE` | `ON` and `WHEN ... AND` conditions, whatever the WHEN branches, also when they all delete | write-default | same | | | `UPDATE SET` / `INSERT VALUES` | write-default | same | | `DELETE` (also `DELETE ... USING`) | anywhere | fails: `cannot be used in a DELETE` | fails: its DELETE has no write schema context | | any | a bound `DEFAULT(x.c)` | `x` is the written table: its write-default by field id; an OLAP or Hive table: that column's default; another Iceberg table: fails with `must reference a column of the table that this statement writes` | same | A bound `DEFAULT(column)` whose writer column has another type than the read column fails and asks to refresh the table. Where `DEFAULT(column)` is bound rather than resolved by name (the third row), it keeps its meaning from before this PR and names a column of its own `FROM` clause, as on master and branch-4.2: for example in `INSERT INTO t SELECT id, DEFAULT(d) FROM src GROUP BY id, d` with an OLAP or Hive `src`, it takes the default of `src.d`. **Results.** - Local cluster, with the table above: the table shows what each statement did before and after. `EXPLAIN UPDATE t SET c = 5 WHERE d = DEFAULT(d)` goes from an empty set to a scan with `d = 42` pushed down to Iceberg. - Unit tests: 6 new tests in `InsertUtilsTest`, 6 in the new `RewriteDefaultExpressionTest` and 1 in `RowLevelDmlCommandTest`. They cover VALUES, and INSERT ... SELECT both from a staging relation whose own column is also `name` and without FROM: each resolves to the pinned default, and an unknown column fails. `DEFAULT(column)` inside a derived table, qualified or not and also under a UNION branch, is left to binding. A `SELECT * REPLACE(DEFAULT(name) AS name)` replacement resolves like the rest of the select list and keeps the star's qualifier and `EXCEPT` list. The pinned write column wins over the read column. A statement that writes nothing fails. A table without the capability is unchanged. With master's `InsertUtils` and `RewriteDefaultExpression`, the 5 tests of the fix fail and the 2 tests of the unchanged path pass. The `SELECT * REPLACE` test fails with this PR's previous `InsertUtils`, where the replacement stays `DEFAULT(name)`. `IcebergConnectorTest`, `HiveConnectorMetadataSiblingDelegationTest` and `ConnectorPluginSurfaceTest` pass. So do the fe-core tests around the write schema: `ConnectorWriteSchemaUtilsTest`, `ExternalRowLevelUpdatePlanBuilderTest`, `ExternalRowLevelMergePlanBuilderTest`, `ChangelogRowLevelDmlTransformTest` and `PluginDrivenExternalTableTest`. Two of the tests pin the typing: `bit_count(DEFAULT(bits))` gets the default as an INT in VALUES and in SELECT, and fails without the cast; a BIGINT writer column over an INT read column is rejected, directly and under an IF bound to INT. `DEFAULT(column)` on the target of a DELETE fails even with a pinned write-default, and only a DELETE marks its target (an UPDATE does not); skipping the check, or marking UPDATE instead of DELETE, makes exactly these two tests fail. `PositionDeleteRowLevelDmlTransformTest`, `PhysicalPlanTranslatorIcebergRowLevelDmlTest`, `DeleteFromCommandTest` and `UpdateCommandTest` pass. - Regression: `test_iceberg_initial_defaults` now writes rows 20 to 22 with `DEFAULT(default_int)` in VALUES and in INSERT ... SELECT with and without FROM. It then updates them with `WHERE default_int = DEFAULT(default_int)` and `MERGE ... ON ... = DEFAULT(...)`, for Parquet and ORC, and checks the rows in the `.out`. A plain `SELECT DEFAULT(default_int)` must fail, and so must a `DELETE ... WHERE default_int = DEFAULT(default_int)` on rows 20 to 22, which hold the write-default. The suite passed locally before the DELETE block was added, and the whole suite, DELETE block included, has since passed in two External regression runs on CI. With master's FE it fails at the first of these statements: `Unknown column 'default_int' in 'table list'`. |
||
|
Mingyu Chen (Rayner)
|
fe58e58386 |
[feature](cloud) Support declaring compute_group on routine load and async MV (#69749)
### What problem does this PR solve? Issue Number: None Related PR: #67010 Problem Summary: **In short.** This ports branch-4.1 #67010 (also on branch-4.2) to master: CREATE / ALTER ROUTINE LOAD and async materialized views accept a `compute_group` property in cloud mode, so a background job can be pinned to a compute group instead of inheriting one implicitly, and the declared group is re-checked before every task. **Background.** In cloud mode every query runs in a compute group. A routine load job snapshots the compute group of the session that created it, and an async MV has no compute group of its own: an automatic refresh runs wherever `admin` resolves to, and a manual `REFRESH` borrows the group of the session that triggered it. **The problem, and what it cost.** On master nothing lets a user choose where a background job runs: | | master today | with this PR | |---|---|---| | Routine load | runs in whatever group the creating session was on; ALTER cannot move it | `"compute_group" = "cg_etl"` on CREATE or ALTER pins it | | Async MV, automatic refresh | runs where `admin` resolves to | runs in the declared group | | Async MV, manual `REFRESH` | runs in the triggering session's group, so the same MV refreshes in different places depending on who triggered it | runs in the declared group; an MV that declares none keeps borrowing the session's group | | Async MV, `REFRESH ... WITH DRY RUN` and `EXPLAIN REFRESH` | run and plan where `admin` resolves to | run and plan in the declared group; an MV that declares none resolves as today | | A pinned group is dropped, or the owner's USAGE on it is revoked | (no pinning) | the next task fails with that reason instead of a later "no available BE found" | | `compute_group` in non-cloud mode, or `DEFAULT` | (no property) | rejected at CREATE / ALTER | Four smaller defects go with it on master: CREATE ROUTINE LOAD checks the `workload_group` in the session's compute group namespace although the job runs elsewhere; `RoutineLoadJob.plan()` builds the sink's plan context from the raw snapshot field rather than the effective group; and the statement re-parsed when a routine load job is loaded from metadata resolves its `workload_group` again. On image load that lookup runs before any workload group exists (`PersistMetaModules` reads `routineLoadJobs` before `workloadGroups`, and until then the Env holds the empty `WorkloadGroupMgr` its constructor created), and a failure there sets the job to the final `CANCELLED`, so a job created with `workload_group` is cancelled by the first restart that loads it from an image (shown by a unit test with an empty manager, not reproduced on a cluster). And when the partition refresh fails, which `RoutineLoadJob.update()` runs before it takes the job lock, it pauses a NEED_SCHEDULE job without that lock, so a STOP that completes in between is turned back into PAUSED on the master while replay keeps STOPPED (Kafka metadata failures and Kinesis shard listing alike). **How this PR fixes it.** Same design as #67010: - The declaration is stored in the existing property maps (`RoutineLoadJob.jobProperties`, `MTMV.mvProperties`) and written only when declared: no new persisted field, journal type or meta version, and an absent key keeps today's implicit resolution, which is also how a later binding model tells a pinned group from an inherited one. `DEFAULT` is rejected because that model reserves it for "follow the owner's default group"; non-cloud mode rejects the property, and `ConnectContext.setCloudCluster()` is a no-op outside cloud mode. - `ComputeGroupBindingUtil` holds the two checks: `validateDeclaredComputeGroup` at CREATE / ALTER (USAGE for the current user, then existence, the order of `USE @<group>`), and `checkComputeGroupBeforeTask` before every task, against the identity the task runs as. A missing group keeps `INTERNAL_ERR`, so a routine load job is paused retryably and resumes by itself when the group comes back (a group scaled to zero backends leaves the cluster map and returns); a revoked USAGE uses `CANNOT_RESUME_ERR`, so `ScheduleRule` does not keep resuming it. - Routine load resolves the effective group in `RoutineLoadJob.getCloudCluster()`, which backend selection, the task sent to BE, the workload group namespace, the plan context and the `ComputeGroup` column of SHOW ROUTINE LOAD all use, so every Kafka job follows it (Kinesis routine load is rejected in cloud mode, the only mode with compute groups). `RoutineLoadTaskScheduler` re-checks the declared group before backend allocation and before `beginTxn`. The partition refresh of a Kafka job runs the same check before its metadata request, which goes through the group's backends and, after RESUME, comes before any task: a dropped group pauses the job with `Compute group '<name>' not found.` instead of `no alive backends` and advice to check the Kafka properties. An auto resume attempt skips that check: it refreshes a job that is still PAUSED, which cannot be paused again with another reason, so the attempt goes on to NEED_SCHEDULE and the check before the first task records a revoked USAGE as `CANNOT_RESUME_ERR`. Every pause from the refresh (this check, a failed Kafka metadata request, a failed Kinesis shard listing) goes through `RoutineLoadJob.pauseIfNeedSchedule()`, which takes the job lock and checks NEED_SCHEDULE again, so it cannot undo a concurrent STOP. CREATE validates the workload group in the declared group's namespace. An ALTER that sets `compute_group` validates in that group the workload group the job has afterwards (the one the ALTER sets, or else the one it keeps), as the owner the tasks run as. An ALTER that sets `workload_group` alone is still not checked, as on master. The statement re-parsed at metadata load adopts both names without resolving either, because a failure there would cancel the job for good: the declared group can have been dropped (and the stored statement is the CREATE one, so a job moved to another group by ALTER still names its old group there), and on image load the workload groups are not loaded yet. - CREATE / ALTER ROUTINE LOAD start a suspended declared group the way `USE @<group>` does (`waitForAutoStart`), on the statement thread before any job lock, since they send their Kafka metadata requests (custom partitions, offset conversion) through that group's backends. An ALTER that sets `compute_group` and `kafka_offsets` together converts the offsets in the group it declares, because the job's current group may be gone. The scheduler keeps resolving backends by name without starting a suspended group, as master does for every cloud routine load job. - SHOW CREATE ROUTINE LOAD writes `compute_group` when the job declares one. It writes the declared value, which follows ALTER, and never the effective group, so a job recreated from the statement is pinned the same way, and the statement of a job that declares nothing does not pin the group it inherited. SHOW CREATE MATERIALIZED VIEW already renders every MV property. - An async MV applies the declared group, then the session's group, on the planning context of the task and on every refresh context, and re-checks the declared group before each refresh. Each refresh context reads the declaration once and binds, records and re-checks that value. `REFRESH ... INCREMENTAL WITH DRY RUN` runs outside the task, on the current FE, in a context of its own: it binds the declared group to that context and re-checks it the same way. `EXPLAIN REFRESH` binds it to its planning context without the re-check, as the task does for its own. Neither borrows the session's group for an MV that declares none, as on master. Where master differs from branch-4.x: - master's `MTMVTask` splits a refresh into the IVM incremental path and the partition / complete path, each with its own context. `applyComputeGroup()` is factored out of the existing `setupComputeGroup()`, and the re-check runs in both paths. - master's routine load also has a Kinesis source, but `KinesisRoutineLoadJob` rejects cloud mode, so a Kinesis job never has a compute group to declare. - `REFRESH ... WITH DRY RUN` (from master's IVM) and `EXPLAIN REFRESH` exist only on master. Both build their context in `RefreshMTMVCommand` instead of `MTMVTask`, so that command binds the declared group itself, and the dry run re-checks it. - SHOW CREATE ROUTINE LOAD writes the declared `compute_group`; branch-4.x does not. - An ALTER that sets `compute_group` checks the job's workload group in that group; branch-4.x does not, so there a job moved to a group without its workload group fails every task. - The Kafka partition refresh checks the declared group before its metadata request, except on an auto resume attempt; branch-4.x has no check there. - Each MV refresh context reads the declaration once to bind and re-check it; branch-4.x reads it twice. - The 4.x unit test for a task without a task context is not carried over: `MTMVJob` always creates a task with a context, and master's `setupComputeGroup()` already relies on it. - The three unit test classes are converted to JUnit 5, as master requires, and `MTMVComputeGroupTest` gains two cases for the master-only parts: `setupComputeGroup()` applies and records the declared group, and the MV re-check fails for a declared group that no longer exists while it ignores the session's group. `RefreshMTMVCommandTest` gains four for the dry run and EXPLAIN REFRESH contexts, and `RoutineLoadJobTest` one for SHOW CREATE ROUTINE LOAD. - `ConnectContextTest.testThreadInfoUsesSessionCloudCluster` (#68111) sets the session variable directly instead of calling `ConnectContext.setCloudCluster()`, which is now a no-op outside cloud mode; branch-4.x made the same change when it picked #68111 (#68948). **Results.** The unit tests below pass locally. The regression suites need a cloud cluster and were not run locally: `test_routine_load_compute_group` and `test_mtmv_compute_group` are p0 suites, so P0 runs their non-cloud rejection checks and cloud_p0 runs them in cloud mode (the routine load part also needs Kafka); `test_compute_group_binding_isolation` is a docker suite that starts its own two-group cloud cluster. |
||
|
Mingyu Chen (Rayner)
|
2b8908b883 |
[feature](be) Count the changelog TTL cleanups in the BE metrics (#69868)
### What problem does this PR solve? Issue Number: #69907 Related PR: None Problem Summary: **In short.** The BEs clean up the changelog of a table by TTL, but `/metrics` cannot tell whether that cleanup runs, fails, or reclaims anything. This PR counts each cleanup on the BE as it finishes, in three new series: how many succeeded or failed, how many bytes of expired changelog they dropped, and when one last succeeded. **Background.** A table with `changelog.enable` keeps its changelog in a hidden changelog index, and `changelog.ttl_seconds` bounds how long the changelog is kept. The BEs remove what is older than that in a cumulative compaction of a tablet of that index (a changelog tablet): rowsets whose commit TSO is at or before the TTL cutoff are left out of the merge, so the output replaces them without their rows. Two submitters start these compactions, both as `CHANGELOG_COMPACTION`: the changelog TTL scanner, for tablets it finds holding expired rowsets, and the regular compaction producer; either compaction picks expired rowsets first. **The problem, and what it cost.** - No alert can watch the cleanup. The bvars `changelog_ttl_discovered`, `changelog_ttl_submitted` and `changelog_ttl_failed` are only on the brpc `/vars` page, and they count what the TTL scanner found and submitted, not what the compactions did: a cleanup that fails after it was submitted counts nowhere, and nothing counts the bytes removed. - The only changelog series in `/metrics`, `compaction_task_state_total{type="changelog"}`, is the number of changelog compactions running and pending. A cleanup that keeps failing lets the changelog grow without bound, and nothing on the BE says why. **How this PR fixes it.** - Three series, counted as a compaction that drops expired changelog ends, and registered at startup so they exist from 0: | Series | Type | Labels | Value | | --- | --- | --- | --- | | `doris_be_changelog_ttl_cleanup_total` | counter | `result`=success/failed | the compactions that drop expired changelog rowsets, counted as each ends | | `doris_be_changelog_ttl_reclaimed_bytes_total` | counter | | the total disk size of the expired rowsets the successful ones dropped | | `doris_be_changelog_ttl_last_success_timestamp_seconds` | gauge | | when one last succeeded, in seconds since the epoch | - A compaction is a cleanup when one of its inputs is expired changelog: at or below the visible version its TTL snapshot was taken at, with a commit TSO at or before the cutoff. That is exactly what `filter_changelog_ttl_rowsets` leaves out of the merge, and the two now ask one predicate, `Compaction::is_expired_changelog_rowset`, which `pick_expired_changelog_rowset` uses too. It is decided from the inputs rather than from what the merge filtered, so that a cleanup that fails before its merge, a cloud compaction that cannot refresh its tablet meta for example, is counted as failed. - Counted where the execution of the compaction ends: `CumulativeCompaction::execute_compact` in shared-nothing mode, `CloudCumulativeCompaction::record_compaction_success` and `record_compaction_failure` in cloud mode. A compaction that does not get to run has not started a cleanup and is not counted: one that finds the tablet under another cumulative compaction, whether in its preparation or as its execution takes the cumulative compaction lock again, and one whose job the meta service refuses. - In cloud mode the BE that runs the compaction counts it, so a query sums over the BEs; the shared storage is not measured. The dropped rowsets become stale rowsets, whose files the stale rowset sweep or the recycler deletes later; the bytes are counted when the rowsets leave the tablet. - The bvars stay as they are. **Results.** Before this PR, the `/metrics` of a BE has none of these series. After it, on a local shared-nothing cluster, a table with `changelog.ttl_seconds` = 1 and two rows inserted (regression `test_changelog_ttl_cleanup_metrics`): | | `cleanup_total{result="success"}` | `cleanup_total{result="failed"}` | `reclaimed_bytes_total` | `last_success_timestamp_seconds` | | --- | --- | --- | --- | --- | | BE started | 0 | 0 | 0 | 0 | | once the changelog rows have been cleaned up | 1 | 0 | 1112 | the time of the cleanup | The 1112 bytes are what the BE logs for that changelog compaction: `input_rowsets_total_size=1.09 KB`, `output_rowset_total_size=0`, `filtered_row_num=2`. **Classes, and how they call each other.** - `DorisMetrics`: the three series. - `Compaction`: `is_expired_changelog_rowset` (new), shared by `pick_expired_changelog_rowset` and `filter_changelog_ttl_rowsets`; `record_changelog_ttl_cleanup` (new) counts a cleanup from the inputs. - `CumulativeCompaction`, `CloudCumulativeCompaction`: call it as the execution ends. ``` CumulativeCompaction::execute_compact shared-nothing └─ CompactionMixin::execute_compact ─► merge: filter_changelog_ttl_rowsets ─┐ └─ record_changelog_ttl_cleanup(ok) │ is_expired_changelog_rowset CloudCumulativeCompaction::execute_compact cloud │ └─ CloudCompactionMixin::execute_compact ─► merge: same filter ─────────────┤ └─ record_compaction_success ─► record_changelog_ttl_cleanup(true) │ record_compaction_failure ─► record_changelog_ttl_cleanup(false) │ └─ DorisMetrics::changelog_ttl_* ◄─────────┘ (from the inputs) ``` Nothing else changes: no config, no thrift or proto change, and what the compactions pick and drop stays the same. |
||
|
Mingyu Chen (Rayner)
|
9dc9dc7566 |
[feature](ivm) Report how a refresh task refreshed an IVM MV, and count the refresh tasks on the Master (#69856)
### What problem does this PR solve? Issue Number: #69907 Related PR: None Problem Summary: **In short.** A refresh of an IVM materialized view can keep the MV up incrementally or quietly recompute it, and nothing tells the two apart: `tasks("type"="mv")` does not say which kinds of work a task committed, and `/metrics` has no IVM series at all. This PR makes each refresh task of an IVM MV report how it refreshed the MV (`IvmRefreshMethod` in `tasks()`) and why it failed or fell back, keeps a refresh summary on the MV for the IVM status table that follows, and counts the tasks on the Master in five new counters. **Background.** An IVM MV is refreshed by a task that runs one or more attempts: the incremental attempt (the delta of its base tables' streams, after rebuilding the MV partitions an invalidation marked), a partition rebuild, and a whole-MV rebuild. A request that may fall back (AUTO, INCREMENTAL FALLBACK, PARTITIONS FALLBACK) moves on to the next attempt when one gives up, for example to COMPLETE after the delta meets a MIN/MAX boundary. `tasks()` already reports `RefreshMode` (which partitions the task covered), `IvmFallbackReason` (why the incremental attempt gave up, or why a strict request failed) and `IvmRebuiltPartitions`. **The problem, and what it cost.** - An operator cannot tell a refresh that kept the MV up incrementally from one that recomputed it. `RefreshMode` says `COMPLETE` for any whole-MV scope, and `IvmFallbackReason` is set even when the attempt that replaced the delta wrote nothing, or when a strict request failed. - No alert can watch IVM refreshes. The asynchronous MV counters (`async_materialized_view_task_success_num`, `_failed_num`, `_skip_num`) cover every MV and say nothing about the method, the fallbacks, or why tasks fail. An MV that falls back to COMPLETE on every run, the expensive failure mode of IVM, goes unnoticed until the load of the cluster shows it. - A task that the full MV task queue turns away never runs, and nothing counts it. - The task history keeps only `max_persistence_task_count` tasks, so how many times in a row an MV has failed, or when it last fell back, cannot be read back from it. **How this PR fixes it.** - `MTMVTask` notes each batch that commits data (the delta, a partition rebuild, a whole-MV rebuild) and decides `IvmRefreshMethod` once, as the task publishes its result: `NONE`, `INCREMENTAL`, `PARTITION_REBUILD`, `COMPLETE`, or `MIXED` when more than one kind committed, typically the rebuild of invalidated partitions before the delta. `tasks("type"="mv")` gets the `IvmRefreshMethod` column, NULL while a task runs and for a task of an MV that is not an IVM MV. - A task falls back when the incremental attempt gives up and an attempt replacing it commits data. Not fallbacks: building the first baseline of an MV no refresh has succeeded on, the rebuild of invalidated partitions before the delta, an explicit COMPLETE or PARTITIONS request, and a fallback that commits nothing. - A failed task reports why: the `IvmFailureReason` a request that may not fall back failed with, or else the stage it failed in, `QUERY_UNUSABLE`, `PARTITION_SYNC_FAILED`, `EXECUTION_FAILED` or `OTHER`, told apart by where the failure was thrown rather than by its message. - The method, the failure reason and whether the task fell back are journaled with the task, as new Gson fields that an older FE ignores. The MV keeps an `IvmRefreshSummary`, updated from each task result in `MTMV.addTaskResult`, which the Master and the replay of the same journal record run alike, from what the record carries: consecutive failures, the latest success and failure (with the reason and the error message, cut to 512 characters), the latest method and run time, and the fallback count with the latest reason. A canceled task leaves it as it is. Nothing reads it in this PR; `information_schema.ivm_mv_status` and the per-MV series of a follow-up PR do. - Five counters, counted on the Master as each task of an IVM MV ends. Every label combination is registered up front, so each of the 61 series exists from 0: | Series | Type | Labels | Value | | --- | --- | --- | --- | | `doris_fe_ivm_refresh_tasks_total` | counter | `result`=success/no_change/failed/canceled, `method`=none/incremental/partition_rebuild/complete/mixed | refresh tasks that ended; `no_change` is a SUCCESS that committed nothing | | `doris_fe_ivm_refresh_duration_seconds_total` | counter | `method` | the run time, StartTime to FinishTime, of the tasks that succeeded or failed; a canceled task has no finish time | | `doris_fe_ivm_fallbacks_total` | counter | `reason`=the `IvmFailureReason` values, `incomplete_refresh_snapshot` | tasks that fell back | | `doris_fe_ivm_refresh_failures_total` | counter | `reason`=the `IvmFailureReason` values, the four stages | tasks that failed | | `doris_fe_ivm_schedule_rejected_total` | counter | | tasks the full MV task queue turned away | Like the other FE event counters they live in the registry of every FE and only the Master counts in them, so a query sums them over the FEs; a restart or a new Master starts them from 0. - `AbstractTask.onRejected`, a new hook that `TaskProcessor.addTask` calls when the executor queue rejects a task. Both the instant dispatch (`JobScheduler`) and the timer dispatch (`DispatchTaskHandler`) add tasks through it, and `MTMVTask` counts the rejection for an IVM MV. A timer-dispatched task that is turned away is canceled as before, so it also counts as `canceled`. **Results.** Before this PR, `tasks()` has no method column and `/metrics` has none of these series. After it, on a local cluster, one IVM MV refreshed through each case (regression `test_ivm_refresh_method_metrics`): | Refresh | `tasks()`: Status, IvmRefreshMethod, IvmFallbackReason | Counters that moved, besides the run time | | --- | --- | --- | | first AUTO refresh of a new MV | SUCCESS, COMPLETE, NULL | `tasks{success,complete}` | | INCREMENTAL after an insert | SUCCESS, INCREMENTAL, NULL | `tasks{success,incremental}` | | INCREMENTAL with nothing new | SUCCESS, NONE, NULL | `tasks{no_change,none}` | | INCREMENTAL after deleting the row that holds a group's minimum | FAILED, NONE, MIN_MAX_BOUNDARY_HIT | `tasks{failed,none}`, `failures{min_max_boundary_hit}` | | INCREMENTAL FALLBACK in the same state | SUCCESS, COMPLETE, MIN_MAX_BOUNDARY_HIT | `tasks{success,complete}`, `fallbacks{min_max_boundary_hit}` | | COMPLETE | SUCCESS, COMPLETE, NULL | `tasks{success,complete}` | | INCREMENTAL after truncating a base partition and inserting into another | SUCCESS, MIXED, NULL | `tasks{success,mixed}` | The first refresh of a new MV finds every partition still to be filled and goes straight to COMPLETE; it counts no fallback. The MV holds what its base table does after each case. **Classes, and how they call each other.** - `IvmRefreshMethod`, `IvmRefreshFailureStage` (`org.apache.doris.mtmv.ivm`): the methods, and the stages a failure is reported by. - `MTMVTask`: notes committed batches (`recordCommitted`), decides whether giving up the incremental attempt is a fallback (`givingUpTheIncrementalAttemptIsAFallback`), moves the failure stage along, settles the method and the failure reason in `after()`, and counts the task (`recordIvmRefreshMetrics`) and a rejection (`onRejected`). - `IvmRefreshSummary` (`org.apache.doris.mtmv.ivm`): persisted on `MTMV` and updated by `update(MTMVTask)` from `MTMV.addTaskResult`; `MTMV.hasSucceededIvmRefresh` tells the first baseline apart. - `MetricRepo`: registers the five counters. `DoubleCounterMetric` (new) keeps a total in seconds without rounding each addition. - `AbstractTask.onRejected` (new, empty by default), called by `TaskProcessor.addTask`. ``` MTMVTask.run() worker thread ├─ executeIvmAttempt │ ├─ rebuild invalidated partitions ──► recordCommitted(PARTITION_REBUILD) │ └─ delta (IvmIncrRefreshManager) ───► recordCommitted(INCREMENTAL) │ gave up ─► fallingBack = givingUpTheIncrementalAttemptIsAFallback() ├─ PARTITIONS / COMPLETE attempts ───────► recordCommitted(...) ivmFallback once fallingBack └─ failureStage: QUERY_UNUSABLE ─► PARTITION_SYNC_FAILED ─► EXECUTION_FAILED, or an IvmFailureReason onSuccess / onFail / cancel ─► after() ├─ ivmRefreshMethod, ivmFailureReason settled ├─ recordIvmRefreshMetrics() ─► MetricRepo.COUNTER_IVM_* Master only └─ Env.addMTMVTaskResult ─► MTMV.addTaskResult ─► IvmRefreshSummary.update(task) (journaled as ADD_TASK; the replay calls addTaskResult the same way) TaskProcessor.addTask ── queue full ─► task.onRejected() ─► MTMVTask: COUNTER_IVM_SCHEDULE_REJECTED ``` Nothing else changes: no thrift or proto change, no new config, and the existing columns and series stay as they are. |
||
|
Mryange
|
fcf143c1aa |
[fix](function) Align timezone offset parsing across FE and BE (#68979)
### What problem does this PR solve? Issue Number: N/A Related PR: #68865 Problem Summary: Timezone offsets can be accepted or rejected differently by FE constant folding and BE runtime CAST. For example, a DATETIME suffix of `-14:00` is valid under the literal policy but is rejected by the BE session-zone resolver. Session offsets such as `+14:30` and `-12:30` can bypass validation through the preloaded BE cache, while FE CONVERT_TZ applies different minute restrictions to bare and UTC/GMT-prefixed offsets. Root cause: these entry points duplicate offset validation and mix session and literal policies. Introduce a shared FE parser using the existing timezone aliases and upstream canonicalization helper, retain separate session/DATETIME/TIMESTAMPTZ policies, validate BE cache endpoints, and construct parsed literal offsets through TimezoneUtils. Strict and non-strict BE CAST use the same checks. Numeric TIMESTAMPTZ offsets are limited to Java's `[-18:00, +18:00]`, retaining historical minute and second offsets within that range. Explicit named and compact suffix recognition is also corrected. This does not add a BE Java alias table. Session alias dispatch is already fixed by #68865. System timezone database differences, ignored external lookup return values and DST rounding order are outside this change. ### Release note Fix inconsistent timezone offset validation in CAST and CONVERT_TZ, reject out-of-range cached session offsets, and correctly recognize explicit timezone suffixes. Numeric TIMESTAMPTZ offsets beyond +/-18:00 are now rejected. ### Check List (For Author) - Test - [x] Regression test - `test_timezone_parser_contract`: generated output through the runner and replayed successfully (1 suite, 0 failures). Compared 55 inputs across DATETIME/TIMESTAMPTZ and five execution paths: 550 results without semantic differences, allowing UTC display-format differences. - Upstream `test_session_time_zone_short_ids`: passed after rebase, covering scans, point queries and stream load. - [x] Unit Test - Earlier focused FE tests: 76 tests, 0 failures/errors, 4 existing skips; BE TimezoneUtilsTest: 5/5 passed. - After rebase: TimeUtilsTest, CoordinatorContextTest and TimeZoneParserTest: 18 tests passed. - [x] Manual test - Rebuilt BE and FE after rebase to master `2d053c83252`; both services report the matching version and Alive=true. ACT session FROM_UNIXTIME on a stored column matches Australia/Darwin without a BE alias table. - Validation limit: the final FE cleanup removing redundant normalization/default arguments and reusing getCanonicalTimeZone has not been rebuilt. The latest regression ran on the preceding build; CI must validate the final source. - clang-format, git diff --check and BE build hygiene passed. The earlier clang-tidy run reported pre-existing modernization diagnostics; it was not a clean clang-tidy pass. - Behavior changed: - [ ] No. - [x] Yes. Offset boundaries and explicit suffix parsing are consistent across the scoped FE/BE paths; invalid endpoints are rejected. - Does this need documentation? - [x] No. This aligns existing offset policies and documents the TIMESTAMPTZ boundary change in the release note. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label |
||
|
Mingyu Chen (Rayner)
|
599616cb0d |
[fix](metrics) Escape label values in the Prometheus output of FE (#69837)
### What problem does this PR solve? Issue Number: #69907 Related PR: None Problem Summary: **In short:** the FE writes metric label values into its Prometheus output without escaping them. A table name may contain a backslash, a double quote or a line feed, and in cloud mode the Master FE labels four per-table series with the table name, so a single such table leaves the Master's whole `/metrics` response outside the text format, and Prometheus keeps no sample of a scrape it cannot parse. This PR escapes every label value that is not a literal through one function, as the format requires. Ordinary names are written exactly as before. #### Background In the Prometheus text format a label value is written between double quotes, and a backslash, a double quote or a line feed inside it must be written as `\\`, `\"` and `\n`; no other character is escaped. A scrape whose response does not parse fails as a whole. With `enable_unicode_name_support`, which is on by default, a table name may contain all three characters: `FeNameFormat.UNICODE_TABLE_NAME_REGEX` only requires the last character not to be white space. A local cluster with default settings accepts tables named ``` `a"b` ```, `` `a\b` `` and one with a line feed in the middle. The names of databases, users, workload groups, jobs and catalogs keep their restricted patterns. #### The problem, and what it cost - `PrometheusMetricVisitor` concatenates label values as they are, in every place that writes one: the labels of registered metrics (`visit`), both kinds of histograms, the cloud per-table series (`visitCloudTableStats`), the workload group series and the JVM GC series. - In cloud mode the Master FE exports `doris_fe_table_data_size`, `doris_fe_table_rowset_count`, `doris_fe_table_segment_count` and `doris_fe_table_row_count` for up to `prom_output_table_metrics_limit` (10000) tables, labeled with `db_name` and `table_name`. One table whose name holds one of the three characters makes the response invalid, and every metric of the Master FE is lost while that table is exported. - The coming monitoring of table streams and IVM materialized views labels series with the names of materialized views and streams, which would run into the same problem in both modes. #### How this PR fixes it - `escapeLabelValue` writes a backslash, a double quote and a line feed as `\\`, `\"` and `\n` and leaves every other character alone, which is the full set the format requires. - Every label value that is not a literal goes through it: the two histogram paths and `visit`, the database and table names of the per-table series (escaped once per table, not once per series), the workload group name and the GC name. The JVM GC and workload group names cannot hold these characters today; they go through the same function so that no label value in the class is written raw. - A name without these characters comes back as the same string, so the output of ordinary names does not change, byte for byte, and costs no extra allocation (`String.replace` returns the string itself when nothing matches). - A null label value is written as `null`, as the concatenation without escaping wrote it. Label values can be null: a table warm-up job has no source cluster, so the series of its start and finish times carry a null `src_cluster_name`. #### Results What the cloud per-table series is written as for each table name, and what `promtool check metrics` (Prometheus v3.2.0) says about it: | Table name | Before this PR | promtool | After this PR | promtool | | --- | --- | --- | --- | --- | | `t1` | `table_name="t1"` | OK | `table_name="t1"` | OK | | `a"b` | `table_name="a"b"` | `unexpected end of label value "a"` | `table_name="a\"b"` | OK | | `a\b` | `table_name="a\b"` | `invalid escape sequence '\b'` | `table_name="a\\b"` | OK | | `a\` | `table_name="a\"` | `label value "a\"} 100" contains unescaped new-line` | `table_name="a\\"` | OK | | `a`, line feed, `b` | the sample split over two lines | `label value "a" contains unescaped new-line` | `table_name="a\nb"` | OK | A parser that follows the format turns each escaped value back into the table name itself. #### Classes, and how they call each other - `PrometheusMetricVisitor`: the new private `escapeLabelValue`, called wherever a label value is written. - `MetricsTest`: `testPrometheusVisitorEscapesLabelValues` (a gauge with an escaped and a plain label, and both kinds of histograms), `testCloudTableStatsEscapeTableName` (the four per-table series, with a mocked `CloudTabletStatMgr`) and `testPrometheusVisitorWritesNullLabelValue` (a gauge and a labeled histogram with a null label value). ``` MetricsAction (/metrics) -> MetricRepo.getMetric(PrometheusMetricVisitor) |-- visitJvm GC name -> escapeLabelValue |-- DORIS_METRIC_REGISTER.accept -> visit labels -> escapeLabelValue |-- visitHistograms -> visitHistogram (both) labels -> escapeLabelValue |-- visitCloudTableStats db / table name -> escapeLabelValue `-- visitWorkloadGroup group name -> escapeLabelValue ``` Not changed: the JSON output (`type=json`), which builds its own strings and would need JSON escaping instead; the core output (`type=core`), which writes no labels; the metric names and the `# HELP` texts, which are literals. |
||
 morrySnow
|
0fe5cc1e31 |
[fix](changelog) Handle disabled TSO in the system table smoke test (#69824)
The system-table smoke test queries every information_schema table. When experimental_enable_feature_changelog is false, querying tso_status intentionally reports "TSO feature is disabled", stopping the suite before the remaining metadata tables are checked. Assert that expected error for the disabled configuration and retain the regular query when enabled. |
||
|
Calvin Kirs
|
f81660bf5f | [chore](github)Update the list of collaborators (#69972) | ||
 Xin Liao
|
c36a40c67b | [fix](regression) Fix test_ttl_lru_evict and test_stale_rowset in cloud file cache (#69847) | ||
 Gabriel
|
49bfbdecff |
[fix](fe) Avoid table handle lookups for external views (#69946)
### What problem does this PR solve? Related PR: #69836 Selecting an external Iceberg REST view can fail during query binding with `BadRequestException: ... is not an Iceberg compatible table`, even when the catalog supports reading the view definition. `BindRelation` loads statement snapshots before expanding the view SQL. The snapshot loader calls `PluginDrivenExternalTable.resolveConnectorTableHandle`, which unconditionally asks the connector for a table handle using the view name. A REST catalog can reject that table request with HTTP 400 instead of reporting a missing table. The Iceberg connector only converts `NoSuchTableException` to an empty handle, so the error escapes before the existing view-existence fallback can run. Resolve the object's view classification before requesting a table handle. Known views return no table handle and retain the live view-existence check, so a view deleted outside Doris still invalidates its cached lookup. Snapshot loading can then return an empty view pin and continue to view expansion; the referenced base tables keep their normal snapshot handling. Ordinary table handles and table-load errors retain their existing behavior. This addresses the shared handle-resolution boundary, including callers outside the snapshot loader. #69836 proposes skipping the snapshot-loader entry instead. ### Release note Fix external view queries failing when an Iceberg REST catalog rejects table metadata requests for view names. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test - [ ] No need to test or manual test. - Behavior changed: - [ ] No. - [x] Yes. External views no longer issue a table-handle request during resolution. - Does this need documentation? - [x] No. - [ ] Yes. Validation: - `run-fe-ut.sh --run` with `PluginDrivenExternalViewHandleTest`, `PluginDrivenExternalTableTest`, `PluginDrivenMvccExternalTableTest`, and `PluginDrivenMissingTableTest`: **164 tests passed**, with no failures, errors, or skips. - Before the fix, both view cases failed with the reproduced table-endpoint error; the ordinary table cases passed. - `mvn checkstyle:check -pl fe-core`: **0 violations**. - `git diff --check` passed. No live Databricks integration test was run. |
||
|
shuke
|
44dfa1596f |
[fix](ci) Preserve early review failure notifications and causes (#69469)
### What problem does this PR solve? Related PR: #69386 Problem Summary: When the pending-status API call fails before repository checkout, the failure notification runs outside a Git checkout. Without an explicit repository, `gh pr comment` fails with `fatal: not a git repository`; both the notification and terminal failure step also misidentify the skipped context preparation step as the cause. This occurred in [run 38864234303](https://github.com/apache/doris/actions/runs/38864234303/job/116853272967). Pass `--repo` explicitly and report failed/cancelled pending-status and checkout steps by name. Distinguish actual context failure/cancellation from skipped or unstarted context in both reporting paths, preserving the existing auth/usage-limit recovery messages and failing exit status. |
||
|
shuke
|
cb41339ffd |
[fix](ci) Retry transient review GET failures within bounded budgets (#69510)
Related PR: #69469 Problem Summary: A single transient GitHub response currently aborts the review pipeline. [Input resolution for #69322](https://github.com/apache/doris/actions/runs/39794013905/job/119876748768) stopped on a GET 503 despite `Retry-After: 2`; [aggregate resolution for #69438](https://github.com/apache/doris/actions/runs/39760934956/job/119764632659) stopped on a paginated GET 504 after emitting partial output. The shared helper now retries recognized body-free REST GETs on 502/503/504, for at most three attempts. Each attempt has a 30-second limit; attempts and waits share a 90-second budget or the caller's shorter timeout. It respects Retry-After, records every failed attempt, discards failed pagination output, and returns only a successful attempt's complete output. Exhaustion still fails. Writes, GraphQL, unknown argument forms, other HTTP errors, transport/timeouts and JSON parsing errors are not retried. Input resolution can also fail before publishing its PR-number output. Failure notices now fall back to the authorized trigger's numeric PR identity, explicitly select the repository, and name the input-resolution failure in both the notice and terminal error. This fallback does not permit status publication for an unresolved head/base. This change can merge independently. It includes the same explicit `--repo` argument as #69469; a local combination with #69469 and #69504 merged cleanly and passed the macOS suite. |
||
|
Jack
|
c31cd7f1cb |
[improvement](be) Read SNII dictionary keys in place in walks and block opens (#69887)
### What problem does this PR solve? Issue Number: None Related PR: #69600 Problem Summary: A regexp without a literal prefix, such as `output MATCH_REGEXP 'transi.*'`, expands by walking every term of the SNII dictionary and testing each key against the pattern. On a 99M-row table, about 40% of the SNII stack samples of that query were spent decoding dictionary entries, twice as much as the pattern match: - `visit_prefix_range` and `visit_term_range` decoded every key into a fresh `DictEntry`: the whole struct reset, a new `std::string`, and a `Status` for every varint. - `DictBlockReader::open` fully decoded every anchor of the block, about 1 entry in 16, only to get its key. Every block open pays for this, exact term lookups included. Both now read an entry's length and front-coded key on the raw cursor. A walk rebuilds the key in one buffer it keeps across entries and a rejected key skips its body by the entry length; `open()` reads only the keys of the anchors. An entry that does not read cleanly goes to the checked decoders, which report what is wrong with it as before, and a walk still decodes the body of every entry it accepts with the checked decoder. The on-disk format and the write path do not change. `open()` no longer decodes the bodies of the anchors. The block's crc32c, verified just before, covers them, and they are checked when they are decoded, like the body of every other entry. | Query (99M rows) | Hot, new/previous | Cold, new/previous | |---|---|---| | regexp `'transi.*'` | 0.69 | 0.77 | | regexp that matches no term | 0.67 | | | regexp `'.*ing'` | 0.72 | | | regexp `'e.*'` (stops at the expansion limit) | 0.90 | | | phrase query, term lookup cache off | 0.78 | | | phrase query, term lookup cache on | 0.99 | 1.01 | | AND of two terms | 0.99 | 1.01 | | the other 22 queries | around 1.0 (reruns of those above 1.05: 0.94-1.03) | | Hot: both versions interleaved in one BE process. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. The build also had #69879 (the term lookup cache) and #69882. With that cache off, which is master today, queries open dictionary blocks more often: the phrase query gains as the 0.78 row shows, and the five other lookup-heavy queries measured that way stayed at 0.96-1.04. |
||
|
shuke
|
5d1bdea509 |
[fix](ci) Resume reviews with Astra after five capacity failures (#69953)
### What problem does this PR solve? Reviews can exhaust all seven attempts while the selected model remains at capacity. After five capacity failures, this change resumes the same review session with `gpt-6-astra` for the remaining two attempts, preserving the existing five-minute backoff, shared deadline, target checks, and final-review verification. The workflow opts in through `--capacity-fallback-model`. Account/model admission fallback remains separate. Recovery instructions preserve completed findings and the cumulative round count, and direct unfinished child-agent work to the selected model. Scope: three files; 17 production lines and 91 test lines added. |
||
|
Jack
|
62cf8c7588 |
[improvement](be) Read SNII phrase-prefix batches on the calling thread when they hit the page cache (#69882)
### What problem does this PR solve? Issue Number: None Related PR: #69648 Problem Summary: #69648 reads a SNII batch on the calling thread when the index file is local, the batch has at most `snii_inline_read_max_segments` (4) physical segments and 256 KiB, and the moving average of recent reads is under 20 us, i.e. they hit the page cache. A phrase prefix reads the postings of every term its prefix expands to, in batches of 5 to 16 segments that stay well under 256 KiB, so those still went segment by segment through the shared buffered-reader prefetch pool: on a hot 99M-row table, about a fifth of the SNII time of such a query was spent on the pool's lock and latch (`ThreadPool::do_submit`, `CountDownLatch::wait` in `read_batch`). The default of `snii_inline_read_max_segments` becomes 16, which covers those batches, and two changes keep larger inline batches safe when the data is not in the page cache: - A batch read inline hands the segments it has left to the pool as soon as one of its reads is slow. When the average is low but this batch's data is not cached, the batch pays one read on the calling thread and overlaps the rest. A single segment left is still read inline. - A read counts by how much longer it took than copying its bytes out of the page cache (at 4 bytes/ns, the low end for one thread), for both the average and the slow test. A page-cache hit of 64-256 KiB spends tens of microseconds on the copy alone, which pushed the average over 20 us and sent about 5% of a phrase query's small batches to the pool. On the 99M-row table, the phrase prefix's batches of 5-16 segments are now all read inline when hot, and the small batches of a phrase query that still went to the pool dropped by 77%. In a cold run (BE restarted, index files evicted from the page cache), the average was at least 40 us whenever such a batch was decided, so they all went to the pool, as before. | Query (99M rows) | Hot, new/previous rule | Cold, new/previous rule | |---|---|---| | phrase prefix 'retry budget dep' | 0.79 / 0.85 / 0.96 (three runs) | 0.96 | | other phrase queries | 0.87-0.95 | 1.01 (A6) | | the other 19 queries | 0.98-1.03 | | Hot: both rules interleaved in one BE process. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. |
||
 lihangyu
|
5af2df3231 | [fix](fe) Lock table metadata while constructing UPDATE plans (#69880) | ||
 daidai
|
ac15da94a2 |
[fix](fe) Load Iceberg views without engine-name metadata (#69829)
### What problem does this PR solve?
Allow missing/empty `engine-name` in Iceberg views. Preserve existing
engine-based SQL selection and only use the Doris/SDK fallback when the
field has no value.
### Release note
Support Iceberg views without `engine-name` while retaining existing SQL
selection when it is present.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
- Behavior changed:
- [ ] No.
- [x] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
 bobhan1
|
b2164ff1ba |
[fix](cloud) Avoid lock inversion in delete bitmap checks (#69892)
### What problem does this PR solve?
Issue Number: N/A
Problem Summary:
With background delete bitmap checks enabled, the checker holds the
delete bitmap read lock while `tablet_schema()` acquires the tablet
header lock. Compaction takes these locks in the opposite order, causing
a deadlock on the same tablet. The checker also runs inside
`TabletMap::traverse()`, so its outer map mutex blocks unrelated tablet
initialization, Rowsets scans, synchronization, and compaction
scheduling.
Read the sequence-column flag before traversing the delete bitmap, and
run checks outside the map mutex by reusing the existing tablet
snapshot. Preserve the active/stale rowset version checks for sequence
and non-sequence tables.
### Release note
Fix BE deadlocks and cascading write and metadata stalls when background
delete bitmap checks are enabled.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test
- [ ] No need to test
- Behavior changed:
- [ ] No.
- [x] Yes. Delete bitmap checks avoid the reversed lock order and no
longer hold the tablet map mutex while checking individual tablets.
- Does this need documentation?
- [x] No.
- [ ] Yes.
Validation: 72 ASAN unit tests passed, including five new instances
covering lock ordering, an empty compaction bitmap merge, unrelated
tablet map operations, and sequence/non-sequence version semantics.
`build-support/check-format.sh`, `build-support/check-build-hygiene.sh`,
and `git diff --check` passed.
```bash
./run-be-ut.sh --run --filter='SequenceModes/CloudTabletDeleteBitmapCheckTest.*:CloudTabletMgrTest.*:CloudTabletDeleteBitmapTest.*:PrefillModes/CloudTabletDeleteBitmapPrefillTest.*:TabletMetaTest.*:CloudCompactionTest.*' -j100
```
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
 deardeng
|
25a8c10334 |
[improvement](tablet) Source active tablet stats from BE reports (#67621)
Problem Summary: FE tracks tablet activity with per-tablet sliding windows, consuming substantial heap and retaining stale entries on followers and observers. Use BE query and load counters to report bounded sets of active tablets. Replace FE sliding windows and asynchronous recording with per-BE report snapshots, and rank query and load activity separately for cloud balancing. Also fix inflated tablet counts when cloud tablet reports retry. ### Release note Reduce FE memory usage for tablet activity tracking. Rename `WindowAccessCount` to `AccessCount`, reflecting activity over a BE report interval. Tablets outside the reported top-N show zero activity. |
||
|
Gabriel
|
378404eeb1 |
[fix](thirdparty) Upgrade Brotli to 1.0.10 for CVE-2026-111497 (#69906)
### What problem does this PR solve? Issue Number: None Related PR: None Doris bundles Brotli 1.0.9, which is affected by [CVE-2026-111497](https://github.com/google/brotli/releases/tag/v1.0.10). The upstream decoder can mistake an invalid block switch for insufficient input, allowing an out-of-bounds write. Upgrade the bundled dependency to the upstream 1.0.10 security release and update its archive checksum. Align Arrow 24's internal Brotli version and SHA256 pin with the overridden archive; changing the Doris manifest alone causes Arrow's ExternalProject verification to fail. Apply that metadata patch with a separate marker so previously patched Arrow source trees also receive it. The fix requires rebuilding the relevant thirdparty libraries and relinking BE because the decoder is statically linked. ### Release note Upgrade bundled Brotli to 1.0.10 to include the upstream fix for CVE-2026-111497. ### Check List (For Author) - Test: Manual validation passed: - Official source archive MD5 and SHA256 verification; the source contains the upstream block-switch error fix. - Bash syntax and `git diff --check`. - Existing thirdparty download checksum and fallback tests. - Actual Brotli/Arrow companion download and extraction; patch application to previously patched Arrow sources and repeated invocation. - Patch dry-run against pristine Arrow 24 metadata. - Arrow 24's Brotli ExternalProject verification, build, and static-library installation in an isolated CMake harness. Confirmed the old SHA256 pin fails with the new archive before the patch, then passes after it. - Brotli 1.0.10 build and all 14 available upstream round-trip/compatibility tests with AddressSanitizer and UndefinedBehaviorSanitizer. - Full Doris and Arrow builds were not run locally; CI validation is requested. - Behavior changed: Yes. Builds use the patched Brotli decoder and updated Arrow dependency metadata. - Does this need documentation: No. This updates a bundled security dependency. |
||
|
shuke
|
112ca45819 |
[fix](ci) Add review findings to one draft before publishing (#69447)
### What problem does this PR solve? Related PR: #68499 The review runner's bulk REST create-review request rejects five valid inline findings on #68499 with HTTP 422 (`Position could not be resolved`; line/side also fails). In a controlled experiment at the same head/base, all five original findings were accepted when added sequentially to **one PENDING review** through GraphQL. The experimental draft was deleted afterward. The exact GitHub internal diff limit is not established. For reviews with inline findings, this change creates one draft, adds one thread per request, verifies the complete comment set, then publishes once. It converts the existing path/position/body input to line/side using the authoritative local PR diff. Summary-only reviews keep their current submission path. The invocation marker, exclusive submission intent, frozen head/base checks, priority/convergence gate, and exact final readback remain required. The helper rechecks competing bot reviews before publication. Add failures trigger best-effort cleanup only of the exactly owned pending draft; uncertain writes are never retried. `review-draft.json` is included in failure artifacts, and GraphQL errors in HTTP 200 responses retain HTTP diagnostics. Ambiguous publication, unavailable cleanup, and hard termination boundaries are documented. Live formal publication and the GitHub Actions identity still need rollout validation. The existing experiment validates draft construction, not end-to-end CI delivery. |
||
 Jerry Hu
|
9177f0cbd6 |
[refactor](be) Separate S3 spill file I/O from common readers and writers (#69781)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: `SpillFileWriter` and `SpillFileReader` mixed their common spill-part lifecycle and serialization/deserialization with object-storage-specific policies. The writer included S3 upload queue admission, cancellation checks, timing, and remote accounting; the reader included S3 footer prefetch, adjacent-block coalescing, and remote byte/request accounting. This change introduces `SpillS3FileWriter` and `SpillS3FileReader`, selected by the spill file factory for remote files. The common classes retain part lifecycle, footer format and validation, block serialization/deserialization, part rotation/traversal, and seek handling. Narrow virtual hooks preserve the S3 policies, failed-part reconciliation, query-buffer cap, and local/remote metric separation. The derived writer closes before base destruction so implicit close still dispatches its S3 hooks; reader cleanup remains entirely in the base class and does not require derived hooks. Public writer/reader APIs and the spill file format remain unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test / Build and static checks - `BUILD_TYPE=ASAN ./build.sh --be -j16` passed on the updated reader/writer head. - `./run-be-ut.sh -j16 --run --filter='SpillFileS3Test.*:SpillFileTest.*:SpillRepartitionerTest.*:SpillSortSourceOperatorTest.*'` passed (109 tests from 4 suites). - New coverage checks S3 reader factory selection and empty-file handling, the query-buffer cap on coalesced GET lengths, and local reader type/accounting isolation. Existing suites exercise multi-part roundtrips, seeking, footer overflow, oversized blocks, and disabled coalescing. - BE formatting/check-format, build hygiene, and `git diff --check` passed. - Changed-line clang-tidy passed for all 7 files in the reader follow-up using the UT compilation database and the Clang resource directory. - S3 unit tests use an in-memory object-storage mock. SQL regression and real-object-store E2E were not rerun for the reader follow-up; these results are not remote CI results. - Behavior changed: No; this is an internal responsibility split preserving the existing local and S3 spill behavior. - Does this need documentation: No |
||
 HappenLee
|
efa20d2b83 |
[improvement](be) Accelerate relative-day weekday lookup (#69897)
### What problem does this PR solve?
`next_day` and `previous_day` copy and uppercase the weekday string and
search an `unordered_map` for every row, including repeated constant
weekday arguments.
Use a specialized, compile-time-checked 32-slot table for the 21
supported English aliases. Fold ASCII case during bounded word
comparisons rather than materializing a normalized string. Verify the
complete key after selecting a slot, so arbitrary input that collides
with an alias is rejected. Parse constant weekday arguments once per
block, preserving NULL propagation before invalid-weekday errors.
Keep the parser and the shared `FunctionRelativeDay` implementation in
the anonymous namespace of `function_date_or_datetime_computation.cpp`,
their only consumer. Both registered functions reuse the same template.
No standalone parser source/header, new dependency or compiler
adaptation remains. The SQL signatures, return types, accepted aliases
and error behavior are unchanged.
#### Performance: original implementation vs specialized implementation
**The overall before/after comparison is the original `unordered_map`
implementation with per-row parsing versus the specialized weekday
lookup and once-per-block constant parsing delivered by this PR.**
End-to-end SQL improves by **1.68–2.59x**, and the complete registered
functions improve by **1.89–2.85x** in the saved A/B measurements below.
**Measurement provenance:** these are the saved original feature-branch
A/B results, not a new run on the latest PR head. The final revision
retains the original specialized parser and function execution logic,
with the implementation now local to the existing function CPP. The SQL
A/B comparison was not rerun after the cherry-pick and final relocation.
Current-head functional/build validation is listed separately below.
Both variants were measured on an x86_64 Intel Xeon Platinum 8457C with
Clang 21.1.8, C++20, RELEASE/AVX2. The nullable scenarios have 25% NULL
dates. These are synthetic local workloads; ARM performance was not
measured.
**End-to-end SQL — milliseconds, lower is better.** One BE, one
pipeline, one bucket, 10 million materialized rows; the same FE, data
and settings for both variants, SQL/query caches disabled, two warmups
followed by seven measurements, reporting the median.
| Function | Scenario | Original (ms) | Specialized (ms) | Speedup |
|---|---|---:|---:|---:|
| `next_day` | Varying weekday | 528.7 | 285.1 | 1.85x |
| `next_day` | Constant weekday | 664.7 | 256.5 | 2.59x |
| `next_day` | Nullable date | 441.6 | 256.2 | 1.72x |
| `previous_day` | Varying weekday | 524.1 | 279.7 | 1.87x |
| `previous_day` | Constant weekday | 649.6 | 254.4 | 2.55x |
| `previous_day` | Nullable date | 435.1 | 258.6 | 1.68x |
All six SQL result checksums match.
**Complete registered-function benchmark — nanoseconds per input row,
lower is better.** Batches of 4096 rows, seven repetitions, CPU-time
medians. Includes function execution, result allocation, NULL handling
and date calculation.
| Function | Scenario | Original (ns/row) | Specialized (ns/row) |
Speedup |
|---|---|---:|---:|---:|
| `next_day` | Varying weekday | 49.06 | 23.86 | 2.06x |
| `next_day` | Constant weekday | 59.76 | 21.53 | 2.78x |
| `next_day` | Nullable date | 35.57 | 18.84 | 1.89x |
| `previous_day` | Varying weekday | 45.86 | 22.66 | 2.02x |
| `previous_day` | Constant weekday | 59.44 | 20.85 | 2.85x |
| `previous_day` | Nullable date | 35.54 | 18.22 | 1.95x |
**Isolated weekday parsing — nanoseconds per lookup, lower is better.**
Measures only string-to-weekday parsing, excluding date calculation,
result columns and query execution.
| Input | Original (ns/lookup) | Specialized (ns/lookup) | Speedup |
|---|---:|---:|---:|
| Valid aliases | 40.51 | 2.66 | 15.24x |
| Invalid names | 38.83 | 1.81 | 21.47x |
Parser-only speedups must not be interpreted as complete-function or SQL
speedups. Constant-weekday results include both faster parsing and
eliminating repeated parsing within the block.
### Release note
Improve `next_day` and `previous_day` weekday lookup and avoid repeated
parsing of constant weekday arguments while preserving SQL results and
error behavior.
### Check List (For Author)
- Test
- [x] Unit Test: seven focused ASan tests via `run-be-ut.sh`, covering
every ASCII case combination of all aliases, arbitrary high bytes,
embedded NULs, invalid names, constant/nullable arguments, NULL/error
precedence and timestamp precision. Parser tests exercise the registered
SQL functions.
- [x] Manual test: standard RELEASE BE and focused benchmark builds;
full-function benchmarks; 123210 ASan/UBSan oracle cases against an
exact extraction of the CPP-local helper, including exact-length
allocations and guard pages.
- [x] clang-format 16.0.6, production/test clang-tidy, whitespace, shell
syntax and build hygiene.
- ASan unit tests use `GLIBC_COMPATIBILITY=OFF` for existing toolchain
duplicate compatibility symbols; the production build keeps
compatibility enabled.
- Existing SQL regression cases and runner-generated output remain in
the PR. They passed in the original dedicated-parser work; the full SQL
regression suite was not rerun for this revision.
- Behavior changed:
- [x] No.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
|
Gabriel
|
49da8785c8 |
[refactor](scan) Remove legacy file scanner and deprecate rollout option (#69723)
### What problem does this PR solve?
File scans maintained both the legacy scanner/readers and the V2 reader
stack. Route query scans, schema inference, file loads and external
row-ID fetches through FileScannerV2, migrate standalone Push and
Iceberg delete-file consumers, and remove the retired implementation and
tests.
- Keep enable_file_scanner_v2 as a deprecated no-op, with the existing
optional Thrift field emitted/defaulted to true.
- Preserve ORC session/adaptive batch caps, stripe dictionary rejection,
lazy predicate-column reuse and sequential CSV/TEXT/JSON S3 prefetch.
- Filter matching ORC dictionary IDs before string expansion in lazy,
eager and predicate-only scans. Retain residual predicates, NULL
semantics, pruning fences, physical column identity and physical row
positions.
- Restore ORC serialized-tail caching using the existing V1
FileMetaCache key policy, including its file-size fallback when mtime is
absent. Keep V2 payload types separate. ORC footer counters have
format-qualified names and publish once across repeated close; native
Parquet cache identity remains unchanged.
- Bound standalone position-delete reads to the native Parquet batch
limit. Broker source projections follow validated src_slot_ids order,
and malformed ORC dictionary offsets return CORRUPTION.
- Retain eight active V2 position-delete system-table regressions and
the operator shared-cache/shard-limit tests in maintained suites.
- Preserve all six Hive nested STRUCT/ARRAY/MAP access-path scenarios in
V2 across ORC/Parquet and name/position mapping, including separate
predicate-column IDs and complete-versus-projected row comparisons.
- Preserve active-file error context and the upstream Arrow column/type
context without changing original validation status codes.
Related PR: #69668
The supported rolling-upgrade direction remains BE first, then FE.
Old-FE/new-BE requests without semantic COUNT arguments retain their
ordinary-read fallback. New-FE/old-BE nullable COUNT is outside that
documented upgrade direction; the deprecated scanner option does not
provide a legacy routing gate.
The branch resolves the modify/delete conflict with master at
|
||
 924060929
|
25968c92bf |
[test](fe) Stabilize scan termination and retry profile assertions (#69894)
### What problem does this PR solve? Issue Number: N/A Problem Summary: Two FE unit tests depend on asynchronous work without owning its complete lifecycle: - `SplitProducerTest.readThatOutlivesItsStatementLoadsNothingIntoTheClosedScope` waits for the source-close signal and immediately checks the producer completion count. `SplitProducer.finish` invokes the completion callback after closing the source, so the assertion can run before that callback. Wait for the assignment termination future, which completes after both cleanup and the callback. - `StmtExecutorTest.testSuccessfulRetryRendersMergedProfileAndActualRows` registers its profile with the global background `ProfileManager` while also writing and reading the profile synchronously. Use a real manager whose daemon is not started, scoped to this test, so automatic spill, memory release and archival cannot interfere with the temporary storage lifecycle. Preserve the real persistence path and all merged-profile and actual-row assertions, and include the readback content in the failing assertion. The tests originated in #69718 and #68658 respectively. These test-only changes are independent of the Coordinator refactoring in #69830. The exact CI interleaving was not reproduced locally; both original tests also passed local repeated execution. No production behavior changes. ### Release note None ### Check List (For Author) - Test: On this independent master-based branch, `./run-fe-ut.sh --run 'SplitProducerTest,StmtExecutorTest'` passed all 81 cases (26 + 55). Before extraction, both target cases also passed 50 repetitions each (179 executions across both classes); FE Checkstyle passed with 0 violations. - Behavior changed: No - Does this need documentation: No |
||
 Mingyu Chen (Rayner)andClaude Opus 5.5
|
a9253e979d |
[fix](iceberg) Decode Parquet timestamptz columns as instants by the Iceberg table type (#69845)
### What problem does this PR solve? Issue Number: None Related PR: #69711 Problem Summary: **In short.** An Iceberg `timestamptz` column stored as Parquet INT96, the encoding Spark and Hive use for timestamps, reads shifted by the session time zone's offset in any non-UTC session: in Asia/Shanghai, 2024-01-01T00:00:00Z reads as `2024-01-01 00:00:00+08:00`, eight hours early, while Iceberg itself reads `2024-01-01 08:00:00+08:00`. Equality deletes on such a key miss their rows for the same reason. This PR has the FE mark each `timestamptz` field of the scan's schema dictionary, and FileScannerV2 then decodes the matching Parquet columns of data files and equality-delete files as instants, which is how Iceberg reads them. It came up in the review of #69711. **Background.** - Parquet INT96 has no logical annotation, so a reader cannot tell from the file whether an INT96 column holds an instant or a wall clock. Spark writes its `timestamp` type (an instant) as INT96 by default, and `add_files`, `migrate` and `snapshot` attach such files to Iceberg tables unchanged, as `timestamptz` columns. - Iceberg does not look at the file annotation at all: its Parquet readers choose the timestamp reader from the table type, so a `timestamptz` field reads INT96, and INT64 with or without the UTC annotation, as an instant. - The FE sends `parquet_timestamp_semantics_version = 1` for every file scan. Parquet scans therefore always use FileScannerV2, and INT96 decodes as a UTC wall clock (`DATETIMEV2`) unless the table format supplies the semantics. Paimon has the same ambiguity (TIMESTAMP and TIMESTAMP_LTZ share INT96) and solves it with `timestamp_is_adjusted_to_utc` on each field of its schema dictionary, which `PaimonReader` applies to the file schema. The Iceberg FE never set that field. **The problem, and what it cost.** For a `timestamptz` column, the projection casts the INT96 wall clock to `TIMESTAMPTZ` in the session time zone, so the instant moves by the session offset. Reproduced on master with an INT96 file attached like `add_files` and an Iceberg-written INT64 file holding the same instants, both in one table: | Session | Rows | Iceberg Java | Before | After | |---|---|---|---|---| | UTC | INT96 and INT64 | 2024-01-01 00:00:00 UTC | correct | correct | | Asia/Shanghai | INT64 (written by Iceberg) | `2024-01-01 08:00:00+08:00` | correct | correct | | Asia/Shanghai | INT96 (written by Spark) | `2024-01-01 08:00:00+08:00` | `2024-01-01 00:00:00+08:00`, 8 hours early | `2024-01-01 08:00:00+08:00` | The result is the same with field ids in the file, with only the table's name mapping, whatever the catalog's `enable.mapping.timestamp_tz` says (master always maps `timestamptz` to TIMESTAMPTZ), and with `enable_file_scanner_v2` on or off. The same shift breaks equality deletes, which compare the projected values: | Equality delete on a `timestamptz` key, non-UTC session | Before | After | |---|---|---| | INT96 data key, Iceberg-written tombstone of the same instant | the tombstone misses the row | the row is deleted | Wrong values and missed or extra deletes come back without any error. **How this PR fixes it.** - FE: `IcebergSchemaUtils.buildField` sets `timestamp_is_adjusted_to_utc = true` on every `timestamptz` field, at every nesting level; the dictionary also carries the hidden equality-delete keys. `timestamp` fields stay unmarked: marking them `false` would make BE read the Parquet statistics of an adjusted-UTC INT64 file as UTC wall clocks while its rows still decode as session wall clocks, so row-group pruning could drop matching rows. - BE, data files: `IcebergTableReader::annotate_file_schema` copies the marker onto the Parquet file column that `TableColumnMapper` will select for each field, using the mapper's own matchers (`find_column_by_field_id` / `find_column_by_name`) after the positional fallback ids are stamped, so field ids, the table's name mapping and positional ids all work. Like the mapper, it accepts an ID-less wrapper at every depth when `allow_idless_complex_wrapper_projection` is on. It then rewrites the column type with the helper `PaimonReader` already used, moved to `schema_history_util` as `apply_file_timestamp_semantics`. The mapping, the scan request and `ParquetReader` all see `TIMESTAMPTZ`, and INT96 decodes as an instant. Only a `DATETIMEV2` or `TIMESTAMPTZ` leaf takes the marker; a `TIMESTAMP_NS` leaf keeps its nanoseconds. Columns without a marked leaf keep the types the file reader reported, and a plan from an older FE carries no marker and reads as before. - BE, equality deletes: three kinds of column are read without a `ColumnMapping`: a hidden key column, the parent struct that is read for the NULLs of a nested key the data file lacks, and the key columns of an equality-delete file. Their projections now carry the markers of the annotated file column (`equality_delete_root_projection`, built on `attach_timestamp_semantics`, which the mapper now exports). Without them Parquet would decode an INT96 leaf as `DATETIMEV2` under a `TIMESTAMPTZ` file schema, and the read would fail its type check. That holds even when the query projects the column, because reading it as a predicate column replaces the projection its mapping marked. Equality-delete files get the same annotation, so tombstones and data keys of a `timestamptz` field are both instants. The `DATETIMEV2` / `TIMESTAMPTZ` comparison rule from #69711 stays for plans from older FEs. - Refactor: `find_current_schema` and `build_schema_identity_from_external_field` (now also carrying the marker) move to `schema_history_util`; `TableReader`'s four copies of the current-schema lookup use the helper. **Results.** - FE unit test `IcebergSchemaUtilsTest#timestamptzFieldsCarryInstantMarkerAtEveryLevel`: top-level, struct, list and map `timestamptz` fields are marked; `timestamp` and `timestamp_ns` fields are not, under both values of the mapping flag. - 5 new BE unit tests in `iceberg_reader_test.cpp`, all failing without the BE change: the file-schema annotation (field ids at every level, `TIMESTAMP_NS` untouched, ORC untouched); INT96 `timestamptz` in Asia/Shanghai with field ids, name mapping and positional ids, with and without the marker; INT96 inside a struct and a list; an INT96 equality key against an Iceberg tombstone, hidden or projected; and the DST fall-back above. Leaving the delete files unannotated fails the DST case (the second tombstone deletes the first row); projecting a hidden key or a tombstone without its marker fails both equality-delete cases with a read error; marking a `TIMESTAMP_NS` leaf fails the annotation case. With neither data nor delete files annotated (the reader before this PR), the INT96 key deletes nothing and the DST case deletes both rows. The existing case for an older FE's plan (no marker) still passes. - 2 more BE unit tests from review: - INT96 under a nested ID-less wrapper struct, read and used as an equality key. It fails when the walk accepts the wrapper only at the root. - A nested equality key missing from an older file whose parent struct holds an INT96 `timestamptz`. It checks the survivors with the parent hidden and projected, that the projected timestamp reads as an instant in Asia/Shanghai, and a plan without the marker. It fails with the reader's type check when the parent is read without the marker. - End to end on a local cluster: an Iceberg table holding an INT96 Parquet file attached like `add_files` (one table with field ids in the file, one relying on `schema.name-mapping.default`) and an Iceberg-written INT64 file with the same instants, read through three catalogs (`enable.mapping.timestamp_tz` unset, `true`, `false`), with `enable_file_scanner_v2` on and off, in UTC and Asia/Shanghai: 24 combinations. Before this PR the INT96 rows were 8 hours early in every Asia/Shanghai combination (8 of the 16 measured); with it the INT96 rows equal the INT64 rows in all 24, and the INT64 rows and the UTC results are unchanged. branch-4.1 and branch-4.2 have the same code. **Classes, and how they call each other.** - `IcebergSchemaUtils` (FE): builds the schema dictionary; now marks `timestamptz` fields. - `schema_history_util`: `find_current_schema`, `build_schema_identity_from_external_field`, and `apply_file_timestamp_semantics` (moved from `PaimonReader`). - `IcebergTableReader`: `annotate_file_schema` → `_apply_positional_id_fallback` (the existing logic) and `_apply_table_timestamp_semantics`; `_append_equality_delete_predicates` (hidden keys), `_build_missing_equality_delete_key_expr` (the parent of a missing nested key) and `_load_equality_delete_file` (tombstones) project their columns with the markers through `equality_delete_root_projection`. - `column_mapper`: `attach_timestamp_semantics` is exported. ``` FE IcebergSchemaUtils.buildField --timestamptz--> TField.timestamp_is_adjusted_to_utc = true (history_schema_info, current_schema_id = -1) BE TableReader::open_reader +-- IcebergTableReader::annotate_file_schema (Parquet only) | +-- _apply_positional_id_fallback | +-- _apply_table_timestamp_semantics | +-- find_current_schema, build_schema_identity_from_external_field | +-- find_column_by_field_id | find_column_by_name (the mapper's matchers) | +-- apply_file_timestamp_semantics (shared with PaimonReader) +-- TableColumnMapper: mapping/projection take the marker from the file column +-- ParquetReader::open: apply_projection_timestamp_semantics -> INT96 decodes as TIMESTAMPTZ IcebergTableReader::_append_equality_delete_predicates +-- equality_delete_root_projection -> attach_timestamp_semantics (hidden key) +-- _build_missing_equality_delete_key_expr +-- equality_delete_root_projection (parent of a missing key) IcebergTableReader::_load_equality_delete_file -> _apply_table_timestamp_semantics + equality_delete_root_projection (tombstones) ``` ORC files are not changed: ORC's file type already separates `TIMESTAMP` from `TIMESTAMP_INSTANT`. ### Release note Fix Iceberg `timestamptz` columns stored as Parquet INT96 (written by Spark or Hive and attached with `add_files`, `migrate` or `snapshot`) reading shifted by the session time zone's offset in non-UTC sessions, and equality deletes on such keys missing their rows. ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [x] Unit Test - BE: `run-be-ut.sh --run --filter='IcebergV2ReaderTest.*:Paimon*ReaderTest.*:Hudi*Test.*:TableReaderTest.*:ColumnMapper*:FlussUnionLakeReaderTest.*:EqualityDeletePredicateTest.*:*PositionDelete*'`: 532 of 532 pass on macOS arm64 (ASAN), including the 5 new cases. After the first review round, `IcebergV2ReaderTest` passes 122 of 122, including the new nested ID-less wrapper case. It fails when the walk accepts the wrapper only at the root. After the second review round, the whole filter passes 534 of 534 and `IcebergV2ReaderTest` 123 of 123, including the new missing nested key case. It fails when the missing key's parent is read without the marker. They ran on a local base; the commit was then applied to master `2a4191c1dc9` without conflicts, and the changed sources and their test files were syntax-checked against that tree with the UT build flags. - FE: `mvn -pl :fe-connector-iceberg -am package -Dtest=IcebergSchemaUtilsTest`: 35 of 35 pass, checkstyle passes. - [x] Manual test (add detailed scripts or steps below) - The local end-to-end check under Results, with an ASAN BE and the Iceberg connector built from this change. - [ ] No need to test or manual test. Explain why: - Behavior changed: - [ ] No. - [x] Yes. Parquet columns of Iceberg `timestamptz` fields decode as instants by the table type, as Iceberg reads them: INT96 no longer shifts by the session offset, an INT64 timestamp without the UTC annotation under a `timestamptz` field reads its value as UTC, and equality deletes on `timestamptz` keys compare instants. Plans from an older FE are unchanged. - Does this need documentation? - [x] No. - [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Jack
|
c4b11b53e1 |
[fix](be) Keep BM25 statistics per index for MATCH on one column (#69901)
### What problem does this PR solve? Issue Number: None Related PR: #69790 Problem Summary: score() over MATCH clauses that read different inverted indexes of one column fails: INDEX idx_english(name) USING INVERTED PROPERTIES("parser" = "english", ...), INDEX idx_standard(name) USING INVERTED PROPERTIES("parser" = "standard", ...) SELECT id, score() FROM t WHERE name MATCH 'apple' USING ANALYZER english AND name MATCH 'apple' USING ANALYZER standard ORDER BY score() DESC LIMIT 10; -- Scoring predicates for one field select different inverted indexes BM25 statistics are keyed by field (column unique id plus suffix path), but each index of a column holds its own terms and lengths: over Apple and apple, an index that folds case has df(apple) = 2 and one that does not has df(apple) = 1. One entry cannot serve both, so the collector refuses the query. Fix: a MATCH keeps its statistics under the field and the index id. CollectInfo records the field as indexed for opening the index, since the map key no longer is one. The readers bind their source to the real field and pass the statistics name to run_leaf, so the scorers and similarities need no change: they already look statistics up by the name they are given. Each clause now scores with the statistics of its own index, and the combined score is the sum of what each clause scores alone. SEARCH keeps its own statistics names and its rejection of two indexes on one field; only MATCH changes. A MATCH and a SEARCH on one index keep separate entries with the same values. |
||
|
Jack
|
eb16c4d81f |
[improvement](be) Keep hot SNII term lookups across queries (#69879)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A SNII dictionary larger than the resident threshold (256 KB) is read block by block on demand. Every exact term lookup reads the term's DICT block (64 KB target), decompresses it with zstd, checks its CRC and parses its anchors, and the request-scoped DictBlockCache keeps nothing across queries. Every query therefore pays this again for every term in every segment. On a 99M-row AgentLogs table with 722 segments, counting one frequent term spent 68 ms of index time on these lookups against 16 ms for V3, and stack samples of postings-only and phrase queries showed 7-14% of SNII time in decompressing dictionary blocks alone. A SNII reader that the searcher cache keeps now keeps its recent exact lookups as well: a bounded LRU of up to 32 terms per reader, each at most 64 bytes long with at most 256 bytes of inline postings, which also remembers terms its dictionary turned out not to hold. `lookup()` and `lookup_batch()` check it before they read a block, so a batch reads only the blocks of the terms it does not hold. The cache's full footprint, about 25 KB, is reserved in the reader's searcher cache charge when the reader is inserted, as norms are, so the memory stays accounted. Only readers kept by the searcher cache enable it, and only when their dictionary is read on demand. The entries live inside the reader, so they go away with it and never outlive the file they describe. Caching the decoded blocks instead was not chosen: an exact lookup touches one 64 KB block per segment, so blocks would cost about 46 MB per hot term and column on that table, while a kept lookup costs a few hundred bytes. Keeping blocks in the searcher cache would also count against its fd-derived entry limit and push readers out. Prefix and pattern walks over the dictionary are not covered. Hot latency on the 99M-row table, cache off/on interleaved in one BE process: | Query | Off -> On | |---|---| | phrase 'retry budget depleted' (DS2) | 0.60 | | AND of two terms in two columns (A4) | 0.74-0.79 | | one term in two columns (A3) | 0.80 | | count of one frequent term (A2) | 0.84 | | regexp, its matched terms looked up again (DS9) | 0.89 | | phrase prefix (DS4) | 0.93 | | the other 19 queries | 0.92-1.05, within run-to-run noise | Against V3 on the same data (SNII/V3), DS2 went from 1.29 to 0.81, A4 from 1.24 to 0.95, A3 from 1.21 to 1.01 and A2 from 1.07 to 0.97. |
||
|
Jack
|
12f49adad2 |
[improvement](be) Add sorted docids to roaring bitmaps a container at a time (#69895)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: An equality filter on an indexed string column, such as `tenant = 'x'`, collects its whole posting list into a roaring bitmap, and a value can cover tens of percent of the table. `RoaringDocIdSink::append_sorted` gets those documents in sorted windows that mostly fall in one 64K container, but `Roaring::addMany` goes through the container's type for every document. On a 99M-row table that was about 20% of the SNII stack samples of a filtered phrase prefix query. `append_sorted` now splits a span at container boundaries and fills each container directly: - A bitset container gets one write per 64-bit word, with the bits gathered in a register, because neighbouring documents mostly share a word and a store per document would wait on the one before it. - An array container gets its tail appended in one loop, up to its limit of 4096. The next document turns it into a bitset, which then takes the rest. - Anything else goes through `addMany` as before: run containers, documents that fall inside an existing array, and spans that are not strictly ascending, which the sink does not rule out and a damaged posting could produce. The containers end up the same as with `addMany`. Two things to look at in review: - The sink is shared by every inverted index format, so V3 queries take the same path. - The code writes to CRoaring's container structs. `null_bitmap.cpp` already includes those headers, but only reads their sizes, so an upgrade of CRoaring has to keep this code in mind. CRoaring 2.1.2 has no public call that adds a sorted run to a container. | Query (99M rows), new/previous | SNII hot | V3 hot | SNII cold | |---|---|---|---| | phrase with tenant, app, type and status | 0.80 / 0.80 / 0.78 | 0.90 | 0.95 | | phrase with a latency filter | 0.78 / 0.77 / 0.78 | 0.78 | | | MATCH_ALL with the same four filters | 0.79 / 0.77 / 0.81 | 0.95 | 0.98 | | filters and a latency range, no text | 0.83 / 0.87 / 0.88 | 0.80 | | | phrase prefix with tenant and app | 0.93 / 0.92 / 0.92 | 0.93 | 0.98 | | top-50 with tenant and payload equalities | 0.77 | 0.82 | | | top-50 tool failures | 0.93 | 0.77 | | | the other 18 queries | 0.89-1.04 | 0.91-1.04 | | Hot: both versions interleaved in one BE process, three runs where three are shown, on a host with a load of about 100 to 140 from other builds. An earlier round at a load of 230 to 400 read up to 1.41 on some queries that do not gain; their reruns gave 0.95-1.07. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each; the change only saves CPU, so cold runs stay about level. The build also had #69879, #69882, #69887 and #69891, with `snii_inline_read_max_segments` at 16. |
||
|
Jack
|
4c1e0ddfe2 |
[improvement](be) Read and collect SNII BKD range hits in bulk (#69908)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A range filter on a numeric SNII index reads the leaves that lie wholly inside the range one at a time, gathers their doc ids, radix-sorts them and adds them to a roaring bitmap. On a 99M-row table, 71% of the SNII stack samples of a query with a wide `latency_ms` range were in this path: 15% in the sort, 9% in `addMany`, and about 27% in reading, mostly per-call overhead, since a leaf is a few KB and takes one positioned read of its own. - Whole-leaf hits lie next to each other in `bkd_data`, so one read now takes up to about 1 MiB of them. The bytes read are the same, in far fewer calls, and sequentially when they are not cached. - A wide range sets the doc ids in a flat bitmap instead, which needs no sort. It is loaded into a Roaring through the portable format, a published spec, because CRoaring 2.1.2 has no call that takes a bitset. - A flat bitmap costs a pass over all its words, so it takes doc ids only up to 256 per point, which holds it to 32 bytes per point. The doc ids decide, not the index's document count, which leaves out rows with no value: a narrow range, or a range on a column that is mostly NULL, keeps the sort. - The bitmap also stops at doc id 2^28, which is 32 MiB. The doc ids of a leaf past that take the sort as before, so a damaged leaf cannot make it allocate more. - The leaf decoder reuses its vector of value runs instead of allocating one per leaf. The on-disk format and the write path do not change. | Query (99M rows), new/previous | Hot | Cold | |---|---|---| | filters and `latency_ms BETWEEN 2000 AND 6000`, no text | 0.42 / 0.39 / 0.37 (212 ms to 85 ms) | 0.87 | | phrase with `latency_ms >= 3000` | 0.62 / 0.61 / 0.58 | 1.00 | | phrase with `latency_ms >= 8000` | 0.90 / 0.89 / 0.87 | 1.00 | | phrase with a 7-day date range | 1.01 / 0.98 / 1.00 | | Hot: both versions interleaved in one BE process on a quiet host, three runs each. The 21 queries of the bench with no range do not reach this code and read 0.86-1.02 in one run, which is the noise of that run. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each; the wide range gains and the others wait on other reads. All 25 queries return the same results with both versions. The build also had #69879, #69882, #69887, #69891 and #69895. A wide range briefly holds about three times its result in memory: the flat bitmap, the portable image and the bitmap loaded from it. That is one bit per row of the segment each, and it is freed when the range ends. The chunks of gathered doc ids it replaces took 8 MiB at most. Like those chunks, the bitmap and the image are plain vectors that the memory tracker does not see. |
||
|
Jack
|
050698f414 |
[improvement](be) Add up SNII position counts a run at a time in selective decode (#69891)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A phrase query decodes the positions of its candidate documents a window at a time. In a PFOR window the position counts of all the documents come first, and they are decoded to find where the positions of a selected document start, however few documents are selected. `decode_selected_pfor_count_ranges` went through every count of the window, with the running checks and a test for the next selected document at each one. On a 99M-row table that loop was 16% of the SNII stack samples of a phrase prefix query, nearly all of it in the loop rather than in the PFOR decode, because such a query selects no document in most runs. A run of counts is now added up whole, with no branch per document, so the compiler can vectorize it, and then walked from one selected document to the next, adding up only the counts in between. The sum of a run is kept in 64 bits and checked against the cap before any of its documents is used, so a window above the cap is rejected with the same error. The on-disk format and the write path do not change, and neither do the positions or the window shape returned. | Query (99M rows) | Hot, new/previous | Cold, new/previous | |---|---|---| | phrase prefix `'retry budget dep'` | 0.96 / 0.94 / 0.95 / 0.95 (four runs) | 0.99 | | phrase with a latency filter | 0.97 / 0.94 / 0.97 / 0.98 | 0.99 | | phrase `'unable to open'` in input or output | 0.98 / 0.96 / 0.98 / 0.99 | 0.92 | | the other 22 queries | around 1.0, no trend (0.90-1.06) | | In stack samples of the phrase prefix, with the same binary and both versions switched at run time, `decode_selected_pfor_count_ranges` fell from 16.1% to 3.4% of the SNII samples. Hot: both versions interleaved in one BE process, on a host with a load of about 100 to 150 from other builds, so differences of a few percent in a single run are noise. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. The change only saves CPU, so cold runs, which mostly wait for reads, stay level as expected. The build also had #69879, #69882 and #69887, with `snii_inline_read_max_segments` at 16. |
||
|
Jack
|
1106a37488 |
[improvement](be) Cut per-document work in SNII docid decode (#69909)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: An equality filter reads a posting list of millions of documents through `decode_window_prefix_plan`, so small costs per document, per PFOR run or per window add up. In the stack samples of a filtered phrase prefix query on a 99M-row table: - The window loop cleared its vector, so every window zero-filled it again before decoding wrote over it (4% of the SNII samples). - The sum of the docid deltas checked for a zero delta and for overflow at every document (about 7%, with the sum itself). - Each PFOR run read its exception count, and each exception its two fields, through the out-of-line varint decoders, although they are nearly always one byte (about 5%). The window loop now decodes over the vector, so a resize zero-fills only what it adds. A window's deltas are checked whole, with no branch per document, before they are summed: the same checks, and the same error code when one fails. If a window breaks more than one of them, the zero delta is reported first, whichever came first in the window. `pfor_decode` and `pfor_skip` use `get_varint32_fast`, which handles a one-byte value inline and leaves anything else to the checked decoder. The on-disk format and the write path do not change. | Query (99M rows), new/previous | Hot | Lowest run, new/previous | Cold | |---|---|---|---| | filtered phrase prefix | 0.94 / 0.96 / 0.86 | 0.95 / 0.93 / 0.93 | 0.98 | | phrase with tenant, app, type and status | 0.92 / 0.92 / 0.98 | 0.92 / 0.95 / 0.95 | 0.95 | | phrase with a latency filter | 0.95 / 0.91 / 0.99 | 0.94 / 0.92 / 0.96 | | | MATCH_ALL with the same four filters | 0.94 / 0.92 / 1.10 | 0.94 / 0.93 / 0.93 | | | top-50 with tenant and payload equalities | 0.94 / 0.94 / 0.95 | 0.95 / 0.94 / 0.99 | 1.00 | | the other queries | 0.93-1.03 | | | Hot: both versions interleaved in one BE process on a quiet host, three runs each. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 8 runs for the phrase prefix, 5 for the others. The build also had #69879, #69882, #69887, #69891 and #69895. |
||
|
Mingyu Chen (Rayner)andClaude Opus 5.5
|
347b9a03cf |
[test](be) Set the thread counts in TimeSharingTaskExecutorTest.test_invalid_task_handle_returns_error (#69915)
### What problem does this PR solve? Issue Number: None Related PR: #65054 Problem Summary: **In short:** `TimeSharingTaskExecutorTest.test_invalid_task_handle_returns_error` builds its executor from a `ThreadConfig` whose thread counts are never set, so BE UT runs intermittently die in it with a core dump. This PR sets the two counts in the test. **Background:** `TimeSharingTaskExecutor::ThreadConfig::max_thread_num` and `min_thread_num` are plain `int` fields without a default value. The other ten `ThreadConfig`s in `time_sharing_task_executor_test.cpp` set both; the test added by #65054 sets only `thread_name` and `workload_group` before calling `init()`. **The problem, and what it cost:** `init()` sizes the thread pool from whatever the stack holds. When that value is large, it keeps creating threads until mmap fails: ``` ==38657==ERROR: AddressSanitizer failed to allocate 0xfc000 (1032192) bytes at address 0x0f63acfd7000 (errno: 12) ==38657==ERROR: AddressSanitizer: SEGV on unknown address 0x0ab44632aa9e ... ``` TeamCity then reports `Build stopped: CORE DUMPED`, and no suite scheduled after this test runs, so the run says nothing about the PR's own BE tests that come later. Between 2026-10-24 and 10-31 this happened in 4 of 11 sampled BE UT runs, on four different PRs (build 1087538 on #69581 is one); a rerun passes. **How this PR fixes it:** set `max_thread_num` and `min_thread_num` to 0, as `test_remove_task_clears_queued_task_count` in the same file does. The test only calls `init()` and checks that null and foreign task handles are rejected, so it needs no worker thread. Test-only change; the executor is unchanged. **Results:** | | Before | After | |---|---|---| | Thread counts of the test's executor | whatever the stack holds | 0 / 0 | | BE UT | aborts in this test in some runs (4 of 11 sampled) | the test no longer depends on stack contents | Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
 Chenjunwei
|
8e10b235cf | [doc](regression) Translate the Iceberg write P0 coverage matrix into English (#69514) | ||
|
Mingyu Chen (Rayner)
|
f521ebfe4e |
[test](fe) Cover format=avro table functions failing during analysis (#69849)
### What problem does this PR solve? Issue Number: None Related PR: #69601, #64699 Problem Summary: **In short.** #69601 made FE reject `format=avro` while it parses file format properties. This PR adds end-to-end coverage for that: a regression suite that runs `s3()` and `hdfs()` with `format=avro` and checks that they fail during analysis, and a unit test for the two table-function constructors. Test only, no code change. **Background.** Master no longer reads standalone Avro files: #61141 removed the BE reader, #65733 the scanner module, and #69601 made `FileFormatProperties.createFileFormatProperties` throw `Avro file reads are no longer supported`. Before #69601, FE accepted the format and the query failed later in BE with `Not supported file format in fetch table schema: 12`. branch-4.2 still reads Avro through the JNI scanner (#64699 fixed a crash in it), so users who upgrade from 4.x with Avro table functions or loads get this error. **What was missing.** #69601's unit test calls `createFileFormatProperties` directly. Nothing checks that a table function reaches that check before it validates storage properties or lists files. If a constructor validated the location first, `format=avro` against an unreachable location would report a storage error instead, and no test would notice. **What this PR adds.** - `external_table_p0/tvf/test_tvf_avro_format_rejected`: `s3()` and `hdfs()` with `format` set to `avro` and to `AVRO`, against a made-up bucket, made-up credentials and an HDFS address nothing listens on. Each call must fail with `Avro file reads are no longer supported`. - `UnsupportedExternalReadsTest#testAvroTableFunctionsAreRejectedBeforeStorageAccess`: constructs `S3TableValuedFunction` and `HdfsTableValuedFunction` with `format=avro` / `AVRO` and checks the same message. **Results.** On master `2a4191c1dc9` with this change, all four statements of the suite fail during analysis with `Can not build s3(): Avro file reads are no longer supported` / `Can not build hdfs(): ...`, and the unit test passes. Before #69601 the same `s3()` call would have gone on to validate and list the made-up location, so the suite pins the order as well as the message. |
||
|
Mingyu Chen (Rayner)
|
a72dcaa17b |
[fix](fe) Keep an ADBC catalog's driver open until the calls still using it finish (#69874)
### What problem does this PR solve? Issue Number: None Related PR: #66331, #68353 Problem Summary: **In short:** dropping or altering an ADBC catalog can crash the FE process. Closing the catalog frees the native ADBC objects at once, including the connections and results that other threads are still reading, such as the column-statistics loads that a query on the catalog has just started. This kills the FE in about one External regression run in 75, whatever the PR under test changes. With this PR, closing a client only refuses new calls, and the last call still in flight releases the driver as it leaves. DROP and ALTER CATALOG still return without waiting. This commit was first pushed as the last commit of #68353, which is still in review. It shares nothing with the rest of that PR, so it is carved out here to land on its own. #### Background An ADBC catalog (#66331) reads its metadata through `AdbcClient`, which owns one native `AdbcDatabase` and one Arrow allocator per catalog. Every metadata call borrows a fresh connection through `AdbcClient.withConnection` and closes it afterwards. FE talks to the C driver through adbc-driver-jni 0.24. In that bridge, closing a parent closes everything still open under it, on the closing thread: `JniDatabase.close()` closes every connection still open on the database, and `JniConnection.close()` closes every result (`getObjects` and similar readers) still open on the connection. The bridge only checks a handle before a call starts, and the C driver does not check at all, so freeing an object that another thread is still using is a use-after-free in native code, which takes the whole JVM down rather than failing one call. Column statistics reach the driver too. When a query plans against an ADBC table, the STATS_FETCH pool loads column statistics in the background. A load that finds nothing in the statistics table asks the table itself: `PluginDrivenExternalTable.getColumnStatistic` → `AdbcConnectorMetadata.getTableHandle` → `withConnection` → `getObjects`. With `"meta.cache.adbc.metadata.enable" = "false"`, every such load calls into the driver. #### The problem, and what it cost - `AdbcClient.close()` closed the database and then the allocator immediately, whatever was still in flight. DROP CATALOG and ALTER CATALOG ... SET PROPERTIES close the catalog's connector this way, on their own thread (since #69665, on the `ExternalResourceReclaimer` pool; before that, synchronously on the statement thread, with the same race). - A statistics load that was reading a `getObjects` result, or was inside the driver on its connection, then read memory that had just been freed. Where it dies depends on where that thread is at that moment. - In CI, the External regression pipeline lost its FE in 11 of the 834 runs between 2026-10-22 and 2026-10-30. Each time, 54 to 56 suites then failed with `Communications link failure`, so the PR under test showed a red run it did not cause. All 11 died at the same moment: `test_adbc_sqlite_catalog_scan` drops its catalog with the metadata cache turned off right after querying it. In the same millisecond, fe.log shows the allocator closing with `Memory was leaked by query` and a statistics load failing with `Failed to read the ADBC getObjects result: ArrowArrayStream is already closed`. That is the same race, caught by a thread that came back only after the close had finished. | FE crash in `fe.out` | Runs | | --- | --- | | `SIGSEGV` in `nanoarrow::ArrayStreamFactory<VectorArrayStream>::get_next_wrapper+0x4` | 6 | | `SIGSEGV` in `PrivateArrowSchemaDeepCopy+0x46` | 2 | | glibc `double free or corruption (fasttop)` | 2 | | no message | 1 | | `SIGSEGV` in `sqlite3FindTable` (an earlier run, 2026-10-06) | 1 | - Users can hit this too, not only CI: DROP or ALTER an ADBC catalog while a statement or a statistics load is still reading it. #### How this PR fixes it - `AdbcClient` counts the calls inside `withConnection` (`enter` / `leave`, under the client's monitor, which also opens the database on first use). - `close()` marks the client closed, so new calls are refused with `AdbcClient has been closed`. It releases the database and the allocator itself only when no call is in flight. Otherwise it returns at once, and the last call to leave releases them. - A deferred release that fails is logged with the driver's path, as the catalog already logs a failed connector close. It is not thrown at that last caller, whose own work succeeded and which did not close anything. - `close()` never waits for the calls in flight, so DROP and ALTER CATALOG are not held up by a slow remote source. #### Results Unit tests (0 skipped). The first case runs against the real JNI bridge and the SQLite driver, the others against a recording database: | `AdbcClientTest` case | `close()` releasing at once, as before this PR | This PR | | --- | --- | --- | | a call still on its connection when the client is closed keeps using it | fails: `Native connection handle is closed` | passes | | a call still reading a `getObjects` result when the client is closed reads it to the end | fails: `the database was released under an open connection` | passes | | the last call to leave a closed client releases the database exactly once, after it leaves | fails: `the database was released under an open connection` | passes | | a release that fails as the last call leaves does not fail that call | fails: `the database was released under an open connection` | passes | | an idle client releases at once | passes | passes | Freeing only the allocator at once, and leaving the database to the last call, fails the `getObjects` case alone: `Attempting operation on allocator when allocator is closed`. A stress program (not committed) closes a client 200 times while four threads loop over `getObjects` and `getTableSchema` through `withConnection` against the SQLite driver: | | `AdbcClient` of master | This PR | | --- | --- | --- | | Runs | 5 | 5 | | JVM crashed | 5 of 5, 1.1 to 1.3 s after the JVM started: `SIGSEGV` in `PrivateArrowSchemaDeepCopy+0x40`, under `ArrowArrayStreamReader.readSchema` ← `AdbcObjectsReader.readNamespaces` ← `AdbcClient.withConnection` | 0 of 5 | | Closes completed per run | crashed before finishing | 200 | | Metadata calls per run | crashed before reporting | 6,019 to 7,238 | | Calls the close reached while they were using the driver | | 0 | | Failed closes, failed deferred releases | | 0, 0 (checked with WARN logging on, where the unit test's deliberate release failure shows up as one warning) | The stress runs used the macOS arm64 build of the same SQLite driver, so the offset differs from the Linux one in CI. #### Classes, and how they call each other - `AdbcClient`: `withConnection` brackets the body with `enter` / `leave`; `close` refuses new calls and releases only when idle; `release` closes the database, then the allocator; `openDatabase` is the one place the database is created, so a test can stand a recording database in for the driver. - `AdbcClientTest`: five new cases, one against the real driver and four against a recording database. The recording connection answers `getObjects` with a result built in memory in the client's allocator: reading a driver's result into Arrow loads arrow-c-data's JNI shim, which needs CXXABI_1.3.9 and does not load on the FE UT agent (see `AdbcNativeTestSupport`, #68510). - `AdbcObjectsReaderTest`: `response`, `catalog` and `schema` are package-private, so `AdbcClientTest` builds that result with them. ``` STATS_FETCH: ColumnStatisticsCacheLoader.doLoad -> PluginDrivenExternalTable.getColumnStatistic -> AdbcConnectorMetadata.getTableHandle (namespaces, table names) -> AdbcClient.withConnection enter() refused once closed; callsInFlight++ -> AdbcConnection.getObjects -> AdbcObjectsReader.readNamespaces / readTableNames leave() callsInFlight--; the last one out of a closed client -> release() DROP CATALOG / ALTER CATALOG ... SET PROPERTIES -> ExternalCatalog.onClose / resetToUninitialized -> ExternalResourceReclaimer -> AdbcConnector.close -> AdbcClient.close() closed = true; release() now only if no call is in flight release(): AdbcDatabase.close (closes its connections and their results) -> allocator.close ``` Not changed: `AdbcConnector`, the catalog lifecycle and `ExternalResourceReclaimer`, BE, and the regression suites; `test_adbc_sqlite_catalog_scan` stays as it is, since what it exposes is this bug. |
||
|
Xin Liao
|
ba3f2c6af0 |
[fix](regression) Sync version 3 before DROP in test_clean_tablet_when_drop_force_table (#69846)
Related PR: #68449 Problem Summary: `test_clean_tablet_when_drop_force_table` fails intermittently with `File cache directories were not cleaned before timeout`. The leftover cache dirs all belong to the version 3 rowsets. #68449 set `disable_auto_compaction=true` for this case. Before that change, cloud compaction called `sync_rowsets()` and loaded version 3 into the BE tablets as a side effect. Now nothing loads version 3 between the second stream load and `DROP TABLE FORCE`, because the case only queries after the version 2 load. The cloud `drop_tablet_callback` only recycles the file cache of rowsets in `get_snapshot_rowset(true)`, so the version 3 cache stays and the cleanup assertion times out. This PR queries the table after the version 3 load, the same way the case already does after version 2. The query loads version 3 on every BE before DROP. It also asserts the row count (160000) to confirm version 3 is visible. |
||
|
Mingyu Chen (Rayner)
|
339e087b11 |
[fix](be) Reject incompatible Arrow arrays without crashing Stream Load (#69872)
### What problem does this PR solve? Issue Number: close #69858 Related PR: None Problem Summary: The shared Arrow BOOLEAN and DECIMAL SerDes dereference unchecked `dynamic_cast` results. Passing an `Int32Array` to the BOOLEAN reader or a `StringArray` to the DECIMAL64 reader can therefore terminate the BE instead of returning a load error. MAP and STRUCT have the same unchecked input-cast pattern. Reject incompatible arrays before accessing buffers, decimal metadata, or values, independently of `enable_arrow_input_validation`. Reject mismatched STRUCT field counts before indexing children. Add the column name and source/destination types to V1/V2 Arrow load errors. The current V2 Stream Load reader already binds supported input types with safe Arrow casts. Preserve those conversions, including valid integer-to-BOOLEAN and string-to-DECIMAL inputs, and test the shared SerDes directly for incompatible arrays. |
||
|
Calvin Kirs
|
fb61aeeb63 |
[fix](plugin) Allow legacy unversioned authentication plugins (#67286)
### What problem does this PR solve? #66211 Problem Summary: The authentication plugin API-version gate rejects every plugin jar that does not contain `Doris-Authentication-Plugin-Api-Version`. Authentication plugins were distributed before this manifest contract existed, so upgrading FE breaks those legacy artifacts even when they implement the current v1 API. This change adds an opt-in legacy mapping to the generic API gate and configures only the authentication family to treat a genuinely absent declaration as API 1.0: - unversioned legacy authentication plugins load with an operator warning; - empty, malformed, and explicitly incompatible declarations remain rejected; - connector, filesystem, lineage, and other strict gates keep their existing fail-closed behavior; - when the FE authentication API moves to another major, legacy v1 plugins are rejected automatically. |
||
 hui lai
|
374d87d81d |
[perf](csv reader) Reduce V2 CSV parsing overhead (#69875)
### What problem does this PR solve? Issue Number: None Related PR: #69716 Problem Summary: After restoring sequential prefetch, large V2 CSV loads still spend substantial CPU time in row parsing and string materialization. The reader compares a general separator at every byte, updates atomic profile counters per cell, starts three stage timers per row, and repeats checked string-column casts per cell. It also ignores the row cap supplied by TableReader. This change combines four optimizations on master: - Compare the separator byte directly for plain single-character separators; retain enclosed, multi-character, Hive/OpenCSV and PROTO parsing paths. - Accumulate exact work counters locally and publish at get_block/count/schema return, including errors. Sample stage timings every 1024 physical read_line calls, starting with the first, and expose unscaled SampledReadLineTime, SampledSplitLineTime and SampledDeserializeTime with sample counts. DelimitedTextReader remains full elapsed time; split-prefix alignment has its own full timer. - Validate nullable string column types and reserve offsets/null maps once per mutable batch, then insert strings directly with the existing NULL, quoting and escape semantics. - Honor FileReader::set_batch_size, retaining the existing byte budget and runtime default when no explicit cap is supplied. The caller still controls the maximum batch size; this does not force 16352-row blocks. Shared counters and batch sizing also apply to the Hive text reader. ### Performance benchmark On TPC-DS SF100, elapsed time decreased from **1,944 seconds to 1,375 seconds**: | Benchmark | Before | After | Time reduction | Speedup | | --- | ---: | ---: | ---: | ---: | | TPC-DS SF100 | 1,944 s | 1,375 s | 29.3% (569 s) | 1.41× | ### Release note Reduce V2 CSV parsing overhead and honor caller-provided batch sizes. Delimited-text per-stage profile timers are now explicitly named Sampled* and contain unscaled sampled durations; use the full DelimitedTextReader timer for elapsed-time comparisons. ### Check List (For Author) - Test - [x] Unit Test: added cases for dynamic/zero batch caps, byte limits, filtered blocks, cross-batch sampling and exact counters, single/multi-character separators, NULL and escape behavior, missing fields, reordered projection and copy-on-write. Extended error, rejected-row and COUNT counter coverage. - Execution: run-be-ut.sh was attempted with CsvV2ReaderTest.*, TextV2ReaderTest.* and LegacyAndV2/PlainCsvSplitTest.*. It exits before C++ compilation because the local JDK 17 fails to initialize (Failed setting boot class path), including outside the sandbox. Full build/UT remain pending. Performance benchmark results are reported above. - Passed: repository clang-format 16 formatting/check scripts, build-support/check-build-hygiene.sh and git diff --check. - Behavior changed: Yes. Existing batch-size contract is honored, and detailed stage timers are explicitly sampled. CSV value semantics are unchanged. - Does this need documentation: No. The profile includes a timing explanation; no new configuration or SQL syntax. For further benchmark comparisons, use identical hardware, input, compression, cache conditions and load batch/byte-budget settings for the parent and this commit. Compare total load time, Scanner CPU time, DelimitedTextReader time and RowsRead/BlockNumber. Do not directly compare Sampled* timers with the previous full stage timers. |
||
|
HappenLee
|
45d1598da2 |
[improvement](be) Accelerate phmap control scans with ARM NEON (#69688)
AArch64 currently uses portable scalar phmap control-byte scans. The subtraction-based H2 mask can include adjacent false-positive candidates: control bytes `0x12, 0x13` can both be returned when looking for `0x12`. Key comparison preserves correctness, but the extra candidates cost work. Add exact NEON control comparisons on little-endian AArch64 while keeping eight-byte groups, the portable fallback and the existing ARM dump/load layout. x86 keeps its existing SSE implementation. `PHMAP_HAVE_ARM_NEON=0` disables the new implementation at compile time. The normal third-party downloader applies this patch alongside the existing AArch64 int128 hash-mixer patch, with separate markers and a subshell that restores the caller's working directory. The NEON patch now includes context so a replay cannot silently insert the same code again. The downloader checks the actual source with a direction-locked reverse dry-run, recovers a missing marker when all hunks are already applied, and preflights every hunk before a fresh application. Partial or duplicated sources fail without further edits and do not retain a success marker. The standalone tests cover control masks, unaligned reads, special slots, conversion bounds, map operations, rehash and dump/load. The standalone C++ benchmark can be compiled directly. For an incremental comparison, retain the existing int128 patch in both the baseline and optimized dependency sources, adding NEON only to the optimized copy. ### Supplied ARM measurements The following measurements come from the supplied report on original Doris baseline `70f7dcbe11c8e142b81536c1dfaaf505ec05437b`, not a new ARM run of this rebased PR. That baseline predates master's AArch64 int128 patch. These numbers therefore do **not** establish the incremental gains of the final combined patch stack; a new ARM run with the combined patch stack is still needed. Environment: Neoverse-N2, 16 cores, nominal 3 GHz; Clang 20.1.8; C++17, `-O3 -DNDEBUG -march=armv8-a+crc -pthread`; one process pinned to CPU 15. Nine alternating baseline/NEON rounds, each measurement at least 0.2 seconds of thread CPU time. Values are median thread CPU ns/op. Reported intervals use 10,000 paired bootstrap resamples. Positive time reduction means faster; negative means slower. | Scenario | Baseline ns/op | NEON ns/op | Time reduction | Reported 95% interval | |---|---:|---:|---:|---:| | Hit lookup, 4,096 keys | 4.0773 | 3.8350 | +5.94% | [+5.84%, +6.11%] | | Hit lookup, 100,000 keys | 10.6120 | 9.6412 | +9.15% | [+8.99%, +12.44%] | | Hit lookup, 114,000 keys | 11.4606 | 10.5508 | +7.94% | [+7.73%, +11.42%] | | Hit lookup, 1,000,000 keys | 16.3554 | 14.4228 | +11.82% | [+10.20%, +12.74%] | | Miss lookup, 4,096 keys | 3.4818 | 3.3069 | +5.02% | [+4.81%, +5.08%] | | Miss lookup, 100,000 keys | 9.7027 | 9.9144 | -2.18% | [-2.91%, -1.27%] | | Miss lookup, 114,000 keys | 16.8860 | 17.5147 | -3.72% | [-3.93%, -2.89%] | | Miss lookup, 1,000,000 keys | 8.6927 | 8.1035 | +6.78% | [+4.93%, +9.01%] | | Insert, 100,000 keys, including allocation/destruction | 9.8116 | 9.2758 | +5.46% | [+3.94%, +6.25%] | | Erase/reinsert, 20,000 operations per iteration | 16.3032 | 16.3810 | -0.48% | [-1.19%, +0.97%] | | Synthetic adjacent-H2 scan, 32,768 groups | 2.7578 | 1.7641 | +36.03% | [+35.75%, +36.09%] | Load factors differ: approximately 50.0%, 76.3%, 87.0% and 47.7% at the four lookup sizes. Two miss-lookup workloads regress reproducibly. The erase/reinsert interval crosses zero. The adjacent-H2 case measures group scans and candidate-key comparisons, not complete table lookups. These are library microbenchmarks, not end-to-end SQL gains. ### Validation **Reported in the supplied ARM bundle:** randomized scalar-oracle checks, baseline/NEON/forced-portable paths, bidirectional dump/load and ASAN/UBSAN passed. The archive contains the patch and report; these ARM executions were not repeated during this submission. **Executed locally after rebasing onto master `3d2d9aac54148056529c13ef4b9fbdbcb65fab77` (x86_64, Clang 16.0.6):** - Standalone correctness tests passed for baseline, patched and explicitly NEON-disabled builds; x86 selected the existing 16-byte SSE groups in all three. Tests also exercise the portable group implementation directly. - Baseline-to-patched and patched-to-baseline dump/load passed; patched ASAN/UBSAN passed. - Benchmark source compiled with `-Wall -Wextra -Werror`; all 11 scenarios passed a short checksum smoke run. No x86 performance conclusion is drawn from it. - Standard third-party download/patch script passed twice with `OLDPWD` unset. The standard downloader produces byte-identical headers to those used by the standalone tests. - clang-format 16, repository clang-tidy entry point for both standalone C++ files, shell syntax and `git diff --check` passed. **Downloader recovery follow-up:** `bash thirdparty/test/parallel-hashmap-patch-test.sh` passed using the pinned cached archive and the real downloader; no dependency downloads were needed. It covers fresh extraction, normal reruns, an applied patch with a missing marker, a stale marker over pristine sources, partially applied sources with/without a marker, and duplicate NEON definitions. Successful reruns compare both complete headers; failing cases verify unchanged source and absence of the success marker. The same test detects the original replay bug. The regenerated patch inserts exactly the same 67 C++ lines, and its applied header bytes match the original optimized source. Bash syntax and shell whitespace checks passed; generated patch context preserves the pinned dependency's existing whitespace. Full Doris BE compilation, SQL regression/performance tests and new ARM measurements against the combined int128+NEON patch stack have not been run. The C++ test and benchmark have no Python dependency and can be compiled directly: ```bash bash thirdparty/download-thirdparty.sh parallel_hashmap clang++ -std=c++17 -O2 -pthread -Wall -Wextra -Werror \ -isystem thirdparty/src/parallel-hashmap-1.3.8 \ thirdparty/tests/parallel_hashmap_neon_test.cpp -o /tmp/phmap-neon-test /tmp/phmap-neon-test clang++ -std=c++17 -O3 -DNDEBUG -pthread -Wall -Wextra -Werror \ -isystem thirdparty/src/parallel-hashmap-1.3.8 \ thirdparty/benchmarks/parallel_hashmap_bench.cpp -o /tmp/phmap-neon-bench taskset -c 15 /tmp/phmap-neon-bench 0.2 ``` Select an available isolated CPU instead of CPU 15 when necessary. For the reported ARM compiler settings, add `-march=armv8-a+crc`. The benchmark emits one JSON line per scenario with CPU/wall timings and checksums. To compare against baseline on current master, extract the verified source archive into another directory, apply only `parallel-hashmap-1.3.8-aarch64-int128.patch`, compile the same C++ benchmark against that directory with identical flags, and alternate baseline/optimized execution order. ### Release note Accelerate phmap control-byte scanning on little-endian AArch64 while preserving eight-byte ARM groups and the portable fallback. |
||
|
yujun
|
96f2558e16 |
[feature](function) Add MIN_TOPK/MAX_TOPK and ARRAY_SORTED_MERGE functions (#69456)
### What problem does this PR solve? Issue Number: N/A (tracked in https://github.com/apache/doris/issues/65418) Related PR: #69286 Problem Summary: Adds the generic building blocks that incremental MIN/MAX maintenance builds on; the IVM integration itself is a separate PR. - `min_topk(x, k)` / `max_topk(x, k)`: an aggregate returning `ARRAY<STRUCT<value T, count BIGINT>>` — the k smallest (largest) distinct non-NULL values with their exact row counts, ordered by value. The array carries no truncation flag: a caller that needs one infers it from the length, treating a list of exactly k entries as possibly truncated. With `k = 0` the array holds at most the single `(extreme, count)` pair. `k` is a constant `BIGINT` with `0 <= k <= 65536`. - `array_sorted_merge(merge_fn, arr1, arr2, is_asc)`: a generic higher-order function that merges two arrays already sorted by the elements' natural order. `merge_fn` receives the two current candidates and returns the combined element, or NULL when the two elements must be kept separately. `is_asc` states the sort direction, so one function covers ascending and descending inputs, and calls can be chained to merge more than two arrays. The aggregates keep a sorted `(value, count)` state with checked int64 counting, so partial aggregation and merge stay exact (`topK(A ∪ B) ⊆ topK(A) ∪ topK(B)`). FLOAT/DOUBLE order NaN as greater than every number so the distinct list keeps a strict order. Returning the array alone keeps the aggregate in the same shape as the other array aggregates and leaves the truncation flag to the caller that actually persists a bound: incremental MIN/MAX maintenance still stores one, but as its own state column rather than inside the aggregate result. `array_sorted_merge` keeps its non-lambda `is_asc` argument through the higher-order binding: the existing pipeline passed only the lambda arguments to the bound function, so the binding contract gained a way to return extra inputs. ### Release note Add the `min_topk`/`max_topk` aggregate functions and the `array_sorted_merge` scalar function. |
||
|
HappenLee
|
9c8f7b2d2e |
[improvement](be) Compact selected row indices with mask lookups (#69818)
`SegmentIterator` expands partially selected SIMD masks one set bit at a
time, creating a serial dependency for every selected row. Expand x86
masks with more than two selected rows using a constexpr eight-row
lookup table and fixed-width stores. Keep the existing bit iteration for
sparse masks and other architectures. Empty/full masks, scalar tails,
row order and downstream column materialization retain their existing
behavior.
The output selector already has one slot per input row. At a complete
SIMD block, the number of emitted indices is at most the input position,
so the fixed-width stores fit within that allocation; only the returned
prefix is consumed. The lookup table has constant initialization and
introduces no dynamic allocation or mutable shared state.
Add unit coverage and a standalone benchmark that measures the original
loop and the actual production helper. On an Intel Xeon Platinum 8457C,
with 4096-row batches, random 50% selection and Clang 20.1.8 `-O3
-march=native`:
| Benchmark | Before (ns/input row) | After (ns/input row) | Speedup |
| --- | ---: | ---: | ---: |
| Row-index generation | 0.693280 | 0.119248 | 5.81x |
| Predicate comparison + selection + 8-byte copy | 1.170464 | 0.643518 |
1.82x |
Performance tradeoff: Clang 16.0.6 with `-march=x86-64-v3` regresses
from 0.104319 to 0.119946 ns/input row at random 1% selection (about 15%
slower). The Clang 20 native 1% case is approximately unchanged. These
are standalone measurements; complete SQL query speedup has not been
measured.
### Release note
None
### Check List (For Author)
- Test
- [ ] Regression test
- [ ] Unit Test
- [x] Manual test (details below)
Validation completed:
- Standalone benchmark with native and AVX2 compiler settings.
- ASan + UBSan verification of all 256 byte masks, 65536 mixed masks,
scalar tails, row order, output bounds, near-uint16 row limits and
payload copies.
- clang-format 16.0.6, repository build-hygiene checks and focused
clang-tidy for the helper, test source and standalone benchmark.
- Prepared this PR as one commit on the latest master; the helper, test
and benchmark source blobs are identical to the verified versions.
Unit tests added: empty/full/single-bit masks, exhaustive byte and mixed
masks, large row bases and consecutive appends. They have **not executed
through the standard BE test runner**. Both `./run-be-ut.sh -j 48 --run
--filter=SelectionTest.*:BitsTest.*` and `BUILD_TYPE=ASAN ./build.sh
--be -j 48` were attempted in the development workspace, but compilation
failed in unrelated existing local `ColumnVector` edits at
`column_vector.h:95` before the tests could execute. Full BE build and
full `SegmentIterator` static analysis remain incomplete. No SQL or ARM
execution.
Standalone reproduction (x86, from the repository root):
```bash
c++ -O3 -DNDEBUG -std=c++20 -march=native -Ibe/src \
be/benchmark/selection_compaction_bench.cpp -o /tmp/selection_compaction_bench
/tmp/selection_compaction_bench --verify-only
/tmp/selection_compaction_bench --pattern random --rate 50 --mode select \
--repeats 9 --min-ms 60
/tmp/selection_compaction_bench --pattern random --rate 50 --input values \
--mode copy8 --repeats 9 --min-ms 80
`
|
||
|
Gabriel
|
4aab9c4a44 |
[fix](iceberg) Honor storage Kerberos settings and retain REST credentials (#69598)
### What problem does this PR solve? Issue Number: N/A Iceberg catalogs configured with `hdfs.authentication.*` can fail against secured HDFS with `Client cannot authenticate via:[TOKEN, KERBEROS]`. Storage binding already translates these properties into Hadoop configuration, but plugin-side authenticator selection checks only the raw legacy `hadoop.security.authentication` key and skips the Kerberos identity. Resolve authentication from the merged Hadoop configuration used by FileIO and create a plugin authenticator only when it resolves to Kerberos with a principal/keytab pair. XML resources can contain a Kerberos mode without credentials; the shared resolver then falls back to SIMPLE. Preserve the existing engine/storage authentication path in that case so FileIO does not switch to the default `hadoop` user. Keep HMS authentication separate. Complete Kerberos credentials can also arrive through XML for a REST catalog that vends temporary S3 credentials. Scan planning wraps the table FileIO for authenticated manifest access, but that wrapper does not expose Iceberg's optional `SupportsStorageCredentials` interface. Iceberg 1.12 stores the REST response's `storage-credentials` separately from `FileIO.properties()`, so inspecting the wrapper alone loses the temporary keys forwarded to BE. Unwrap the authentication decorator only while reading credential metadata; keep file operations authenticated and preserve the existing optional capability surface. Add a canonical-property unit test and an HMS/storage identity separation check, and switch the existing Kerberos regression suite to canonical storage properties so its read/write, rollback, and snapshot-expiration coverage exercises this path. ### Release note Iceberg catalogs honor canonical HDFS Kerberos authentication properties for plugin-side file access and retain REST-vended storage credentials when Kerberos is also enabled. ### Check List (For Author) - Test - [x] Unit Test: canonical credential selection and XML identity fallback both have red/green verification; all 14 authenticator tests pass, including incomplete principal/keytab pairs and the original storage/HMS separation checks. Two additional tests use the real Iceberg 1.12 `S3FileIO` to cover XML-resolved Kerberos with separate REST credentials, nested authentication decorators, credential refresh, the disabled-vended gate, and BE access-key/secret-key/session-token forwarding; both fail before the credential extraction fix and pass afterward. - [x] Unit Test: the complete Iceberg module reports 1743 tests, 0 failures, 0 errors, and 14 skipped (1729 passed), run through `run-fe-ut.sh` with all 106 module test classes selected explicitly. - [ ] Regression test: updated `test_iceberg_hadoop_catalog_kerberos`; KDC/S3/HDFS end-to-end suites were not run locally. - [x] FE Checkstyle: all 88 reactor modules pass `mvn -f fe/pom.xml checkstyle:check`. - Behavior changed: - [x] Yes. Canonical HDFS Kerberos properties now select the plugin-side storage authenticator. - Does this need documentation? - [x] No. This restores the existing properties' documented behavior. |