github-actions[bot] commented on code in PR #65329:
URL: https://github.com/apache/doris/pull/65329#discussion_r3610936128
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructField.java:
##########
@@ -85,7 +86,8 @@ public org.apache.doris.catalog.StructField
toCatalogDataType() {
public String toSql() {
return name + ":" + dataType.toSql()
+ (nullable ? "" : " NOT NULL")
- + (comment.isEmpty() ? "" : " COMMENT " + comment);
+ + (comment.isEmpty() ? "" : " COMMENT "
Review Comment:
[P2] Quote STRUCT member identifiers in the renderer
This renderer now makes nested complex definitions round-trippable for
comments, but it still writes the decoded member `name` verbatim. The
identifier post-processor removes quoting first, so a valid type such as
``STRUCT<`Metric``Name`:STRING>`` is stored with a literal backtick and
rendered as invalid SQL; reserved member names such as `key` have the same
problem. Please render the member through `SqlUtils.getIdentSql` and add a
nested complex ADD/MODIFY `toSql()`/reparse case with an escaped or reserved
name.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -901,42 +1015,73 @@ public void renameColumn(ExternalTable dorisTable,
String oldName, String newNam
refreshTable(dorisTable, updateTime);
}
+ @Override
+ public void renameColumn(ExternalTable dorisTable, ColumnPath columnPath,
String newName, long updateTime)
+ throws UserException {
+ if (!columnPath.isNested()) {
+ renameColumn(dorisTable, columnPath.getTopLevelName(), newName,
updateTime);
+ return;
+ }
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ ResolvedColumnPath resolvedPath =
validateNestedStructFieldPath(icebergTable.schema(), columnPath, "rename");
+ ResolvedColumnPath parentPath =
resolveColumnPath(icebergTable.schema(), columnPath.getParentPath(), "rename");
+
validateNoCaseInsensitiveSiblingCollision(parentPath.getType().asStructType(),
+ parentPath.getColumnPath(), newName, resolvedPath.getField(),
"rename");
+
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ updateSchema.renameColumn(resolvedPath.getFullPath(), newName);
+ try {
+ executionAuthenticator.execute(() -> updateSchema.commit());
+ } catch (Exception e) {
+ throw new UserException("Failed to rename nested column: " +
columnPath.getFullPath() + " to " + newName
+ + " in table: " + icebergTable.name() + ", error message
is: " + e.getMessage(), e);
+ }
+ refreshTable(dorisTable, updateTime);
+ }
+
@Override
public void modifyColumn(ExternalTable dorisTable, Column column,
ColumnPosition position, long updateTime)
throws UserException {
+ modifyTopLevelColumn(dorisTable, column, position, updateTime, true);
+ }
+
+ private void modifyTopLevelColumn(ExternalTable dorisTable, Column column,
ColumnPosition position,
+ long updateTime, boolean legacyMode) throws UserException {
Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
- NestedField currentCol =
icebergTable.schema().findField(column.getName());
+ validateRowLineageColumnMutation(icebergTable, column.getName(),
"modify");
+ NestedField currentCol =
icebergTable.schema().caseInsensitiveFindField(column.getName());
if (currentCol == null) {
throw new UserException("Column " + column.getName() + " does not
exist");
}
+ ResolvedColumnPath columnPath = new
ResolvedColumnPath(ColumnPath.of(currentCol.name()),
Review Comment:
[P1] Preserve the resolved top-level component
This canonicalization can turn a quoted one-component name into a different
column. With top-level `a STRUCT<b:INT>` and `b INT`, ``MODIFY COLUMN `a.b`
BIGINT`` passes the single name `a.b`; `Schema.caseInsensitiveFindField`
resolves nested `a.b`, but rebuilding from `currentCol.name()` produces `b`, so
the subsequent UpdateSchema call commits the promotion on unrelated top-level
`b`. Please resolve the original one-component path against `schema.asStruct()`
and carry that component through, with a regression covering this
nested/top-level collision.
--
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]
