github-actions[bot] commented on code in PR #70289:
URL: https://github.com/apache/doris/pull/70289#discussion_r4480204638
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPlanUtil.java:
##########
@@ -567,8 +565,9 @@ public static MTMVAnalyzeQueryInfo analyzeQueryWithSql(MTMV
mtmv, ConnectContext
mtmvPartitionDefinition.setPartitionType(mvPartitionInfo.getPartitionType());
Expr expr = mvPartitionInfo.getExpr();
if (expr != null) {
+ // 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)));
Review Comment:
[P2] Preserve the raw name of dotted partition outputs
A supported implicit partition definition can use a dotted output alias:
```sql
PARTITION BY(`q.day`) AS
SELECT dt, date_trunc(dt, 'day') AS `q.day`, v FROM src
```
Creation 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.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPlanUtil.java:
##########
@@ -567,8 +565,9 @@ public static MTMVAnalyzeQueryInfo analyzeQueryWithSql(MTMV
mtmv, ConnectContext
mtmvPartitionDefinition.setPartitionType(mvPartitionInfo.getPartitionType());
Expr expr = mvPartitionInfo.getExpr();
if (expr != null) {
+ // 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)));
Review 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:
```sql
PARTITION BY(`q``day`) AS
SELECT dt, date_trunc(dt, 'day') AS `q``day`, v FROM src
```
The 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:
```text
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.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVPlanUtil.java:
##########
@@ -567,8 +565,9 @@ public static MTMVAnalyzeQueryInfo analyzeQueryWithSql(MTMV
mtmv, ConnectContext
mtmvPartitionDefinition.setPartitionType(mvPartitionInfo.getPartitionType());
Expr expr = mvPartitionInfo.getExpr();
if (expr != null) {
+ // 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)));
Review Comment:
[P2] Avoid adding an unmergeable WEEK/QUARTER truncation
For a range-partitioned DATETIME source with day-sized partitions, this
implicit definition is supported:
```sql
PARTITION BY(q) AS
SELECT dt, date_trunc(dt, 'week') AS q, v FROM src
```
The 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:
```text
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
