github-actions[bot] commented on code in PR #65329:
URL: https://github.com/apache/doris/pull/65329#discussion_r3597962871
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -4234,6 +4242,102 @@ public ColumnDefinition visitColumnDef(ColumnDefContext
ctx) {
onUpdateDefaultValue, comment, desc);
}
+ @Override
+ public ColumnDefinitionWithPath
visitColumnDefWithPath(ColumnDefWithPathContext ctx) {
+ ColumnPath columnPath = ColumnPath.of(visitQualifiedName(ctx.colName));
+ String colName = columnPath.getLeafName();
+ DataType colType = ctx.type instanceof PrimitiveDataTypeContext
+ ? visitPrimitiveDataType(((PrimitiveDataTypeContext) ctx.type))
+ : ctx.type instanceof ComplexDataTypeContext
+ ? visitComplexDataType((ComplexDataTypeContext)
ctx.type)
+ : ctx.type instanceof VariantPredefinedFieldsContext
+ ?
visitVariantPredefinedFields((VariantPredefinedFieldsContext) ctx.type)
+ : visitAggStateDataType((AggStateDataTypeContext)
ctx.type);
+ colType = colType.conversion();
+ boolean isKey = ctx.KEY() != null;
+ ColumnNullableType nullableType = ColumnNullableType.DEFAULT;
+ if (ctx.NOT() != null) {
+ nullableType = ColumnNullableType.NOT_NULLABLE;
+ } else if (ctx.nullable != null) {
+ nullableType = ColumnNullableType.NULLABLE;
+ }
+ String aggTypeString = ctx.aggType != null ? ctx.aggType.getText() :
null;
+ Optional<DefaultValue> defaultValue = Optional.empty();
+ Optional<DefaultValue> onUpdateDefaultValue = Optional.empty();
+ if (ctx.DEFAULT() != null) {
+ if (ctx.INTEGER_VALUE() != null) {
+ if (ctx.SUBTRACT() == null) {
+ defaultValue = Optional.of(new
DefaultValue(ctx.INTEGER_VALUE().getText()));
+ } else {
+ defaultValue = Optional.of(new DefaultValue("-" +
ctx.INTEGER_VALUE().getText()));
+ }
+ } else if (ctx.DECIMAL_VALUE() != null) {
+ if (ctx.SUBTRACT() == null) {
+ defaultValue = Optional.of(new
DefaultValue(ctx.DECIMAL_VALUE().getText()));
+ } else {
+ defaultValue = Optional.of(new DefaultValue("-" +
ctx.DECIMAL_VALUE().getText()));
+ }
+ } else if (ctx.stringValue != null) {
+ defaultValue = Optional.of(new
DefaultValue(toStringValue(ctx.stringValue.getText())));
Review Comment:
[P1] Decode path-aware DEFAULT and COMMENT literals
`toStringValue` only removes the outer quotes, so a valid nested add such as
`ADD COLUMN s.c STRING NULL DEFAULT 'owner''s'` reaches Iceberg as the literal
value `owner''s` rather than `owner's`; the manual COMMENT branch likewise
preserves doubled quotes and removes every backslash. Because this new path
forwards those strings directly to `UpdateSchema`, the wrong initial default or
field doc is committed. Please decode both tokens with the SQL-mode-aware
`parseStringLiteral` helper and cover doubled quotes/backslashes for a nested
ADD.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -855,12 +898,56 @@ public void addColumn(ExternalTable dorisTable, Column
column, ColumnPosition po
refreshTable(dorisTable, updateTime);
}
+ @Override
+ public void addColumn(ExternalTable dorisTable, ColumnPath columnPath,
Column column, ColumnPosition position,
+ long updateTime) throws UserException {
+ if (!columnPath.isNested()) {
+ addColumn(dorisTable, column, position, updateTime);
+ return;
+ }
+ validateAddColumnMetadata(column);
+ if (!column.isAllowNull()) {
+ throw new UserException("New nested field '" +
columnPath.getFullPath() + "' must be nullable");
+ }
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ ResolvedColumnPath parentPath =
resolveColumnPath(icebergTable.schema(), columnPath.getParentPath(), "add");
+ if (!parentPath.getType().isStructType()) {
+ throw new UserException("Parent column path '" +
columnPath.getParentPathString()
+ + "' is not a struct in Iceberg table: " +
icebergTable.name());
+ }
+
validateNoCaseInsensitiveSiblingCollision(parentPath.getType().asStructType(),
+ parentPath.getColumnPath(), columnPath.getLeafName(), null,
"add");
+
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ org.apache.iceberg.types.Type dorisType =
IcebergUtils.dorisTypeToIcebergType(column.getType());
+ Literal<?> defaultValue = IcebergUtils.parseIcebergLiteral(
+ column.getDefaultValue(), column.getType(), dorisType);
+ updateSchema.addColumn(parentPath.getFullPath(),
columnPath.getLeafName(), dorisType,
Review Comment:
[P1] Preserve nested initial defaults in scan metadata
This commits the default on the nested Iceberg field, but
`IcebergUtils.parseSchema` only copies `initialDefault()` from top-level
`schema.columns()`. Child `Column`s are created without defaults, and
`ExternalUtil` can only recover nested defaults from a side map limited to
UUID/BINARY/FIXED. Therefore, after writing old rows and running `ADD COLUMN
s.c INT NULL DEFAULT 7` on a v3 table, Doris has no scan metadata for the
required value of `s.c` in those old files and materializes the nullable
fallback instead. Please propagate every nested field's initial default by
field ID into the child scan schema, have both legacy and v2 nested
missing-child materialization consume it, and add an old-file read regression.
--
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]
