github-actions[bot] commented on code in PR #67166:
URL: https://github.com/apache/doris/pull/67166#discussion_r3862726983
##########
fe/fe-type/src/main/java/org/apache/doris/catalog/StructField.java:
##########
@@ -51,7 +54,25 @@ public StructField(String name, Type type, String comment,
boolean containsNull)
public StructField(String name, Type type, String comment, boolean
containsNull,
boolean commentSpecified) {
+ this(name, name, type, comment, containsNull, commentSpecified);
+ }
+
+ /**
+ * Creates a field with separate names for case-insensitive runtime lookup
and external schema spelling.
+ *
+ * @param name field name normalized internally for runtime lookup
+ * @param originalName field spelling preserved for external schema
metadata
+ * @param type field type
+ * @param comment field comment
+ * @param containsNull whether the field accepts null values
+ * @param commentSpecified whether the comment was explicitly specified
+ */
+ public StructField(String name, String originalName, Type type, String
comment, boolean containsNull,
+ boolean commentSpecified) {
this.name = name.toLowerCase();
+ // Runtime struct lookup is case-insensitive, but external schemas
such as Iceberg must preserve the
+ // original spelling. Keep both so metadata writes never leak the
normalized lookup key.
+ this.originalName = originalName;
Review Comment:
[P1] Preserve FILE-TVF spelling before constructing this field
This can preserve only the constructor argument. In `CREATE TABLE
iceberg_catalog... AS SELECT payload FROM S3(...)`, BE sends the raw Parquet
child spelling in `PStructField`, but
`ExternalFileTableValuedFunction.getColumnType()` immediately lowercases
`structField.getName()` and passes that value to the legacy constructor. Both
`name` and `originalName` are therefore already lowercase; the structured CTAS
path reaches `ConnectorColumnConverter` and `IcebergSchemaBuilder` with no raw
spelling to recover, so `CaseSensitive` is silently persisted as
`casesensitive` (also for STRUCT under ARRAY/MAP). Please construct the field
with the raw protobuf spelling, use the lowercase form only for duplicate
detection, and cover FILE TVF to external Iceberg CTAS.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergComplexTypeDiff.java:
##########
@@ -172,10 +172,11 @@ private static void applyStructChange(UpdateSchema
updateSchema, String path,
Types.NestedField oldField = oldFields.get(i);
Types.NestedField newField = newFields.get(i);
String fieldPath = path + "." + oldField.name();
- existingNames.add(oldField.name());
+ existingNames.add(oldField.name().toLowerCase(Locale.ROOT));
- // Legacy ColumnType rule: existing fields are matched by position
and may not be renamed.
- if (!oldField.name().equals(newField.name())) {
+ // A full-type MODIFY is case-insensitive and must preserve
existing Iceberg spelling; only the
+ // dedicated RENAME operation is allowed to change a field name.
+ if (!oldField.name().equalsIgnoreCase(newField.name())) {
Review Comment:
[P1] Match using Iceberg's lowercase-key identity
`equalsIgnoreCase` is broader than the `toLowerCase(Locale.ROOT)` identity
used by Iceberg and by `existingNames`. For example, Java treats `Σ` and `ς` as
equal ignoring case even though their ROOT-lowercase keys (`σ` and `ς`) are
distinct and Iceberg permits them as sibling fields. With existing
`STRUCT<Σ:INT, ς:INT>` and requested `STRUCT<ς:BIGINT, Σ:INT>`, both positional
checks pass, so the canonical old path makes `UpdateSchema` commit BIGINT to
`Σ` instead of rejecting the apparent swap; comment and nullability changes can
be misapplied the same way. Please compare the same ROOT-lowercase key used by
Iceberg and the collision set, and add an InMemoryCatalog regression for this
boundary.
--
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]
