Repository navigation
Conversation
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Refresh-time schema validation parses a stored base-table partition expression as if it referred to MV query outputs. When a date-trunc output has an alias, reanalysis fails to find the base column in those outputs. Reuse the SHOW CREATE expression renderer to reconstruct the expression using the MV output column, preserving the stored lineage and existing compatibility checks. ### Release note Fix asynchronous materialized view refresh failures after base-table schema changes when a date partition expression uses a query output alias. ### Check List (For Author) - Test: Unit Test: MTMVPartitionReanalysisTest and MTMVPlanUtilTest (29 passed); FE build including Checkstyle passed. End-to-end refresh execution was not run locally. - Behavior changed: Yes, compatible schema changes no longer cause a spurious partition-column lookup failure. - Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review of PR #70289 at 2e57ef1 found three P2 regressions in partition-expression reconstruction: WEEK/QUARTER outputs gain an unsupported nested truncation, dotted output aliases fail name lookup, and embedded backticks produce invalid expression SQL. Each inline example retains the ordinary base column in the SELECT list, so the previous reanalysis path succeeds. These are distinct introduced failures; please address them before merging. No P0/P1 finding or existing blocking comment was identified. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: output-namespace reconstruction fixes the intended ordinary DAY alias mismatch, but the three supported cases above still regress. The four added parameterized cases exercise the real refresh compatibility gate, with single-source/aggregate-over-UNION ALL and implicit/explicit definitions.
- Scope and reuse: the production change is small and reuses the existing SHOW CREATE expression service; its assumptions need the corrections described inline.
- Concurrency and locking: refresh reanalysis still runs under the existing base-table read locks acquired in ID order. New objects are local; no new lock, shared mutation, external IO or journal wait was added.
- Lifecycle and initialization: reanalysis builds separate analysis objects and does not mutate stored lineage. No new long-lived resource, callback or static-initialization dependency was introduced.
- Configuration: no configuration was added or changed. Unicode naming is already enabled by default, making the embedded-backtick example reachable with defaults.
- Compatibility: serialized fields, storage formats and function signatures are unchanged; legacy single-PCT adaptation remains. Behavioral compatibility of existing MVs is affected by the three findings.
- Thrift/Protobuf: no definitions changed; branch field-ID checks are inapplicable.
- Parallel paths: both analyzeQueryWithSql overloads, refresh-time schema validation, DDL-triggered invalidation and IVM property validation share the changed implementation. Null-expression and self-managed paths are unchanged.
- Conditional checks and errors: the existing non-null expression guard remains appropriate. Factory/parser/analysis errors still propagate through UserException/JobException; the findings cause valid definitions to take those failure paths.
- Test coverage: the new tests meaningfully cover DAY aliases and unchanged persisted lineage. They omit WEEK/QUARTER and quoted-identifier cases. Their VARCHAR widening uses synchronous light schema change; no new timing race was established. Existing tests cover unusable/changed partition lineage rejection. Full task execution, returned rows and the additional failing cases were not executed in this static review.
- Expected results: no generated result files changed. Assertions and setup were inspected; author-reported build/test results were not independently executed.
- Observability: existing task warnings and DDL invalidation logs include the MV/task or base-table context and failure cause; no additional instrumentation is required by this local change.
- Transactions and persistence: no EditLog or replay logic changed, and existing column/PCT/time-unit compatibility checks remain after reanalysis. No new persistence or failover obligation was introduced.
- Writes, atomicity and crashes: no write protocol changed. Visibility versions, delete bitmaps and refresh transaction atomicity are untouched; validation failures still stop refresh.
- FE/BE propagation: no transmitted variable or wire contract changed.
- Performance: the additional short-lived service and expression string occur per reanalysis, with no new row/partition loop or asymptotic cost. No benchmark was run.
- Other risks: explicit MV column lists, multi-PCT lineage, factory restrictions and legacy metadata were traced without another introduced issue. Same-name base/output backtick serialization was already broken; the separate alias example in the inline finding was previously working.
Both changed files, direct callers, analysis/normalization and compatibility checks were covered. All candidate and risk dispositions are recorded in the review ledger; no unresolved static-review question remains. Review convergence and round count are reported with this delivery. Validation was static only: no builds, tests, service execution, source edits or submodule initialization.
Review converged after 2 round(s).
| // The stored expression references base-table columns; reanalysis starts from MV output columns. | ||
| mtmvPartitionDefinition.setFunctionCallExpression(new NereidsParser().parseExpression( | ||
| expr.accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE))); | ||
| MTMVPartitionExprFactory.getExprService(expr).toSql(mvPartitionInfo))); |
There was a problem hiding this comment.
[P2] Avoid adding an unmergeable WEEK/QUARTER truncation
For a range-partitioned DATETIME source with day-sized partitions, this implicit definition is supported:
PARTITION BY(q) AS
SELECT dt, date_trunc(dt, 'week') AS q, v FROM srcThe stored expression is date_trunc(dt, 'week'); because raw dt is also projected, the previous reanalysis resolves it and succeeds. This renderer instead supplies date_trunc(q, 'week'), producing this lineage:
Project(date_trunc(q, 'week'))
Project(dt, q = date_trunc(dt, 'week'), v)
Scan(src)
PartitionIncrementMaintainer expands that to two nested truncations. MergeDateTrunc explicitly skips WEEK and QUARTER, including identical units, and checkPartition rejects expressions containing more than one DateTrunc. Consequently a compatible VARCHAR widening now makes refresh reanalysis fail for an existing MV. Preserve inferred-expression semantics or normalize these identical-unit pairs safely, and cover both units; all four new cases use DAY.
| // The stored expression references base-table columns; reanalysis starts from MV output columns. | ||
| mtmvPartitionDefinition.setFunctionCallExpression(new NereidsParser().parseExpression( | ||
| expr.accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE))); | ||
| MTMVPartitionExprFactory.getExprService(expr).toSql(mvPartitionInfo))); |
There was a problem hiding this comment.
[P2] Preserve the raw name of dotted partition outputs
A supported implicit partition definition can use a dotted output alias:
PARTITION BY(`q.day`) AS
SELECT dt, date_trunc(dt, 'day') AS `q.day`, v FROM srcCreation stores raw q.day as partitionCol and infers the base expression date_trunc(dt, 'day'). Keeping raw dt in the output lets the previous reanalysis succeed. The new renderer emits a function over the quoted alias, so MTMVPartitionDefinition.analyzeAndTransferToMTMVPartitionInfo now obtains the name through UnboundSlot.getName(). That method adds literal backticks around any name part containing a period. getRelatedTableInfos then compares the backtick-bearing string against the raw output name and fails with partition column can not find from sql select column. Preserve the raw single identifier part in this reconstruction path and add an implicit dotted-alias reanalysis case. This fails for DAY independently of nested-truncation normalization.
| // The stored expression references base-table columns; reanalysis starts from MV output columns. | ||
| mtmvPartitionDefinition.setFunctionCallExpression(new NereidsParser().parseExpression( | ||
| expr.accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE))); | ||
| MTMVPartitionExprFactory.getExprService(expr).toSql(mvPartitionInfo))); |
There was a problem hiding this comment.
[P2] Escape embedded backticks before reparsing the output alias
The reused MTMVPartitionExprDateTrunc.toSql wraps partitionCol in backticks without escaping its contents. Embedded backticks are legal with the default Unicode naming setting, so this implicit definition can be created:
PARTITION BY(`q``day`) AS
SELECT dt, date_trunc(dt, 'day') AS `q``day`, v FROM srcThe old path reparses the stored expression over ordinary dt, which is also a query output, and succeeds. The new path interpolates the raw alias into the SQL text, producing malformed expression SQL:
date_trunc(`q`day`, 'day')
parseExpression now fails before compatibility validation, preventing schema-change refresh for this existing MV. Quote the identifier with the existing escaping utility, or build the unbound expression directly. This is separate from dotted-name handling: correcting UnboundSlot.getName() does not repair malformed SQL, and escaping embedded backticks does not fix the dotted-name lookup mismatch.
TPC-H: Total hot run time: 28525 ms |
TPC-DS: Total hot run time: 154070 ms |
ClickBench: Total hot run time: 24.29 s |
Background
Partitioned asynchronous materialized views retain both their query output partition column and the base-table expression inferred from partition lineage. A base-table schema change causes refresh to reanalyze the stored MV query and validate its compatibility.
What problem does this PR solve?
Issue Number: None
Related PR: None
PR Type: Bugfix
Problem Summary:
After a base-table schema change, refreshing an MV whose date partition output has an alias can fail with
partition column can not find from sql select column. Reanalysis parses the stored base-table expression and incorrectly searches for its base column in the query outputs. Render the partition expression using the MV output column, reusing the existing SHOW CREATE expression service, before parsing it again. Existing MVs benefit without metadata migration or changes to schema compatibility checks.Details
MTMVPartitionExprFactory.getExprService(expr).toSql(mvPartitionInfo)to reconstruct the analysis input. Do not mutate stored lineage or bypass partition/type compatibility checks.Testing
./build.sh --fe: passed, including FE Checkstyle../run-fe-ut.sh --run MTMVPartitionReanalysisTest,MTMVPlanUtilTest: 29 tests passed, with no failures or skips.partition column can not find from sql select column; with the fix all four pass.Release note
Fix asynchronous materialized view refresh failures after base-table schema changes when a date partition expression uses a query output alias.
Check List (For Author)