Repository navigation
[improvement](build) Fully exclude OBS/COS deps via --exclude-{obs,cos}-dependencies - #4
Merged
Merged
Conversation
apache#66342) ### What problem does this PR solve? Issue Number: N/A Problem Summary: Session and global variable annotations currently store both Chinese and English descriptions as a two-element array. The Chinese description is unused and the array shape makes callers cumbersome. This change keeps only the English description, updates the VarAttr annotation type, and adjusts the affected unit tests. ### Release note None ### Check List (For Author) - Test: Unit Test - `./run-fe-ut.sh --run org.apache.doris.qe.SessionVariablesTest` - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Problem Summary: Snapshot tasks persisted the complete offset provider after every split commit, producing frequent edit logs while the snapshot state kept growing. The previous default snapshot split size also produced too many small snapshot tasks. Natural terminal transitions needed explicit persistence so finished jobs replay with their terminal metadata. This change: - Limits snapshot offset edit logs with a mutable 300-second interval while keeping binlog offset commits immediate. - Increases the default snapshot split size from 8,096 to 40,960 rows, while preserving explicitly configured values. - Removes obsolete snapshot recovery state after the first committed binlog offset for FROM-TO and CDC stream TVF jobs. - Persists natural FINISHED transitions and restores terminal replay fields and callbacks consistently. - Adds focused unit coverage and a MySQL snapshot-only FE restart regression case.
…-job config (apache#66541) ### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#66342, apache#66238 Problem Summary: master does not compile. `fe-common` fails at ``` Config.java:[1177,65] annotation value not of an allowable type ``` apache#66342 retyped `ConfigBase.ConfField.description()` from `String[]` to `String` and rewrote all 419 call sites accordingly. apache#66238 landed shortly after with ```java @ConfField(mutable = true, masterOnly = true, description = { "Minimum interval in seconds between snapshot offset persistence operations"}) public static int streaming_job_snapshot_offset_persist_interval_sec = 300; ``` written against the older `String[]` signature. The two changes are textually disjoint, so git merges them without a conflict and neither pull request could see the other — each was green on its own base. The seven further errors reported in the same module are secondary. The bad annotation value aborts the annotation-processing round, so lombok never contributes its generated members, and `@Slf4j`'s `log` plus `@AllArgsConstructor`/`@Data`'s constructors go missing: ``` DiskUtils.java:[70,13] cannot find symbol JobBaseConfig ... constructor cannot be applied to given types AbstractSourceSplit ... constructor cannot be applied to given types ``` All seven disappear once the annotation value is fixed; nothing else in the tree needed a change. This is the only remaining array-form description under `fe/` (`grep -rnE 'description\s*=\s*\{'`), and the wrapped-argument layout matches the neighbouring long descriptions such as `max_create_table_timeout_second`. `ConfigTest.testConfFieldDescriptionsAreEnglishStrings`, the guard apache#66342 added, reflects over the annotation at runtime, so it cannot catch a compile-time signature mismatch; it passes here because the text is already English. Verified on a clean checkout of master `4e3c1b84dd5`: - reproduced the failure before the change, and confirmed all eight errors are gone after it - full FE reactor `mvn test-compile` (checkstyle included): **74/74 modules SUCCESS** - `fe-common` module tests: **157 tests, 0 failures, 0 skipped**, including `ConfigTest.testConfFieldDescriptionsAreEnglishStrings` - the tests both colliding PRs added — `StreamingInsertJobOffsetPersistenceTest`, `JdbcSourceOffsetProviderOffsetTest`, `SessionVariablesTest`: **39 tests, 0 failures, 0 skipped**
Problem Summary: The test inserts five batches, exactly matching the default cumulative compaction threshold. The partition can become visible in FE before its latest visible version reaches BE, so manual cumulative compaction may return `E-2000: _input_rowsets is empty`. The generic helper ignored E-2000, leaving no tracker record for the test to verify. The fix targets the BE hosting the tablet, retries only E-2000 while requesting tablet reports, and waits for that MANUAL cumulative tracker record to reach FINISHED or FAILED. The case still uses the original five batches, and other trigger errors fail immediately.
…ge (apache#66337) Related PR: apache#63112 Problem Summary: This PR takes over apache#63112 and adds explicit local and Cloud unit-test coverage. While a schema change target tablet is `TABLET_NOTREADY`, cumulative compaction should merge only older rowsets and leave the newest 10 versions unmerged. The filter used the inverse comparison, skipping older rowsets and selecting the newest versions. A compaction output could then cross the base tablet's maximum version and prevent incremental schema-change conversion with `VERSION_ALREADY_MERGED`. This change reverses the comparison in both local and Cloud size-based cumulative compaction policies. The new tests verify that versions 2 through 10 are selected and versions 11 through 20 remain unmerged. ### Release note Fix schema changes that could fail when cumulative compaction merged the latest versions on the new tablet.
…he#66534) ### What problem does this PR solve? Issue Number: None Related PR: apache#66328 Problem Summary: `WorkloadGroupManagerTest` creates and initializes a `SpillFileManager` for every test case. Initialization starts a background spill GC thread, but the fixture did not stop or delete the manager during teardown. Each subsequent test overwrote the global manager pointer, leaving the previous manager and its GC thread alive. The leaked threads could continue accessing process-wide test utilities during static destruction and trigger an ASAN heap-use-after-free when the test binary exited. They could also interfere with tests that use global spill GC debug points. This change releases workload-group-owned query resources first, then stops and joins the spill GC thread, deletes the manager, and finally removes the temporary spill directory. This matches the lifecycle used by other spill-related test fixtures. ### Release note None ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - `GTEST_REPEAT=10 ./run-be-ut.sh --run --filter='WorkloadGroupManagerTest.*:DebugPointsTest.AddTest:SpillFileTest.RetryPreservesDirectoryQueuedAfterPendingDrain' -j 8` - [ ] 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: - [x] No. This only corrects test fixture resource teardown. - [ ] 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
…66442) Problem Summary: Nullable TopN comparisons can return a `ColumnConst` that wraps a `ColumnNullable` result, especially when an input batch is entirely NULL. The previous normalization only handled a top-level `ColumnNullable`, so the constant nullable result could reach filter execution without converting SQL NULL into the NULLS FIRST or NULLS LAST filter decision. This change recursively normalizes constant wrappers, collapses nullable Boolean results according to the TopN null ordering, preserves the constant shape, and aligns the unit-test expression metadata with production nullability. The new unit test covers both NULLS FIRST and NULLS LAST through `execute_column` and `execute_filter`.
…e#66345) ### What problem does this PR solve? This PR fixes correctness and memory-usage edge cases in Iceberg MVCC writes and nested schema evolution: - An explicitly empty read could be reinterpreted as the latest snapshot after a concurrent first append. A MERGE could then miss the concurrent row and silently insert a duplicate. - Write-schema validation used the INSERT column subset instead of the complete bind-time target schema, rejecting valid partial and static-partition INSERTs while weakening ordinal drift checks. - Iceberg v3 row-lineage metadata columns could be mistaken for user-schema drift. - Parent null-mask handling for nested ARRAY/MAP values could scan descendant entry maps and allocate entry-sized scratch even when the inherited mask was absent or all-clear. - Strict cast failure semantics were incorrectly used to mark a struct field physically non-nullable even though BE narrowing casts still return `ColumnNullable`. - Nested DESCRIBE comments were not emitted as round-trippable SQL literals. The fix preserves the explicit empty-snapshot marker through scan planning and keeps RowDelta conflict validation anchored at table creation. In the concurrent empty-table MERGE/INSERT window, MERGE keeps reading the original empty view and its commit conflicts with the concurrent first append instead of producing duplicate data. Write planning now carries an immutable complete target-schema snapshot separately from the write subset, validates column order, type, and Iceberg field id, and excludes engine-generated row-lineage metadata from the user-schema comparison. Nested collection alignment gates descendant scans on an inherited mask that can actually hide a row, and struct constructors preserve the physical nullability of cast results. ### Release note Fix Iceberg empty-snapshot concurrency, partial-write schema validation, nested collection memory usage, cast-result nullability, and nested DESCRIBE comment rendering. ### Check List (For Author) - Test - [x] Regression tests - [x] Unit tests - Behavior changed: - [x] Yes. Correctness and memory-usage fixes for the affected edge cases. - Does this need documentation? - [x] No. ### Tests - Iceberg write-plan tests: 50 passed - Iceberg concurrent empty-snapshot barrier test: passed - BE `TableReaderTest`: 95 passed - FE connector sink, binding, executor, and struct tests: 38 passed - FE Checkstyle: 0 violations - Clang Formatter 16: passed - `git diff --check`: passed
…and make each key have one reader (apache#66507) ### What problem does this PR solve? Issue: apache#65185 Problem Summary: Every connector had grown its own way of dealing with catalog properties. Some kept a `XxxConnectorProperties` constant class and parsed values at each read site; some had no property class at all and inlined the key names; iceberg and paimon had *two* readers for the same keys — a typed holder used for CREATE-time validation, and a separate raw-map scan with its own copy of the alias arrays used to actually build the catalog. Nothing kept those two in agreement, so an alias priority or a blank-value rule could be validated one way and assembled another. This PR gives every connector the same four-way split, and makes each key have exactly one reader. No new SPI: `ConnectorPluginSurfaceTest` is untouched throughout. ### Where things live now **A — `<Xxx>CatalogProperties`: what a user writes in `CREATE CATALOG`.** One class per connector, in the connector module. Fields carry `@ConnectorProperty` with the alias list, so a key name and its aliases are declared exactly once, and `ConnectorPropertiesUtils` binds them. This is what the connector, the metadata layer and the scan/write planners read — none of them touch the raw map for a key this class declares. The two entry points are deliberately not interchangeable, and this is the part most worth reviewing: - `of(Map)` **binds and derives, and never throws.** It runs at CREATE, at ALTER validation, and on every connector build — including the lazy rebuild after an FE restart. A rule placed here that a live catalog violates does not fail at review time; it fails months later, as a catalog that stops coming back after a restart. - `checkCreateTimeOnlyRules()` carries everything that judges, and only the provider calls it, from `validateProperties` — one line in most connectors. Unknown keys are never rejected: the same map carries engine keys and storage keys, and `ALTER CATALOG` can only overwrite a key, never remove one, so a rejected unknown key could not be repaired. **B — `<Xxx>Conf`: deployment-level settings.** The keys of the plugin's own `<name>.conf`, each falling back to the `fe.conf` key it used to live under. Static accessors (`driversDir(context)`, `metastoreClientTimeoutSecond(context)`, …) rather than a bound object, because these belong to the deployment and not to any one catalog. Only the connectors that actually have such settings have one. **Per-flavor `*MetaStoreProperties` (iceberg / paimon): the keys of one metastore backend.** These already existed in `fe-connector-metastore-{iceberg,paimon}` but described themselves as "validation only" while the connector re-scanned the same keys to build the catalog. They are now the single declaration: they gained the fields the assembly needed and the getters it reads, and the factories consume the bound holder instead of `firstNonBlank(props, ALIASES)`. The alias arrays that duplicated them are deleted. The metastore modules stay SDK-free — they expose neutral getters and maps; engine-SDK option assembly stays in the connector factory. Splitting by flavor also splits the "annotation count == key count" invariant: the connector-level holder declares the flavor-independent keys, each backend holder declares its own. **F/G — literals the assembly emits, and mode enums.** Option keys and values that a connector *writes* (rather than a user setting them) are private to the class that writes them — `IcebergCatalogFactory` for the iceberg SDK dialect, and so on. Cache key names sit next to the cache they configure. In several places these had drifted into two spellings of the same string in two files; folding them removed those duplicates. ### Responsibilities, end to end | Layer | Reads | Writes | |---|---|---| | `<Xxx>ConnectorProvider` | — | `validateProperties` → `of(props).checkCreateTimeOnlyRules()` | | `<Xxx>CatalogProperties` | the raw map, once | typed getters; the derived flavor | | `*MetaStoreProperties` | the raw map, once, for its own backend | typed getters; neutral conf maps | | `<Xxx>CatalogFactory` | the bound holders (+ raw map only for copy-all / prefix passthroughs) | the engine-SDK option map | | `<Xxx>Conf` | `ConnectorContext` conf + environment | — | | connector / metadata / scan / write | the bound holders | — | The raw map is still read in three legitimate shapes, each commented where it happens: copy-all passthrough into the SDK options, whole-namespace forwarding (`fs.` / `dfs.` / `hadoop.`, `paimon.`, `jdbc.`), and alias sets that span namespaces and so belong to no single flavor (the S3 region aliases, the AWS credentials-provider mode). ### Verification Unit tests only — this is a refactor with no intended behavior change, and the guard against unintended change is a set of whole-map snapshot tests added *before* each rework: paimon 8 cases and iceberg 20 cases assert the ENTIRE catalog option map, one per flavor and per emission branch. Both SDKs silently ignore an option they do not recognize, so a dropped or misspelled key does not throw — it produces a catalog that connects with different settings than the operator asked for. Those tests stay in the tree afterwards as the permanent guard that the holder and the assembly agree. Every touched module, run together at the final commit with `-Dmaven.build.cache.enabled=false` (the build cache otherwise reports a stale green): | module | tests | module | tests | |---|---|---|---| | iceberg | 1215 (5 skip) | hudi | 203 | | paimon | 532 (1 skip) | adbc | 200 | | hive | 411 | maxcompute | 147 (1 skip) | | jdbc | 222 | es | 118 | | connector-spi | 140 | trino | 55 | | hms shared lib | 107 | metastore-{iceberg,paimon,spi,api} | 77 | | foundation | 161 | cache framework | 33 | **3780 tests, 0 failures, 0 errors, 7 skips** — every skip is a pre-existing live-connectivity test gated on environment variables, and each was already skipped before this PR. checkstyle clean; `ConnectorPluginSurfaceTest` green. ### Behavior changes Small, and all on inputs that are already degenerate. Full per-connector tables are in the commit messages; the classes of change are: 1. **Values are trimmed.** The property binder trims and the old hand-written scans did not, so a `uri` written with a trailing space was already *validated* trimmed while the catalog was *built* from the untrimmed string. The two now agree. For paimon HMS this removed an existing internal inconsistency (HiveConf got the trimmed value, the paimon `Options` got the raw one). 2. **Blank now means unset**, where old code used `containsKey` / `getOrDefault`. Affects e.g. `iceberg.rest.view-enabled = ""` (was false, now its default true) and `external_catalog.name = ""` (was an empty namespace level, now absent). 3. **Values the FE interprets are now sent downstream interpreted.** This fixed three real bugs where the FE parsed a value and then forwarded the unparsed original — the ES `http_ssl_enabled` payload to BE, the hive `uri` shorthand, and the trino `connector.name`. Numeric keys were audited per connector and deliberately left as Strings wherever the value is forwarded verbatim to an engine SDK, so that a catalog created with a value the SDK tolerates keeps building; the JDBC connection-pool knobs are the one place where the strict/lenient choice is made per key, with the reasoning in that commit. ### Release note None ### Check List (For Author) - Test - [x] Unit Test - [ ] Regression test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - Behavior changed: - [x] Yes. Whitespace around property values is now trimmed; an explicitly blank value now reads as unset rather than as an empty string; and three keys the FE interprets are now forwarded in their interpreted form. Details per connector are in the individual commit messages. - Does this need documentation? - [x] No. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pache#66515) ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] 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 <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#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 -->
…s}-dependencies Turn the existing `--exclude-obs-dependencies` / `--exclude-cos-dependencies` flags from a scope=provided downgrade (which still resolved the jars from their remote repositories, only skipping the bundling step) into a full exclusion: the dependencies are no longer resolved, compiled, or bundled. This is driven by active-by-default Maven profiles (`obs` / `cos`, deactivated via -Ddisable.obs=true / -Ddisable.cos=true) that wrap every touch point: - fe-core: the fe-filesystem-obs / fe-filesystem-cos test couplings. - hadoop-deps: the hadoop-huaweicloud dependency and the Huawei OBS repository. - fe-filesystem: the fe-filesystem-obs / fe-filesystem-cos modules. build.sh maps the flags to -Ddisable.obs=true / -Ddisable.cos=true and drops the corresponding provider from both the -pl reactor list and the plugin packaging loop, so the build stays consistent when a provider is excluded. Default builds are unchanged (profiles active unless the flag is passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RoanHeNaN
force-pushed
the
feat/disable-huaweicloud
branch
from
August 7, 2026 02:02
1c700d5 to
52d3bc4
Compare
RoanHeNaN
changed the title
RoanHeNaN
pushed a commit
that referenced
this pull request
Aug 21, 2026
…projects (apache#66531) ### What problem does this PR solve? Related PR: apache#63690 Problem Summary: Eager aggregation pushdown may fail when different aggregate functions become the same expression after passing through a Project. report error: ```text 2026-08-06 04:26:54,285 INFO (mysql-nio-pool-14|325) [PushDownAggregation.visitLogicalAggregate():280] PushDownAggregation failed: Cannot invoke "org.apache.doris.nereids.trees.expressions.NamedExpression.toSlot()" because "namedExpression" is null at org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalProject(EagerAggRewriter.java:718) at org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalProject(EagerAggRewriter.java:90) at org.apache.doris.nereids.trees.plans.logical.LogicalProject.accept(LogicalProject.java:160) at org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalUnion(EagerAggRewriter.java:582) at org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalUnion(EagerAggRewriter.java:90) at org.apache.doris.nereids.trees.plans.logical.LogicalUnion.accept(LogicalUnion.java:155) ``` For example: ```text Aggregate: SUM(x)#4, SUM(y)#5 Union All Project: 0 AS x, 0 AS y Join ``` After pushing the aggregates through the Project, both functions become`SUM(0)`: ```text functions: [SUM(0), SUM(0)] aliasMap: SUM(0) -> #5 ``` Because `aliasMap` uses expression equality, only one entry is retained. The Project still tries to read both `#4` and `#5` from `BilateralState`, causing a null lookup. This PR deduplicates the child aggregate and records the ExprId mapping: ```text child aggregate: SUM(0) -> apache#8 ExprId mapping: #4 -> apache#8, #5 -> apache#8 ``` The Project then restores both required outputs: ```text slot#8 AS slot#4 slot#8 AS slot#5 ``` When no aggregate functions are merged, the original ExprIds are reused to avoid unnecessary aliases. The same fix also covers cases such as: ```text Project: a#1 AS x, a#1 AS y ``` where `SUM(x)` and `SUM(y)` both become `SUM(a#1)` after pushdown. ### Release note None ### Check List (For Author) - Test - [x] Regression test - `query_p0/eager_agg/bilateral_eager_agg` - Covers two aggregate functions that become the same function after Project pushdown. - [ ] Unit Test - [ ] Manual test - [ ] No need to test or manual test. - Behavior changed: - [x] Yes. - Prevents eager aggregation pushdown from failing when multiple aggregate functions become identical after Project rewriting. - `eager_aggregation_mode=1` can force eligible pushdown after a UNION. - 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
RoanHeNaN
pushed a commit
that referenced
this pull request
Sep 11, 2026
Problem Summary: Streaming `ROWS` window aggregates retain state across
frame evaluations. The eviction path previously considered only whether
buffered blocks had been emitted, so it could erase either the outgoing
row needed by a bounded sliding frame or the next unread row needed by
an `UNBOUNDED PRECEDING ... N PRECEDING` frame. After rebasing, negative
partition and outgoing positions could allow a nullable aggregate to
access its null map out of bounds. Evicting either kind of required row
could also produce incorrect aggregate results.
Root cause: `_remove_unused_rows()` did not account for the earliest row
required by the next ROWS frame evaluation, and
`BoundaryPose::remove_unused_rows()` allowed retained-column coordinates
to become negative.
This change defers block-aligned eviction when the candidate prefix
contains either `frame_start - 1`, the outgoing row required by a
bounded sliding update, or the next unread row required by an `UNBOUNDED
PRECEDING ... N PRECEDING` frame. It also rebases partition and order
boundaries to nonnegative physical-column coordinates. The BE unit
coverage exercises both ROWS executors across eviction boundaries and
verifies boundary rebasing.
Observed ASAN failure before this change (`output/be/log/be.out`):
```text
ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1
#0 doris::AggregateFunctionNullUnaryInlineV2<...>::execute_function_with_incremental(...)
be/src/exprs/aggregate/aggregate_function_null_v2.h:595
#1 doris::AggFnEvaluator::execute_function_with_incremental(...)
be/src/exprs/vectorized_agg_fn.cpp:334
#2 doris::AnalyticSinkLocalState::_execute_for_function<true>(...)
be/src/exec/operator/analytic_sink_operator.cpp:385
#3 doris::AnalyticSinkLocalState::_get_next_for_sliding_rows(...)
be/src/exec/operator/analytic_sink_operator.cpp:203
#4 doris::AnalyticSinkLocalState::_execute_impl(...)
be/src/exec/operator/analytic_sink_operator.cpp:358
#5 doris::AnalyticSinkOperatorX::sink_impl(...)
be/src/exec/operator/analytic_sink_operator.cpp:757
SUMMARY: AddressSanitizer: heap-buffer-overflow in
doris::AggregateFunctionNullUnaryInlineV2<...>::execute_function_with_incremental(...)
```
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#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 -->
RoanHeNaN
pushed a commit
that referenced
this pull request
Sep 29, 2026
…erived group by projection as a group by key (apache#67362) ### What problem does this PR solve? Problem Summary: Given a sync materialized view on `sync_tz_base`: ```sql CREATE MATERIALIZED VIEW sync_tz_day AS SELECT date_trunc(ts, 'day') AS day_ts, sum(v) AS day_sum FROM sync_tz_base WHERE ts IS NOT NULL GROUP BY date_trunc(ts, 'day'); ``` the following query fails during planning with an NPE: ```sql SELECT CAST(date_trunc(ts, 'day') AS STRING) AS day_ts, SUM(v) FROM sync_tz_base WHERE ts IS NOT NULL GROUP BY date_trunc(ts, 'day'); ``` ``` java.lang.NullPointerException: Cannot invoke "org.apache.doris.analysis.Expr.getChildren()" because "root" is null at org.apache.doris.analysis.Expr.extractSlots(Expr.java:173) at org.apache.doris.nereids.glue.translator.PhysicalPlanTranslator.visitPhysicalProject(PhysicalPlanTranslator.java:2173) ``` **Root cause** In `AbstractMaterializedViewAggregateRule.aggregateRewriteByView`, the group by keys of the rewritten aggregate were collected from the query **top plan** output expressions. For a `project -> aggregate` structure, `topPlanSplitToGroupAndFunction` classifies the derived projection `cast(date_trunc(ts, 'day') AS STRING)` as a group-by expression, because it is derived from the real group key. This derived projection was then wrongly added as a group by key of the rewritten aggregate: ``` finalGroupExpressions = [cast(day_ts#7 as TEXT) AS apache#9, day_ts#7] finalOutputExpressions = [cast(day_ts#7 as TEXT) AS apache#9, sum(day_sum#8) AS apache#10] ``` This led to two problems: 1. A redundant group by key `cast(day_ts#7 as TEXT)` which is only a projection of the real group key `day_ts#7`. 2. The real group key `day_ts#7` was added by the group-by compensation logic but was **not** present in the aggregate output expressions, so the top project referenced a slot that the physical aggregate never produced, causing the `"root" is null` NPE during physical plan translation. **Fix** The rewritten aggregate is now built directly from the query bottom aggregate: - The group by keys are the query bottom aggregate's group by expressions rewritten against the MV scan (always correct, so the previous group-by compensation is no longer needed). - The aggregate output contains the rewritten group keys and the rolled-up aggregate functions. - The query top plan output expressions — including derived projections of group keys such as `cast(date_trunc(ts, 'day') AS STRING)` — are recomputed by a `LogicalProject` above the rewritten aggregate when they cannot be produced by the aggregate directly. After the fix the rewritten plan is valid and the query returns correct results: ``` Project [cast(day_ts#7 as TEXT) AS day_ts#3, sum(day_sum#8) AS SUM(v)#4] Aggregate [group by [day_ts#7], output [day_ts#7, sum(day_sum#8)]] MVScan(sync_tz_day) ```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Turn the existing
--exclude-obs-dependenciesand--exclude-cos-dependenciesbuild flags into a full exclusion of the respective cloud provider (Huawei OBS / Tencent COS): when passed, nothing from that provider is resolved, compiled, or bundled into the FE binary.Why
The two flags were introduced in apache#58071 as a
scope=provideddowngrade. That only kept the jars out of the shipped artifact — Maven still resolved and downloaded them at build time. In particularcom.huaweicloud:hadoop-huaweicloud:3.1.1-hw-46is published only on Huawei's own Maven repo (repo.huaweicloud.com). Environments that cannot or must not reach it (corporate proxies blocking the host, or compliance constraints forbidding Huawei/Tencent artifacts) still could not build the FE, because resolution was unconditional. The flags now do what their names imply.How
Active-by-default Maven profiles
obs/cos, deactivated via-Ddisable.obs=true/-Ddisable.cos=true, wrap every touch point:fe-filesystem-obs/fe-filesystem-costest couplings.hadoop-huaweiclouddependency and the Huawei OBS Maven repository.fe-filesystem-obs/fe-filesystem-cosmodules (which transitively pull the Huawei / Tencent SDKs).build.shmaps--exclude-obs-dependencies→-Ddisable.obs=trueand--exclude-cos-dependencies→-Ddisable.cos=true(replacing the old*.dependency.scope=providedmapping), and drops the corresponding provider from both the-plreactor list and the filesystem plugin packaging loop, so the reactor and the dist layout stay consistent when a provider is excluded.Behaviour
sh build.sh --fe --exclude-obs-dependenciesbuilds the FE with no Huawei artifact resolved and no OBS code compiled or bundled; same for--exclude-cos-dependenciesand Tencent COS.Testing
mvn help:active-profilesonfe-core,fe-filesystem, andhadoop-deps:obs/cosactive by default, and absent with-Ddisable.obs=true/-Ddisable.cos=true.compileandtest-compilestay green when the profiles are off.xmllinton all changed poms andbash -n build.shpass.🤖 Generated with Claude Code