github-actions[bot] commented on code in PR #67166:
URL: https://github.com/apache/doris/pull/67166#discussion_r3922039767
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java:
##########
@@ -555,24 +559,25 @@ public void modifyColumn(String dbName, String tableName,
IcebergColumnChange co
throw new DorisConnectorException("Modify column type
from complex to primitive is not"
+ " supported: " + column.getName());
}
- updateSchema.updateColumn(column.getName(),
newType.asPrimitiveType(), targetComment);
+ updateSchema.updateColumn(currentName,
newType.asPrimitiveType(), targetComment);
} else {
// A complex (STRUCT/ARRAY/MAP) modify diffs the new type
against the current one field-by-field
// (IcebergComplexTypeDiff); the top-level column doc is
updated separately, as in legacy.
if (current.type().isPrimitiveType()) {
throw new DorisConnectorException("Modify column type
from non-complex to complex is not"
+ " supported: " + column.getName());
}
- IcebergComplexTypeDiff.apply(updateSchema,
column.getName(), current.type(), newType,
+ IcebergComplexTypeDiff.apply(updateSchema, currentName,
current.type(), newType,
column.getSourceType());
if (!Objects.equals(current.doc(), targetComment)) {
- updateSchema.updateColumnDoc(column.getName(),
targetComment);
+ updateSchema.updateColumnDoc(currentName,
targetComment);
}
}
if (column.isNullable()) {
- updateSchema.makeColumnOptional(column.getName());
+ updateSchema.makeColumnOptional(currentName);
}
- applyPosition(updateSchema, position, column.getName());
+ IcebergNestedColumnEvolution.applyTopLevelPosition(
Review Comment:
[P1] Canonicalize the other top-level positioning paths too. This fixes
MODIFY by resolving both the target and AFTER sibling to persisted Iceberg
spellings, but ADD still routes through `applyPosition`, which passes
`position.getAfterColumn()` verbatim, and `reorderColumns` still passes every
caller-provided name directly to the case-sensitive `moveFirst`/`moveAfter`
APIs. On a table with stored fields `Id`/`Label`, `ADD ... AFTER id` and a
reorder using `label`/`id` therefore still fail even though Doris resolves
external columns case-insensitively. Please reuse schema-backed
canonicalization for ADD and resolve the complete reorder list before staging
it, with mixed-case catalog-backed tests.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java:
##########
@@ -622,6 +630,20 @@ public Expression visitBoundFunction(BoundFunction
boundFunction, ExpressionRewr
return TypeCoercionUtils.processBoundFunction(boundFunction);
}
+ private ElementAt canonicalizeStructSelector(ElementAt elementAt) {
+ Expression left = elementAt.left();
+ Expression right = elementAt.right();
+ if (left.getDataType() instanceof StructType && right instanceof
StringLikeLiteral) {
Review Comment:
[P1] Unwrap bracket-expression variables before canonicalizing. The parser
builds `payload[@field]` directly as `ElementAt`, and `ElementAt` legality/type
resolution uses `getArgument(1)`, so a user variable whose value is displayed
Unicode `Σ` is accepted. This helper checks the raw right child, sees
`Variable`, and skips the rewrite; `VariableToLiteral` later exposes `Σ`, but
only integer selectors are normalized afterward. BE then receives `Σ` against
the thrift runtime name `σ` and fails lookup. Please canonicalize the
wrapper-aware argument while preserving variable-use/SQL-cache accounting, and
add a variable-backed bracket execution test. Function syntax is already
unwrapped by `constructUnboundFunctionArguments`.
--
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]
