diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java index 91d6c9b1c9376e..3b55a98be7cad3 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergCatalogOps.java @@ -531,10 +531,14 @@ public void renameColumn(String dbName, String tableName, String oldName, String public void modifyColumn(String dbName, String tableName, IcebergColumnChange column, boolean commentSpecified, ConnectorColumnPosition position) { withTable(dbName, tableName, table -> { - Types.NestedField current = table.schema().findField(column.getName()); + Schema schema = table.schema(); + Types.NestedField current = IcebergNestedColumnEvolution.findTopLevelField( + schema, column.getName()); if (current == null) { throw new DorisConnectorException("Column " + column.getName() + " does not exist"); } + // Iceberg update paths are case-sensitive, so stage every change with the persisted spelling. + String currentName = current.name(); // Iceberg can widen required -> optional but never optional -> required (existing data may hold // nulls), so a NOT NULL request on an already-nullable column fails loud — legacy parity // (IcebergMetadataOps.validateForModifyColumn / validateForModifyComplexColumn). @@ -555,7 +559,7 @@ 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. @@ -563,16 +567,17 @@ public void modifyColumn(String dbName, String tableName, IcebergColumnChange co 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( + updateSchema, position, currentName, schema, "modify"); updateSchema.commit(); return null; }); diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergComplexTypeDiff.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergComplexTypeDiff.java index 36aa7cef28f0ea..ad8ab08a917ce2 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergComplexTypeDiff.java +++ b/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(lowercaseName(oldField.name())); - // Legacy ColumnType rule: existing fields are matched by position and may not be renamed. - if (!oldField.name().equals(newField.name())) { + // Iceberg defines case-insensitive identity with ROOT-lowercase keys. Java equalsIgnoreCase is + // broader for some Unicode characters and could otherwise route an update to the wrong field. + if (!lowercaseName(oldField.name()).equals(lowercaseName(newField.name()))) { throw new DorisConnectorException("Cannot rename struct field from '" + oldField.name() + "' to '" + newField.name() + "'"); } @@ -216,7 +217,7 @@ private static void applyStructChange(UpdateSchema updateSchema, String path, // Append the new fields (legacy parity: must be nullable and not clash with an existing name). for (int i = oldFields.size(); i < newFields.size(); i++) { Types.NestedField newField = newFields.get(i); - if (existingNames.contains(newField.name())) { + if (!existingNames.add(lowercaseName(newField.name()))) { throw new DorisConnectorException("Added struct field '" + newField.name() + "' conflicts with existing field"); } @@ -227,6 +228,10 @@ private static void applyStructChange(UpdateSchema updateSchema, String path, } } + private static String lowercaseName(String name) { + return name.toLowerCase(Locale.ROOT); + } + private static void applyListChange(UpdateSchema updateSchema, String path, Types.ListType oldList, Types.ListType newList, ConnectorType newConn) { String elementPath = path + "." + oldList.field(oldList.elementId()).name(); diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java index bbb838940267cb..10318ee9ee19c5 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java @@ -1303,7 +1303,7 @@ public void modifyColumn(ConnectorSession session, ConnectorTableHandle handle, // generic "Unsupported type for Iceberg: SMALLINT" here. Restore the legacy parity message ("Cannot // change int to smallint in nested types") by validating the requested nested type against the // CURRENT type — legacy validated in Doris type space, where the narrow target still exists. - throw upgradeNestedModifyError(iceHandle, column, buildError); + throw upgradeNestedModifyError(iceHandle, ConnectorColumnPath.of(column.getName()), column, buildError); } // Carry the neutral source type so a complex-type diff can read each STRUCT field's commentSpecified. IcebergColumnChange change = new IcebergColumnChange(column.getName(), icebergType, @@ -1328,15 +1328,15 @@ public void modifyColumn(ConnectorSession session, ConnectorTableHandle handle, * against the CURRENT column type. Best-effort: a scalar modify, a load failure, or no offending nested leaf * keeps the original build error — so no other modify path changes. */ - private DorisConnectorException upgradeNestedModifyError(IcebergTableHandle handle, ConnectorColumn column, - DorisConnectorException buildError) { + private DorisConnectorException upgradeNestedModifyError(IcebergTableHandle handle, ConnectorColumnPath path, + ConnectorColumn column, DorisConnectorException buildError) { if (!isComplexType(column.getType())) { return buildError; } try { Types.NestedField current = executeAuthenticated(() -> catalogOps.withTable(handle.getDbName(), handle.getTableName(), - table -> table.schema().findField(column.getName()))); + table -> IcebergNestedColumnEvolution.findFieldForErrorUpgrade(table.schema(), path))); if (current != null && !current.type().isPrimitiveType()) { IcebergComplexTypeDiff.validateNestedModifyRepresentable(current.type(), column.getType()); } @@ -1487,7 +1487,8 @@ public void modifyNestedColumn(ConnectorSession session, ConnectorTableHandle ha try { icebergType = IcebergSchemaBuilder.buildColumnType(column.getType()); } catch (DorisConnectorException buildError) { - throw upgradeNestedModifyError(iceHandle, column, buildError); + // Preserve the complete target identity so error parity cannot bind a same-named top-level field. + throw upgradeNestedModifyError(iceHandle, path, column, buildError); } // Carry the neutral source type so the nested complex-type diff can read each STRUCT field's // commentSpecified (an omitted COMMENT on a sub-field must keep its current doc, not clear it). diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergNestedColumnEvolution.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergNestedColumnEvolution.java index b4ca1c02c7df97..7bc5a0e52b8942 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergNestedColumnEvolution.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergNestedColumnEvolution.java @@ -492,6 +492,19 @@ private static ResolvedColumnPath resolveColumnPath(Schema schema, ConnectorColu return new ResolvedColumnPath(ConnectorColumnPath.of(canonicalParts), currentType, currentField); } + static NestedField findTopLevelField(Schema schema, String columnName) { + return schema.asStruct().caseInsensitiveField(columnName); + } + + static NestedField findFieldForErrorUpgrade(Schema schema, ConnectorColumnPath columnPath) { + try { + return resolveColumnPath(schema, columnPath, "modify").getField(); + } catch (DorisConnectorException ignored) { + // Error-message upgrading is best-effort and must not replace the original type-build failure. + return null; + } + } + /** * Resolves {@code columnPath}'s parent (which must be a struct) and its leaf within that struct * (case-insensitive). Used by nested DROP / RENAME, which target an existing struct field. @@ -584,6 +597,13 @@ private static void applyPosition(UpdateSchema updateSchema, ConnectorColumnPosi } } + static void applyTopLevelPosition(UpdateSchema updateSchema, ConnectorColumnPosition position, + String columnName, Schema schema, String operation) { + if (position != null) { + applyPosition(updateSchema, position, ConnectorColumnPath.of(columnName), schema, operation); + } + } + private static String getPositionReferencePath(Schema schema, ConnectorColumnPath columnPath, ConnectorColumnPosition position, String operation) { if (position == null || position.isFirst()) { diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java index c0b9089b6fdbf7..450ae232695798 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/CatalogBackedIcebergCatalogOpsColumnEvolutionTest.java @@ -367,6 +367,32 @@ private static ConnectorType structType(List names, List return ConnectorType.structOf(names, types, nullable, comments); } + @Test + public void testModifyColumnsCanonicalizeMixedCaseRootAndPosition() { + createMixedCaseTable(); + ops.modifyColumn("db1", "mixed", change("id", Types.LongType.get(), "identifier", true), true, null); + + ConnectorType requestedType = structType( + Arrays.asList("metric"), Arrays.asList(ConnectorType.of("BIGINT")), + Arrays.asList(true), Arrays.asList((String) null)); + + ops.modifyColumn("db1", "mixed", + new IcebergColumnChange("info", IcebergSchemaBuilder.buildColumnType(requestedType), + "updated", null, true, requestedType), + true, ConnectorColumnPosition.after("id")); + + Schema schema = reload("mixed"); + Assertions.assertEquals(Arrays.asList("Id", "Info", "Label"), schema.columns().stream() + .map(Types.NestedField::name).collect(Collectors.toList())); + Assertions.assertEquals(Type.TypeID.LONG, schema.findField("Id").type().typeId()); + Assertions.assertEquals("identifier", schema.findField("Id").doc()); + Types.NestedField info = schema.findField("Info"); + Assertions.assertEquals("updated", info.doc()); + Types.NestedField metric = info.type().asStructType().fields().get(0); + Assertions.assertEquals("Metric", metric.name()); + Assertions.assertEquals(Type.TypeID.LONG, metric.type().typeId()); + } + @Test public void testModifyStructAddsNullableField() { createTable("s_add", new ConnectorColumn("st", @@ -397,6 +423,124 @@ public void testModifyStructWidensFieldTypeAndComment() { Assertions.assertEquals("new", a.doc()); } + @Test + public void testModifyStructMatchesExistingFieldCaseInsensitively() { + createTable("s_case", new ConnectorColumn("st", + structType(Arrays.asList("CaseSensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(true), Arrays.asList((String) null)), "", true, null, false)); + + modifyComplex("s_case", "st", + structType(Arrays.asList("casesensitive"), Arrays.asList(ConnectorType.of("BIGINT")), + Arrays.asList(true), Arrays.asList((String) null)), true); + + Types.NestedField field = reload("s_case").findField("st").type().asStructType().fields().get(0); + Assertions.assertEquals("CaseSensitive", field.name()); + Assertions.assertEquals(Type.TypeID.LONG, field.type().typeId()); + } + + @Test + public void testModifyStructCommentMatchesExistingFieldCaseInsensitively() { + createTable("s_case_doc", new ConnectorColumn("st", + structType(Arrays.asList("CaseSensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(true), Arrays.asList("old")), "", true, null, false)); + + modifyComplex("s_case_doc", "st", + structType(Arrays.asList("casesensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(true), Arrays.asList("new")), true); + + Types.NestedField field = reload("s_case_doc").findField("st").type().asStructType().fields().get(0); + Assertions.assertEquals("CaseSensitive", field.name()); + Assertions.assertEquals("new", field.doc()); + } + + @Test + public void testModifyStructNullabilityMatchesExistingFieldCaseInsensitively() { + createTable("s_case_null", new ConnectorColumn("st", + structType(Arrays.asList("CaseSensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(false), Arrays.asList((String) null)), "", true, null, false)); + + modifyComplex("s_case_null", "st", + structType(Arrays.asList("casesensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(true), Arrays.asList((String) null)), true); + + Types.NestedField field = reload("s_case_null").findField("st").type().asStructType().fields().get(0); + Assertions.assertEquals("CaseSensitive", field.name()); + Assertions.assertTrue(field.isOptional()); + } + + @Test + public void testModifyStructUnderArrayMatchesExistingFieldCaseInsensitively() { + ConnectorType oldStruct = structType( + Arrays.asList("CaseSensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(true), Arrays.asList((String) null)); + createTable("a_case", new ConnectorColumn( + "arr", ConnectorType.arrayOf(oldStruct), "", true, null, false)); + ConnectorType newStruct = structType( + Arrays.asList("casesensitive"), Arrays.asList(ConnectorType.of("BIGINT")), + Arrays.asList(true), Arrays.asList((String) null)); + + modifyComplex("a_case", "arr", ConnectorType.arrayOf(newStruct), true); + + Types.NestedField field = reload("a_case").findField("arr").type().asListType() + .elementType().asStructType().fields().get(0); + Assertions.assertEquals("CaseSensitive", field.name()); + Assertions.assertEquals(Type.TypeID.LONG, field.type().typeId()); + } + + @Test + public void testModifyStructUnderMapMatchesExistingFieldCaseInsensitively() { + ConnectorType oldStruct = structType( + Arrays.asList("CaseSensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(true), Arrays.asList((String) null)); + createTable("m_case", new ConnectorColumn("m", + ConnectorType.mapOf(ConnectorType.of("STRING"), oldStruct), "", true, null, false)); + ConnectorType newStruct = structType( + Arrays.asList("casesensitive"), Arrays.asList(ConnectorType.of("BIGINT")), + Arrays.asList(true), Arrays.asList((String) null)); + + modifyComplex("m_case", "m", + ConnectorType.mapOf(ConnectorType.of("STRING"), newStruct), true); + + Types.NestedField field = reload("m_case").findField("m").type().asMapType() + .valueType().asStructType().fields().get(0); + Assertions.assertEquals("CaseSensitive", field.name()); + Assertions.assertEquals(Type.TypeID.LONG, field.type().typeId()); + } + + @Test + public void testModifyStructRejectsCaseInsensitiveAppendedFieldCollision() { + createTable("s_case_collision", new ConnectorColumn("st", + structType(Arrays.asList("CaseSensitive"), Arrays.asList(ConnectorType.of("INT")), + Arrays.asList(true), Arrays.asList((String) null)), "", true, null, false)); + + DorisConnectorException ex = Assertions.assertThrows(DorisConnectorException.class, + () -> modifyComplex("s_case_collision", "st", + structType(Arrays.asList("casesensitive", "CASESENSITIVE"), + Arrays.asList(ConnectorType.of("INT"), ConnectorType.of("STRING")), + Arrays.asList(true, true), Arrays.asList(null, null)), true)); + + Assertions.assertTrue(ex.getMessage().contains("conflicts with existing field"), ex.getMessage()); + } + + @Test + public void testModifyStructUsesIcebergLowercaseIdentity() { + createTable("s_unicode_identity", new ConnectorColumn("st", + structType(Arrays.asList("Σ", "ς"), + Arrays.asList(ConnectorType.of("INT"), ConnectorType.of("INT")), + Arrays.asList(true, true), Arrays.asList(null, null)), "", true, null, false)); + + DorisConnectorException ex = Assertions.assertThrows(DorisConnectorException.class, + () -> modifyComplex("s_unicode_identity", "st", + structType(Arrays.asList("ς", "Σ"), + Arrays.asList(ConnectorType.of("BIGINT"), ConnectorType.of("INT")), + Arrays.asList(true, true), Arrays.asList(null, null)), true)); + + Assertions.assertTrue(ex.getMessage().contains("Cannot rename struct field"), ex.getMessage()); + Types.StructType fields = reload("s_unicode_identity").findField("st").type().asStructType(); + Assertions.assertEquals(Type.TypeID.INTEGER, fields.field("Σ").type().typeId()); + Assertions.assertEquals(Type.TypeID.INTEGER, fields.field("ς").type().typeId()); + } + @Test public void testModifyStructFieldWidensNotNullToNullable() { createTable("s_null", new ConnectorColumn("st", diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataColumnEvolutionTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataColumnEvolutionTest.java index da1a8e3557b214..c82513e421c1bb 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataColumnEvolutionTest.java +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataColumnEvolutionTest.java @@ -20,6 +20,7 @@ import org.apache.doris.connector.spi.ConnectorColumn; import org.apache.doris.connector.spi.ConnectorType; import org.apache.doris.connector.spi.DorisConnectorException; +import org.apache.doris.connector.spi.ddl.ConnectorColumnPath; import org.apache.doris.connector.spi.ddl.ConnectorColumnPosition; import org.apache.iceberg.Schema; @@ -84,6 +85,21 @@ private static Table tableWithStructIntColumn() { return catalog.createTable(TableIdentifier.of("db1", "t1"), schema); } + /** A real iceberg table with a nested {@code Root.Leaf ARRAY} target and an unrelated + * top-level {@code LEAF ARRAY} field sharing the target's leaf name. */ + private static Table tableWithNestedArrayAndTopLevelDecoy() { + InMemoryCatalog catalog = new InMemoryCatalog(); + catalog.initialize("test", Collections.emptyMap()); + catalog.createNamespace(Namespace.of("db1")); + Schema schema = new Schema( + Types.NestedField.optional(1, "Root", Types.StructType.of( + Types.NestedField.optional(2, "Leaf", + Types.ListType.ofOptional(3, Types.IntegerType.get())))), + Types.NestedField.optional(4, "LEAF", + Types.ListType.ofOptional(5, Types.FloatType.get()))); + return catalog.createTable(TableIdentifier.of("db1", "t1"), schema); + } + // ---------- addColumn ---------- @Test @@ -290,6 +306,37 @@ public void testModifyStructFieldNarrowToUnrepresentableRestoresLegacyMessage() Assertions.assertEquals("Cannot change int to smallint in nested types", ex.getMessage()); } + @Test + public void testModifyNestedColumnBuildErrorUsesFullTargetPath() { + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = tableWithNestedArrayAndTopLevelDecoy(); + RecordingConnectorContext ctx = new RecordingConnectorContext(); + ConnectorColumn leaf = new ConnectorColumn("leaf", + ConnectorType.arrayOf(ConnectorType.of("SMALLINT")), "", true, null, false); + + DorisConnectorException ex = Assertions.assertThrows(DorisConnectorException.class, + () -> metadata(ops, ctx).modifyNestedColumn(null, HANDLE, + ConnectorColumnPath.of(Arrays.asList("root", "leaf")), leaf, null)); + + // Error parity must use the resolved nested target, never a same-named top-level field. + Assertions.assertEquals("Cannot change int to smallint in nested types", ex.getMessage()); + } + + @Test + public void testModifyMissingComplexColumnKeepsBuildError() { + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = tableWithArrayIntColumn(); + RecordingConnectorContext ctx = new RecordingConnectorContext(); + ConnectorColumn missing = new ConnectorColumn("missing", + ConnectorType.arrayOf(ConnectorType.of("SMALLINT")), "", true, null, false); + + DorisConnectorException ex = Assertions.assertThrows(DorisConnectorException.class, + () -> metadata(ops, ctx).modifyColumn(null, HANDLE, missing, null)); + + // Error upgrading is best-effort; an unresolved target must retain the original build failure. + Assertions.assertEquals("Unsupported type for Iceberg: SMALLINT", ex.getMessage()); + } + @Test public void testModifyScalarColumnToUnrepresentableKeepsBuildError() { // A TOP-LEVEL (non-nested) modify to an iceberg-unrepresentable type keeps the generic build error: the diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverter.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverter.java index 8a2d4dd38c6a2e..24986554c6452c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverter.java @@ -238,7 +238,7 @@ public static ConnectorType toConnectorType(Type dorisType) { // isCommentSpecified() so the diff can tell an omitted COMMENT (preserve the current doc) from // COMMENT '' (clear it) — the comment string is "" for both (#65329 omit-preserves-metadata). for (StructField f : struct.getFields()) { - names.add(f.getName()); + names.add(f.getOriginalName()); types.add(toConnectorType(f.getType())); nullables.add(f.getContainsNull()); comments.add(f.getComment()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java index a8f9d7bb27bf42..d84348c1513163 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzer.java @@ -272,7 +272,9 @@ public Expression visitDereferenceExpression(DereferenceExpression dereferenceEx StructType structType = (StructType) dataType; StructField field = structType.getField(dereferenceExpression.fieldName); if (field != null) { - return new ElementAt(expression, dereferenceExpression.child(1)); + // This newly constructed node returns directly and will not be revisited by visitElementAt. + return canonicalizeStructSelector( + new ElementAt(expression, dereferenceExpression.child(1))); } } else if (dataType.isMapType()) { return new ElementAt(expression, dereferenceExpression.child(1)); @@ -298,6 +300,7 @@ public Expression visitElementAt(ElementAt elementAt, ExpressionRewriteContext c } Expression right = elementAt.right().accept(this, context); elementAt = (ElementAt) elementAt.withChildren(left, right); + elementAt = canonicalizeStructSelector(elementAt); Expression coerced = TypeCoercionUtils.processBoundFunction(elementAt); if (isEnableVariantSchemaAutoCast(context)) { return wrapVariantElementAtWithCast(coerced); @@ -608,7 +611,12 @@ public Expression visitUnboundFunction(UnboundFunction unboundFunction, Expressi // we do type coercion in build function in alias function, so it's ok to return directly. return buildResult.first; } else { - Expression castFunction = TypeCoercionUtils.processBoundFunction((BoundFunction) buildResult.first); + BoundFunction boundFunction = (BoundFunction) buildResult.first; + if (boundFunction instanceof ElementAt) { + // SQL function syntax binds here directly and therefore does not visit visitElementAt above. + boundFunction = canonicalizeStructSelector((ElementAt) boundFunction); + } + Expression castFunction = TypeCoercionUtils.processBoundFunction(boundFunction); if (castFunction instanceof RewriteWhenAnalyze) { castFunction = ((RewriteWhenAnalyze) castFunction).rewriteWhenAnalyze(); } @@ -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) { + String selector = ((StringLikeLiteral) right).getStringValue(); + StructField field = ((StructType) left.getDataType()).getField(selector); + if (field != null && !field.getName().equals(selector)) { + // BE struct names use the normalized thrift identity and cannot Unicode-fold external spelling. + return (ElementAt) elementAt.withChildren(left, new StringLiteral(field.getName())); + } + } + return elementAt; + } + @Override public Expression visitWindow(WindowExpression windowExpression, ExpressionRewriteContext context) { windowExpression = (WindowExpression) super.visitWindow(windowExpression, context); @@ -1277,7 +1299,8 @@ private Optional bindNestedFields(UnboundSlot unboundSlot, Slot slot throw new AnalysisException("No such struct field '" + fieldName + "' in '" + lastFieldName + "'"); } lastFieldName = fieldName; - expression = new ElementAt(expression, new StringLiteral(fieldName)); + // Dereference-created selectors also cross the thrift boundary and must use runtime identity. + expression = new ElementAt(expression, new StringLiteral(field.getName())); continue; } else if (dataType.isMapType()) { expression = new ElementAt(expression, new StringLiteral(fieldName)); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java index fdbc88de615072..a6e6a8bfa7ed0a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollector.java @@ -73,6 +73,7 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; @@ -147,7 +148,7 @@ public Void visitSlotReference(SlotReference slotReference, CollectorContext con return null; } if (dataType instanceof NestedColumnPrunable) { - context.accessPathBuilder.addPrefix(slotReference.getName().toLowerCase()); + context.accessPathBuilder.addPrefix(slotReference.getName().toLowerCase(Locale.ROOT)); ImmutableList path = Utils.fastToImmutableList(context.accessPathBuilder.accessPath); int slotId = slotReference.getExprId().asInt(); slotToAccessPaths.put(slotId, new CollectAccessPathResult(path, context.bottomFilter, context.type)); @@ -358,7 +359,8 @@ public Void visitElementAt(ElementAt elementAt, CollectorContext context) { return continueCollectAccessPath(first, context); } } - context.accessPathBuilder.addPrefix(((Literal) fieldName).getStringValue().toLowerCase()); + context.accessPathBuilder.addPrefix( + ((Literal) fieldName).getStringValue().toLowerCase(Locale.ROOT)); return continueCollectAccessPath(first, context); } return visit(elementAt, context); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java index 3faf0d581a5c72..64153355fc34a2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/NestedColumnPruning.java @@ -56,6 +56,7 @@ import java.util.Comparator; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; @@ -620,7 +621,8 @@ public boolean replacePathByAnotherTree(DataTypeAccessTree cast, List pa List fields = ((StructType) cast.type).getFields(); for (int i = 0; i < fields.size(); i++) { String castFieldName = path.get(index); - if (fields.get(i).getName().equalsIgnoreCase(castFieldName)) { + // Struct runtime keys are ROOT-normalized; broad folding can merge distinct siblings. + if (fields.get(i).getName().equals(castFieldName)) { String originFieldName = ((StructType) type).getFields().get(i).getName(); path.set(index, originFieldName); return children.get(originFieldName).replacePathByAnotherTree( @@ -662,7 +664,7 @@ public void setAccessByPath(List path, int accessIndex, ColumnAccessPath accessPartialChild = true; if (this.type.isStructType()) { - String fieldName = path.get(accessIndex).toLowerCase(); + String fieldName = path.get(accessIndex).toLowerCase(Locale.ROOT); DataTypeAccessTree child = children.get(fieldName); if (child != null) { child.setAccessByPath(path, accessIndex + 1, pathType); @@ -730,7 +732,8 @@ public void setAccessByPath(List path, int accessIndex, ColumnAccessPath accessAll = true; return; } else if (isRoot) { - children.get(path.get(accessIndex).toLowerCase()).setAccessByPath(path, accessIndex + 1, pathType); + children.get(path.get(accessIndex).toLowerCase(Locale.ROOT)) + .setAccessByPath(path, accessIndex + 1, pathType); return; } throw new AnalysisException("unsupported data type: " + this.type); @@ -739,7 +742,7 @@ public void setAccessByPath(List path, int accessIndex, ColumnAccessPath public static DataTypeAccessTree ofRoot(Slot slot, ColumnAccessPathType pathType) { DataTypeAccessTree child = of(slot.getDataType(), pathType); DataTypeAccessTree root = new DataTypeAccessTree(true, NullType.INSTANCE, pathType); - root.children.put(slot.getName().toLowerCase(), child); + root.children.put(slot.getName().toLowerCase(Locale.ROOT), child); return root; } @@ -749,7 +752,8 @@ public static DataTypeAccessTree of(DataType type, ColumnAccessPathType pathType if (type instanceof StructType) { StructType structType = (StructType) type; for (Entry kv : structType.getNameToFields().entrySet()) { - root.children.put(kv.getKey().toLowerCase(), of(kv.getValue().getDataType(), pathType)); + root.children.put(kv.getKey().toLowerCase(Locale.ROOT), + of(kv.getValue().getDataType(), pathType)); } } else if (type instanceof ArrayType) { root.children.put(AccessPathInfo.ACCESS_ALL, of(((ArrayType) type).getItemType(), pathType)); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CreateNamedStruct.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CreateNamedStruct.java index c27b5cc94c9a54..49d089bd4a696b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CreateNamedStruct.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/CreateNamedStruct.java @@ -35,6 +35,7 @@ import com.google.common.collect.Sets; import java.util.List; +import java.util.Locale; import java.util.Set; /** @@ -70,7 +71,7 @@ public void checkLegalityBeforeTypeCoercion() { throw new AnalysisException("named_struct only allows" + " constant string parameter in odd position: " + this); } else { - String name = ((StringLikeLiteral) child(i)).getStringValue().toLowerCase(); + String name = ((StringLikeLiteral) child(i)).getStringValue().toLowerCase(Locale.ROOT); if (names.contains(name)) { throw new AnalysisException("The name of the struct field cannot be repeated." + " same name fields are " + name); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java index d70079555c7e0c..6c62dbc2bd0108 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java @@ -467,9 +467,9 @@ public static DataType fromCatalogType(Type type) { if (type.isStructType()) { List structFields = ((org.apache.doris.catalog.StructType) (type)).getFields().stream() - .map(cf -> new StructField(cf.getName(), fromCatalogType(cf.getType()), + .map(cf -> new StructField(cf.getName(), cf.getOriginalName(), fromCatalogType(cf.getType()), cf.getContainsNull(), cf.getComment() == null ? "" : cf.getComment(), - cf.isCommentSpecified())) + cf.isCommentSpecified(), !cf.hasOriginalName())) .collect(ImmutableList.toImmutableList()); return new StructType(structFields); } else if (type.isMapType()) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructField.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructField.java index aefcf20f227ff1..7e8699293ae809 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructField.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructField.java @@ -22,6 +22,7 @@ import org.apache.doris.nereids.util.SqlLiteralUtils; import org.apache.doris.nereids.util.Utils; +import java.util.Locale; import java.util.Objects; /** @@ -32,10 +33,12 @@ public class StructField { public static final String DEFAULT_FIELD_NAME = "col"; private final String name; + private final String originalName; private final DataType dataType; private final boolean nullable; private final String comment; private final boolean commentSpecified; + private final boolean legacyLocaleDependentName; /** * StructField Constructor @@ -49,17 +52,44 @@ public StructField(String name, DataType dataType, boolean nullable, String comm public StructField(String name, DataType dataType, boolean nullable, String comment, boolean commentSpecified) { - this.name = Objects.requireNonNull(name, "name should not be null").toLowerCase(); + this(name, name, dataType, nullable, comment, 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 dataType field data type + * @param nullable whether the field accepts null values + * @param comment field comment + * @param commentSpecified whether the comment was explicitly specified + */ + public StructField(String name, String originalName, DataType dataType, boolean nullable, String comment, + boolean commentSpecified) { + this(name, originalName, dataType, nullable, comment, commentSpecified, false); + } + + StructField(String name, String originalName, DataType dataType, boolean nullable, String comment, + boolean commentSpecified, boolean legacyLocaleDependentName) { + // Runtime field identity must stay stable across FE locales and match external schema lookup keys. + this.name = Objects.requireNonNull(name, "name should not be null").toLowerCase(Locale.ROOT); + this.originalName = Objects.requireNonNull(originalName, "originalName should not be null"); this.dataType = Objects.requireNonNull(dataType, "dataType should not be null"); this.nullable = nullable; this.comment = Objects.requireNonNull(comment, "comment should not be null"); this.commentSpecified = commentSpecified; + this.legacyLocaleDependentName = legacyLocaleDependentName; } public String getName() { return name; } + public String getOriginalName() { + return originalName; + } + public DataType getDataType() { return dataType; } @@ -76,6 +106,10 @@ public boolean isCommentSpecified() { return commentSpecified; } + boolean isLegacyLocaleDependentName() { + return legacyLocaleDependentName; + } + public StructField conversion() { if (this.dataType.equals(dataType.conversion())) { return this; @@ -84,16 +118,19 @@ public StructField conversion() { } public StructField withDataType(DataType dataType) { - return new StructField(name, dataType, nullable, comment, commentSpecified); + return new StructField(name, originalName, dataType, nullable, comment, commentSpecified, + legacyLocaleDependentName); } public StructField withDataTypeAndNullable(DataType dataType, boolean nullable) { - return new StructField(name, dataType, nullable, comment, commentSpecified); + return new StructField(name, originalName, dataType, nullable, comment, commentSpecified, + legacyLocaleDependentName); } public org.apache.doris.catalog.StructField toCatalogDataType() { return new org.apache.doris.catalog.StructField( - name, dataType.toCatalogDataType(), comment, nullable, commentSpecified); + name, legacyLocaleDependentName ? null : originalName, + dataType.toCatalogDataType(), comment, nullable, commentSpecified); } public String toSql() { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructType.java index 13f28c2e06e986..8f2893ec55f87d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/StructType.java @@ -29,6 +29,7 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -60,7 +61,7 @@ public StructType(List fields) { // ATTN: should use LinkedHashMap to keep order this.nameToFields = new LinkedHashMap<>(); for (StructField field : this.fields) { - String fieldName = field.getName().toLowerCase(); + String fieldName = field.getName().toLowerCase(Locale.ROOT); StructField existingField = this.nameToFields.put(fieldName, field); if (existingField != null) { throw new AnalysisException("Duplicate field name found: " + fieldName); @@ -76,8 +77,25 @@ public Map getNameToFields() { return nameToFields; } + /** Get a field by its case-insensitive runtime name. */ public StructField getField(String name) { - return nameToFields.get(name.toLowerCase()); + StructField field = nameToFields.get(name.toLowerCase(Locale.ROOT)); + if (field != null && (!field.isLegacyLocaleDependentName() || field.getName().equals(name))) { + return field; + } + StructField legacyMatch = null; + for (int i = fields.size() - 1; i >= 0; i--) { + StructField legacyField = fields.get(i); + // Limit broad case folding to old replayed fields so new ROOT-distinct names remain distinct. + if (legacyField.isLegacyLocaleDependentName() && legacyField.getName().equalsIgnoreCase(name)) { + if (legacyMatch != null) { + // The old locale was not persisted, so choosing either folded sibling could return wrong data. + return null; + } + legacyMatch = legacyField; + } + } + return legacyMatch != null ? legacyMatch : field; } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunction.java b/fe/fe-core/src/main/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunction.java index 01abd8b482123e..eb21be855a8cab 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunction.java @@ -98,6 +98,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -421,13 +422,16 @@ private Pair getColumnType(List typeNodes, int start) for (int i = 0; i < typeNodes.get(start).getStructFieldsCount(); ++i) { Pair fieldType = getColumnType(typeNodes, start + parsedNodes); PStructField structField = typeNodes.get(start).getStructFields(i); - String fieldName = structField.getName().toLowerCase(); + String originalFieldName = structField.getName(); + String fieldName = originalFieldName.toLowerCase(Locale.ROOT); if (fieldLowerNames.contains(fieldName)) { throw new NotSupportedException("Repeated lowercase field names: " + fieldName); } else { fieldLowerNames.add(fieldName); - fields.add(new StructField(fieldName, fieldType.key(), structField.getComment(), - structField.getContainsNull())); + // File readers return the external schema spelling, which must survive CTAS metadata writes; + // only the runtime lookup key and duplicate detection are normalized. + fields.add(new StructField(fieldName, originalFieldName, fieldType.key(), structField.getComment(), + structField.getContainsNull(), !structField.getComment().isEmpty())); } parsedNodes += fieldType.value(); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnGsonSerializationTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnGsonSerializationTest.java index 6e81121ca04366..dddc1a7ecf1e0f 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnGsonSerializationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/ColumnGsonSerializationTest.java @@ -23,6 +23,10 @@ import org.apache.doris.persist.gson.GsonUtils; import com.google.common.collect.Lists; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import com.google.gson.annotations.SerializedName; import org.junit.After; import org.junit.Assert; @@ -121,4 +125,72 @@ public void testSerializeColumnList() throws IOException, AnalysisException { in.close(); } + @Test + public void testReplayPreRootTurkishStructFieldName() { + StructType structType = new StructType(new StructField("I", Type.INT)); + JsonObject legacyJson = JsonParser.parseString( + GsonUtils.GSON.toJson(structType, Type.class)).getAsJsonObject(); + + JsonObject fieldMap = legacyJson.getAsJsonObject("fieldMap"); + JsonElement legacyMapField = fieldMap.remove("i"); + setLegacyTurkishFieldName(legacyMapField.getAsJsonObject()); + fieldMap.add("ı", legacyMapField); + JsonArray fields = legacyJson.getAsJsonArray("fields"); + setLegacyTurkishFieldName(fields.get(0).getAsJsonObject()); + + StructType replayed = (StructType) GsonUtils.GSON.fromJson(legacyJson, Type.class); + Assert.assertEquals("ı", replayed.getField("I").getName()); + + org.apache.doris.nereids.types.StructType nereidsType = + (org.apache.doris.nereids.types.StructType) + org.apache.doris.nereids.types.DataType.fromCatalogType(replayed); + Assert.assertEquals("ı", nereidsType.getField("I").getName()); + + StructType roundTrip = (StructType) nereidsType.toCatalogDataType(); + Assert.assertEquals("ı", roundTrip.getField("I").getName()); + } + + @Test + public void testCurrentDotlessStructFieldDoesNotMatchAsciiI() { + StructType structType = new StructType(new StructField("ı", Type.INT)); + Assert.assertNull(structType.getField("I")); + + org.apache.doris.nereids.types.StructType nereidsType = + (org.apache.doris.nereids.types.StructType) + org.apache.doris.nereids.types.DataType.fromCatalogType(structType); + Assert.assertNull(nereidsType.getField("I")); + } + + @Test + public void testReplayPreRootTurkishStructFieldCollisionIsAmbiguous() { + StructType structType = new StructType( + new StructField("ı", Type.INT), new StructField("i", Type.BIGINT)); + JsonObject legacyJson = JsonParser.parseString( + GsonUtils.GSON.toJson(structType, Type.class)).getAsJsonObject(); + legacyJson.getAsJsonObject("fieldMap").entrySet().forEach( + entry -> entry.getValue().getAsJsonObject().remove("originalName")); + legacyJson.getAsJsonArray("fields").forEach( + field -> field.getAsJsonObject().remove("originalName")); + + StructType replayed = (StructType) GsonUtils.GSON.fromJson(legacyJson, Type.class); + Assert.assertNull(replayed.getField("I")); + Assert.assertEquals("ı", replayed.getField("ı").getName()); + Assert.assertEquals("i", replayed.getField("i").getName()); + + org.apache.doris.nereids.types.StructType nereidsType = + (org.apache.doris.nereids.types.StructType) + org.apache.doris.nereids.types.DataType.fromCatalogType(replayed); + Assert.assertNull(nereidsType.getField("I")); + Assert.assertEquals("ı", nereidsType.getField("ı").getName()); + Assert.assertEquals("i", nereidsType.getField("i").getName()); + + StructType roundTrip = (StructType) nereidsType.toCatalogDataType(); + Assert.assertNull(roundTrip.getField("I")); + } + + private static void setLegacyTurkishFieldName(JsonObject field) { + field.addProperty("name", "ı"); + field.remove("originalName"); + } + } diff --git a/fe/fe-core/src/test/java/org/apache/doris/connector/ddl/CreateTableInfoToConnectorRequestConverterTest.java b/fe/fe-core/src/test/java/org/apache/doris/connector/ddl/CreateTableInfoToConnectorRequestConverterTest.java index 6abe426c049475..471acd6861d19c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/connector/ddl/CreateTableInfoToConnectorRequestConverterTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/connector/ddl/CreateTableInfoToConnectorRequestConverterTest.java @@ -37,6 +37,8 @@ import org.apache.doris.nereids.trees.plans.commands.info.SortFieldInfo; import org.apache.doris.nereids.types.IntegerType; import org.apache.doris.nereids.types.StringType; +import org.apache.doris.nereids.types.StructField; +import org.apache.doris.nereids.types.StructType; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -98,6 +100,20 @@ public void columnsAndScalarFieldsArePassedThrough() { Assertions.assertNull(req.getBucketSpec()); } + @Test + public void nestedFieldSpellingIsPreservedForConnectorSchemas() { + StructType payloadType = new StructType(ImmutableList.of( + new StructField("CaseSensitive", IntegerType.INSTANCE, true, ""))); + ColumnDefinition payload = new ColumnDefinition("payload", payloadType, true); + CreateTableInfo info = stubInfo("t", Collections.singletonList(payload), + null, null, "", Collections.emptyMap(), false); + + ConnectorCreateTableRequest request = CreateTableInfoToConnectorRequestConverter.convert(info, "db"); + + Assertions.assertEquals(Collections.singletonList("CaseSensitive"), + request.getColumns().get(0).getType().getFieldNames()); + } + @Test public void autoIncInitValueIsPropagatedAsIsAutoInc() { // ColumnDefinition is mocked (its auto-inc ctor pulls in ColumnNullableType machinery); diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverterTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverterTest.java index 71bb443e1866bc..85a5c65f3e5ef5 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverterTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/connector/converter/ConnectorColumnConverterTest.java @@ -35,6 +35,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Locale; class ConnectorColumnConverterTest { @@ -117,6 +118,48 @@ void testStructTypeRoundtrip() { Assertions.assertEquals(ScalarType.INT, backStruct.getFields().get(0).getType()); } + @Test + void mixedCaseStructFieldKeepsSchemaSpellingAndNormalizedRuntimeName() { + ConnectorType connectorType = ConnectorType.structOf( + Arrays.asList("CaseSensitive"), Arrays.asList(ConnectorType.of("INT"))); + + StructType converted = (StructType) ConnectorColumnConverter.convertType(connectorType); + StructField field = converted.getFields().get(0); + + Assertions.assertEquals("casesensitive", field.getName()); + Assertions.assertEquals("CaseSensitive", field.getOriginalName()); + Assertions.assertEquals("struct", converted.toSql()); + Assertions.assertEquals("casesensitive", + converted.toThrift().getTypes().get(0).getStructFields().get(0).getName()); + } + + @Test + void structRuntimeNamesUseRootLocale() { + Locale originalLocale = Locale.getDefault(); + try { + Locale.setDefault(Locale.forLanguageTag("tr-TR")); + ConnectorType connectorType = ConnectorType.structOf( + Arrays.asList("I", "ı"), + Arrays.asList(ConnectorType.of("INT"), ConnectorType.of("STRING"))); + + StructType catalogType = (StructType) ConnectorColumnConverter.convertType(connectorType); + Assertions.assertEquals("i", catalogType.getFields().get(0).getName()); + Assertions.assertEquals("ı", catalogType.getFields().get(1).getName()); + Assertions.assertSame(catalogType.getFields().get(0), catalogType.getField("i")); + Assertions.assertSame(catalogType.getFields().get(1), catalogType.getField("ı")); + + org.apache.doris.nereids.types.StructType nereidsType = + (org.apache.doris.nereids.types.StructType) + org.apache.doris.nereids.types.DataType.fromCatalogType(catalogType); + Assertions.assertEquals("i", nereidsType.getFields().get(0).getName()); + Assertions.assertEquals("ı", nereidsType.getFields().get(1).getName()); + Assertions.assertSame(nereidsType.getFields().get(0), nereidsType.getField("i")); + Assertions.assertSame(nereidsType.getFields().get(1), nereidsType.getField("ı")); + } finally { + Locale.setDefault(originalLocale); + } + } + @Test void testNestedComplexType() { // ARRAY> diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzerTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzerTest.java index 6c70a37aa0c572..4f15f00ab56448 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzerTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/ExpressionAnalyzerTest.java @@ -17,13 +17,16 @@ package org.apache.doris.nereids.rules.analysis; +import org.apache.doris.nereids.CascadesContext; import org.apache.doris.nereids.analyzer.Scope; import org.apache.doris.nereids.analyzer.UnboundFunction; import org.apache.doris.nereids.analyzer.UnboundSlot; import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.And; import org.apache.doris.nereids.trees.expressions.BoundStar; import org.apache.doris.nereids.trees.expressions.Cast; +import org.apache.doris.nereids.trees.expressions.DereferenceExpression; import org.apache.doris.nereids.trees.expressions.ExprId; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.IsFalse; @@ -31,11 +34,17 @@ import org.apache.doris.nereids.trees.expressions.IsTrue; import org.apache.doris.nereids.trees.expressions.Not; import org.apache.doris.nereids.trees.expressions.SlotReference; +import org.apache.doris.nereids.trees.expressions.functions.scalar.ElementAt; import org.apache.doris.nereids.trees.expressions.literal.DateTimeV2Literal; +import org.apache.doris.nereids.trees.expressions.literal.StringLikeLiteral; import org.apache.doris.nereids.trees.expressions.literal.StringLiteral; import org.apache.doris.nereids.trees.expressions.literal.TinyIntLiteral; import org.apache.doris.nereids.types.BigIntType; import org.apache.doris.nereids.types.BooleanType; +import org.apache.doris.nereids.types.IntegerType; +import org.apache.doris.nereids.types.StructField; +import org.apache.doris.nereids.types.StructType; +import org.apache.doris.qe.ConnectContext; import com.google.common.collect.ImmutableList; import org.junit.jupiter.api.Assertions; @@ -133,4 +142,96 @@ public void testAnalyzeIsTrueAndIsFalse() { Assertions.assertInstanceOf(Not.class, isNotFalse); Assertions.assertInstanceOf(And.class, isNotFalse.child(0)); } + + @Test + public void testStructElementAtCanonicalizesUnicodeSelector() { + StructType structType = new StructType(ImmutableList.of( + new StructField("Σ", "Σ", IntegerType.INSTANCE, true, "", false), + new StructField("ẞ", "ẞ", IntegerType.INSTANCE, true, "", false))); + SlotReference payload = new SlotReference( + new ExprId(1), "payload", structType, true, ImmutableList.of()); + ExpressionAnalyzer analyzer = new ExpressionAnalyzer(null, new Scope(ImmutableList.of()), + null, true, true); + + Expression analyzedSigma = analyzer.analyze(new ElementAt(payload, new StringLiteral("Σ"))); + Expression analyzedSharpS = analyzer.analyze(new ElementAt(payload, new StringLiteral("ẞ"))); + + Assertions.assertInstanceOf(ElementAt.class, analyzedSigma); + Assertions.assertInstanceOf(ElementAt.class, analyzedSharpS); + // The BE receives the ROOT-normalized thrift name and cannot Unicode-fold the displayed spelling. + Assertions.assertEquals("σ", ((StringLikeLiteral) analyzedSigma.child(1)).getStringValue()); + Assertions.assertEquals("ß", ((StringLikeLiteral) analyzedSharpS.child(1)).getStringValue()); + } + + @Test + public void testStructElementAtFunctionCanonicalizesUnicodeSelector() { + StructType structType = new StructType(ImmutableList.of( + new StructField("Σ", "Σ", IntegerType.INSTANCE, true, "", false))); + SlotReference payload = new SlotReference( + new ExprId(1), "payload", structType, true, ImmutableList.of()); + ExpressionAnalyzer analyzer = new ExpressionAnalyzer(null, new Scope(ImmutableList.of()), + null, true, true); + + Expression analyzed = analyzer.analyze(new UnboundFunction("element_at", + ImmutableList.of(payload, new StringLiteral("Σ")))); + + Assertions.assertInstanceOf(ElementAt.class, analyzed); + Assertions.assertEquals("σ", ((StringLikeLiteral) analyzed.child(1)).getStringValue()); + } + + @Test + public void testStructDereferenceCanonicalizesUnicodeSelector() { + StructType structType = new StructType(ImmutableList.of( + new StructField("Σ", "Σ", IntegerType.INSTANCE, true, "", false))); + SlotReference payload = new SlotReference( + new ExprId(1), "payload", structType, true, ImmutableList.of()); + ConnectContext connectContext = new ConnectContext(); + connectContext.setThreadLocalInfo(); + try { + CascadesContext cascadesContext = CascadesContext.initTempContext(); + ExpressionAnalyzer analyzer = new ExpressionAnalyzer(null, new Scope(ImmutableList.of(payload)), + cascadesContext, true, true); + + Expression analyzed = analyzer.analyze(new UnboundSlot("payload", "Σ")); + + Assertions.assertInstanceOf(Alias.class, analyzed); + Assertions.assertInstanceOf(ElementAt.class, analyzed.child(0)); + Assertions.assertEquals("σ", ((StringLikeLiteral) analyzed.child(0).child(1)).getStringValue()); + } finally { + ConnectContext.remove(); + } + } + + @Test + public void testComputedStructDereferenceCanonicalizesUnicodeSelector() { + StructType structType = new StructType(ImmutableList.of( + new StructField("Σ", "Σ", IntegerType.INSTANCE, true, "", false))); + SlotReference payload = new SlotReference( + new ExprId(1), "payload", structType, true, ImmutableList.of()); + ExpressionAnalyzer analyzer = new ExpressionAnalyzer(null, new Scope(ImmutableList.of()), + null, true, true); + + Expression analyzed = analyzer.analyze(new DereferenceExpression( + new Cast(payload, structType), new StringLiteral("Σ"))); + + Assertions.assertInstanceOf(ElementAt.class, analyzed); + Assertions.assertEquals("σ", ((StringLikeLiteral) analyzed.child(1)).getStringValue()); + } + + @Test + public void testLegacyStructFieldCollisionRejectsAmbiguousSelector() { + org.apache.doris.catalog.StructType catalogType = new org.apache.doris.catalog.StructType( + new org.apache.doris.catalog.StructField( + "ı", null, org.apache.doris.catalog.Type.INT, "", true, false), + new org.apache.doris.catalog.StructField( + "i", null, org.apache.doris.catalog.Type.BIGINT, "", true, false)); + StructType structType = (StructType) org.apache.doris.nereids.types.DataType.fromCatalogType(catalogType); + SlotReference payload = new SlotReference( + new ExprId(1), "payload", structType, true, ImmutableList.of()); + ExpressionAnalyzer analyzer = new ExpressionAnalyzer(null, new Scope(ImmutableList.of()), + null, true, true); + + Assertions.assertThrows(AnalysisException.class, + () -> analyzer.analyze(new ElementAt(payload, new StringLiteral("I")))); + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollectorTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollectorTest.java new file mode 100644 index 00000000000000..d06e6905109d19 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/AccessPathExpressionCollectorTest.java @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.nereids.rules.rewrite; + +import org.apache.doris.analysis.ColumnAccessPathType; +import org.apache.doris.nereids.rules.rewrite.AccessPathExpressionCollector.CollectAccessPathResult; +import org.apache.doris.nereids.rules.rewrite.NestedColumnPruning.DataTypeAccessTree; +import org.apache.doris.nereids.trees.expressions.Cast; +import org.apache.doris.nereids.trees.expressions.SlotReference; +import org.apache.doris.nereids.trees.expressions.functions.scalar.ElementAt; +import org.apache.doris.nereids.trees.expressions.literal.StringLiteral; +import org.apache.doris.nereids.types.IntegerType; +import org.apache.doris.nereids.types.StringType; +import org.apache.doris.nereids.types.StructField; +import org.apache.doris.nereids.types.StructType; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Multimap; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + +public class AccessPathExpressionCollectorTest { + + @Test + public void testStructAccessPathUsesRootLocale() { + Locale originalLocale = Locale.getDefault(); + try { + Locale.setDefault(Locale.forLanguageTag("tr-TR")); + StructType structType = new StructType(ImmutableList.of( + new StructField("I", IntegerType.INSTANCE, true, ""), + new StructField("ı", StringType.INSTANCE, true, ""))); + SlotReference slot = new SlotReference("I", structType); + Multimap accessPaths = ArrayListMultimap.create(); + AccessPathExpressionCollector collector = + new AccessPathExpressionCollector(null, accessPaths, false, false); + + collector.collect(new ElementAt(slot, new StringLiteral("I"))); + + List results = new ArrayList<>( + accessPaths.get(slot.getExprId().asInt())); + Assertions.assertEquals(1, results.size()); + Assertions.assertEquals(ImmutableList.of("i", "i"), results.get(0).getPath()); + + DataTypeAccessTree tree = DataTypeAccessTree.ofRoot(slot, ColumnAccessPathType.DATA); + tree.setAccessByPath(results.get(0).getPath(), 0, ColumnAccessPathType.DATA); + StructType prunedType = (StructType) tree.pruneDataType().orElseThrow(); + Assertions.assertEquals(1, prunedType.getFields().size()); + Assertions.assertEquals("i", prunedType.getFields().get(0).getName()); + Assertions.assertEquals(IntegerType.INSTANCE, prunedType.getFields().get(0).getDataType()); + } finally { + Locale.setDefault(originalLocale); + } + } + + @Test + public void testCastStructAccessPathKeepsRootKeyIdentity() { + StructType originType = new StructType(ImmutableList.of( + new StructField("first", IntegerType.INSTANCE, true, ""), + new StructField("second", StringType.INSTANCE, true, ""))); + StructType castType = new StructType(ImmutableList.of( + new StructField("I", IntegerType.INSTANCE, true, ""), + new StructField("ı", StringType.INSTANCE, true, ""))); + SlotReference slot = new SlotReference("s", originType); + Multimap accessPaths = ArrayListMultimap.create(); + AccessPathExpressionCollector collector = + new AccessPathExpressionCollector(null, accessPaths, false, false); + + collector.collect(new ElementAt(new Cast(slot, castType), new StringLiteral("ı"))); + + List results = new ArrayList<>( + accessPaths.get(slot.getExprId().asInt())); + Assertions.assertEquals(1, results.size()); + Assertions.assertEquals(ImmutableList.of("s", "second"), results.get(0).getPath()); + + DataTypeAccessTree tree = DataTypeAccessTree.ofRoot(slot, ColumnAccessPathType.DATA); + tree.setAccessByPath(results.get(0).getPath(), 0, ColumnAccessPathType.DATA); + StructType prunedType = (StructType) tree.pruneDataType().orElseThrow(); + Assertions.assertEquals(1, prunedType.getFields().size()); + Assertions.assertEquals("second", prunedType.getFields().get(0).getName()); + Assertions.assertEquals(StringType.INSTANCE, prunedType.getFields().get(0).getDataType()); + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/literal/StructLiteralTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/literal/StructLiteralTest.java index 1982a6ae67f742..835f7ba344d0ba 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/literal/StructLiteralTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/literal/StructLiteralTest.java @@ -36,6 +36,8 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; +import java.util.Locale; + public class StructLiteralTest { @Test @@ -74,6 +76,24 @@ public void testNamedStructInfersValueNullability() { Assertions.assertTrue(nullableType.getFields().get(0).isNullable()); } + @Test + public void testNamedStructFieldIdentityUsesRootLocale() { + Locale originalLocale = Locale.getDefault(); + try { + Locale.setDefault(Locale.forLanguageTag("tr-TR")); + CreateNamedStruct namedStruct = new CreateNamedStruct( + new StringLiteral("I"), new IntegerLiteral(1), + new StringLiteral("ı"), new IntegerLiteral(2)); + + Assertions.assertDoesNotThrow(namedStruct::checkLegalityBeforeTypeCoercion); + StructType type = (StructType) namedStruct.customSignature().returnType; + Assertions.assertEquals("i", type.getFields().get(0).getName()); + Assertions.assertEquals("ı", type.getFields().get(1).getName()); + } finally { + Locale.setDefault(originalLocale); + } + } + @Test public void testStructFunctionsKeepPhysicalCastNullabilityInStrictMode() { SlotReference requiredString = new SlotReference("metric", StringType.INSTANCE, false); diff --git a/fe/fe-core/src/test/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunctionTest.java b/fe/fe-core/src/test/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunctionTest.java index 34df496439589c..a76d9c39b0ca15 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunctionTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/tablefunction/ExternalFileTableValuedFunctionTest.java @@ -19,10 +19,19 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.StructField; +import org.apache.doris.catalog.StructType; +import org.apache.doris.catalog.Type; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; +import org.apache.doris.common.Pair; import org.apache.doris.common.util.FileFormatConstants; import org.apache.doris.common.util.FileFormatUtils; +import org.apache.doris.proto.Types.PScalarType; +import org.apache.doris.proto.Types.PStructField; +import org.apache.doris.proto.Types.PTypeNode; +import org.apache.doris.thrift.TPrimitiveType; +import org.apache.doris.thrift.TTypeNodeType; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -30,10 +39,43 @@ import org.junit.Test; import org.mockito.Mockito; +import java.lang.reflect.Method; +import java.util.Arrays; import java.util.List; import java.util.Map; public class ExternalFileTableValuedFunctionTest { + @Test + public void testFileSchemaPreservesNestedFieldSpelling() throws Exception { + ExternalFileTableValuedFunction tvf = Mockito.mock( + ExternalFileTableValuedFunction.class, Mockito.CALLS_REAL_METHODS); + PTypeNode structNode = PTypeNode.newBuilder() + .setType(TTypeNodeType.STRUCT.getValue()) + .setScalarType(PScalarType.newBuilder().setType(TPrimitiveType.STRUCT.getValue())) + .addStructFields(PStructField.newBuilder() + .setName("CaseSensitive") + .setComment("mixed-case child") + .setContainsNull(true)) + .build(); + PTypeNode intNode = PTypeNode.newBuilder() + .setType(TTypeNodeType.SCALAR.getValue()) + .setScalarType(PScalarType.newBuilder().setType(TPrimitiveType.INT.getValue())) + .build(); + + Method getColumnType = ExternalFileTableValuedFunction.class + .getDeclaredMethod("getColumnType", List.class, int.class); + getColumnType.setAccessible(true); + @SuppressWarnings("unchecked") + Pair parsed = (Pair) getColumnType.invoke( + tvf, Arrays.asList(structNode, intNode), 0); + + StructField field = ((StructType) parsed.key()).getFields().get(0); + Assert.assertEquals("casesensitive", field.getName()); + Assert.assertEquals("CaseSensitive", field.getOriginalName()); + Assert.assertEquals("mixed-case child", field.getComment()); + Assert.assertTrue(field.getContainsNull()); + } + @Test public void testHiveParquetTimeZoneIsCanonicalizedAndRemovedFromStorageProperties() throws AnalysisException { diff --git a/fe/fe-type/src/main/java/org/apache/doris/catalog/StructField.java b/fe/fe-type/src/main/java/org/apache/doris/catalog/StructField.java index e9432c1efad1c5..a4339d239d60ad 100644 --- a/fe/fe-type/src/main/java/org/apache/doris/catalog/StructField.java +++ b/fe/fe-type/src/main/java/org/apache/doris/catalog/StructField.java @@ -24,10 +24,15 @@ import com.google.common.base.Strings; import com.google.gson.annotations.SerializedName; +import java.util.Locale; + public class StructField { @SerializedName(value = "name") protected final String name; + @SerializedName(value = "originalName") + protected final String originalName; + @SerializedName(value = "type") protected final Type type; @@ -51,7 +56,24 @@ 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.toLowerCase(); + 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) { + // Keep runtime identity locale-independent while preserving external schema spelling separately. + this.name = name.toLowerCase(Locale.ROOT); + this.originalName = originalName; this.type = type; this.comment = comment; this.containsNull = containsNull; @@ -82,6 +104,15 @@ public String getName() { return name; } + public String getOriginalName() { + return originalName == null ? name : originalName; + } + + /** Whether this field was persisted with its external schema spelling. */ + public boolean hasOriginalName() { + return originalName != null; + } + public Type getType() { return type; } @@ -105,7 +136,7 @@ public String toSql(int depth) { } else { typeSql = "..."; } - StringBuilder sb = new StringBuilder(name); + StringBuilder sb = new StringBuilder(getOriginalName()); if (type != null) { sb.append(":").append(typeSql); } @@ -121,7 +152,7 @@ public String toSql(int depth) { */ public String prettyPrint(int lpad) { String leftPadding = Strings.repeat(" ", lpad); - StringBuilder sb = new StringBuilder(leftPadding + name); + StringBuilder sb = new StringBuilder(leftPadding + getOriginalName()); if (type != null) { // Pass in the padding to make sure nested fields are aligned properly, // even if we then strip the top-level padding. @@ -162,7 +193,7 @@ public boolean equals(Object other) { @Override public String toString() { - StringBuilder sb = new StringBuilder(name); + StringBuilder sb = new StringBuilder(getOriginalName()); if (type != null) { sb.append(":").append(type); } diff --git a/fe/fe-type/src/main/java/org/apache/doris/catalog/StructType.java b/fe/fe-type/src/main/java/org/apache/doris/catalog/StructType.java index df06da74313c25..28e3a074382528 100644 --- a/fe/fe-type/src/main/java/org/apache/doris/catalog/StructType.java +++ b/fe/fe-type/src/main/java/org/apache/doris/catalog/StructType.java @@ -33,6 +33,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Objects; /** @@ -51,7 +52,7 @@ public StructType(ArrayList fields) { this.fields = fields; for (int i = 0; i < this.fields.size(); ++i) { this.fields.get(i).setPosition(i); - fieldMap.put(this.fields.get(i).getName().toLowerCase(), this.fields.get(i)); + fieldMap.put(this.fields.get(i).getName().toLowerCase(Locale.ROOT), this.fields.get(i)); } } @@ -122,7 +123,7 @@ public boolean supportSubType(Type subType) { public void addField(StructField field) { field.setPosition(fields.size()); fields.add(field); - fieldMap.put(field.getName().toLowerCase(), field); + fieldMap.put(field.getName().toLowerCase(Locale.ROOT), field); } public ArrayList getFields() { @@ -130,7 +131,23 @@ public ArrayList getFields() { } public StructField getField(String fieldName) { - return fieldMap.get(fieldName.toLowerCase()); + StructField field = fieldMap.get(fieldName.toLowerCase(Locale.ROOT)); + if (field != null && (field.hasOriginalName() || field.getName().equals(fieldName))) { + return field; + } + StructField legacyMatch = null; + for (int i = fields.size() - 1; i >= 0; i--) { + StructField legacyField = fields.get(i); + // Old images lack originalName and may contain keys normalized with the FE's default locale. + if (!legacyField.hasOriginalName() && legacyField.getName().equalsIgnoreCase(fieldName)) { + if (legacyMatch != null) { + // The old locale was not persisted, so choosing either folded sibling could return wrong data. + return null; + } + legacyMatch = legacyField; + } + } + return legacyMatch != null ? legacyMatch : field; } @Override diff --git a/fe/fe-type/src/main/java/org/apache/doris/catalog/Type.java b/fe/fe-type/src/main/java/org/apache/doris/catalog/Type.java index 38dcd8f359c180..fb203cedf9a1f1 100644 --- a/fe/fe-type/src/main/java/org/apache/doris/catalog/Type.java +++ b/fe/fe-type/src/main/java/org/apache/doris/catalog/Type.java @@ -526,7 +526,7 @@ public String hideVersionForVersionColumn( StructType structType = (StructType) this; for (int i = 0; i < structType.getFields().size(); i++) { StructField field = structType.getFields().get(i); - StringBuilder desc = new StringBuilder(field.getName()).append(":") + StringBuilder desc = new StringBuilder(field.getOriginalName()).append(":") .append(field.getType().hideVersionForVersionColumn( isToSql, showNestedComment, noBackslashEscapes)); // Requiredness is schema semantics and must survive independently of whether diff --git a/regression-test/data/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.out b/regression-test/data/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.out index a364316df427a9..8cfc10b82043fa 100644 --- a/regression-test/data/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.out +++ b/regression-test/data/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.out @@ -81,7 +81,7 @@ a_struct struct Yes -- !case_desc -- id bigint Yes true \N -a_struct struct Yes true \N +a_struct struct Yes true \N -- !case_select_all -- 1 {"renamed":11, "keep":12, "drop_and_add":null, "added":null} @@ -129,7 +129,7 @@ a_struct struct Yes -- !case_orc_desc -- id bigint Yes true \N -a_struct struct Yes true \N +a_struct struct Yes true \N -- !case_orc_select_all -- 1 {"renamed":11, "keep":12, "drop_and_add":null, "added":null} diff --git a/regression-test/suites/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.groovy b/regression-test/suites/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.groovy index be03b238c92ac0..d8a6a427d8277d 100644 --- a/regression-test/suites/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.groovy +++ b/regression-test/suites/external_table_p0/iceberg/test_iceberg_struct_schema_evolution.groovy @@ -160,8 +160,7 @@ suite("test_iceberg_struct_schema_evolution", "p0,external") { qt_case_struct_renamed """SELECT element_at(a_struct, 'renamed') FROM ${case_table_name} ORDER BY id""" // Test 3: Query struct field that was dropped and re-added with case change - // Note: Even though we use DROP_AND_ADD (uppercase) in SQL, the system normalizes - // field names to lowercase, so we query with 'drop_and_add' (lowercase) + // Iceberg metadata retains the external spelling, while runtime lookup still uses the normalized name. qt_case_struct_drop_and_add """SELECT element_at(a_struct, 'drop_and_add') FROM ${case_table_name} ORDER BY id""" // Test 4: Query struct field that was newly added @@ -198,8 +197,7 @@ suite("test_iceberg_struct_schema_evolution", "p0,external") { qt_case_orc_struct_renamed """SELECT element_at(a_struct, 'renamed') FROM ${case_orc_table_name} ORDER BY id""" // Test 3: Query struct field that was dropped and re-added with case change - // Note: Even though we use DROP_AND_ADD (uppercase) in SQL, the system normalizes - // field names to lowercase, so we query with 'drop_and_add' (lowercase) + // Iceberg metadata retains the external spelling, while runtime lookup still uses the normalized name. qt_case_orc_struct_drop_and_add """SELECT element_at(a_struct, 'drop_and_add') FROM ${case_orc_table_name} ORDER BY id""" // Test 4: Query struct field that was newly added diff --git a/regression-test/suites/external_table_p0/iceberg/write/test_iceberg_write_ctas_format_boundary.groovy b/regression-test/suites/external_table_p0/iceberg/write/test_iceberg_write_ctas_format_boundary.groovy index 9be6f3ba421281..a6242cc9886a82 100644 --- a/regression-test/suites/external_table_p0/iceberg/write/test_iceberg_write_ctas_format_boundary.groovy +++ b/regression-test/suites/external_table_p0/iceberg/write/test_iceberg_write_ctas_format_boundary.groovy @@ -156,7 +156,89 @@ suite("test_iceberg_write_ctas_format_boundary", } assertEquals(0, (sql """show tables like 'ctas_failed_atomicity'""").size()) - // WC01-S03: Iceberg allows Avro, but the current Doris writer supports + // WC01-S03: FILE TVF must keep the Parquet schema spelling until Iceberg CTAS persists it. + // The normalized names remain available for Doris runtime lookup. + spark_iceberg_multi """ + DROP TABLE IF EXISTS demo.${dbName}.file_tvf_case_source; + CREATE TABLE demo.${dbName}.file_tvf_case_source ( + id INT, + payload STRUCT>, + NestedMap:MAP>> + ) USING iceberg + TBLPROPERTIES ('write.format.default' = 'parquet'); + INSERT INTO demo.${dbName}.file_tvf_case_source VALUES ( + 1, + NAMED_STRUCT( + 'CaseSensitive', CAST(7 AS BIGINT), + 'NestedArray', ARRAY(NAMED_STRUCT('ArrayChild', CAST(8 AS BIGINT))), + 'NestedMap', MAP('k', NAMED_STRUCT('MapChild', CAST(9 AS BIGINT))) + ) + ); + """ + sql """refresh catalog ${catalogName}""" + String sourceFile = (sql """ + select file_path from file_tvf_case_source\$files order by file_path limit 1 + """)[0][0].toString() + + sql """drop table if exists ctas_file_tvf_case""" + sql """ + create table ctas_file_tvf_case as + select payload from file ( + "uri" = "${sourceFile}", + "format" = "parquet", + "s3.endpoint" = "http://${externalEnvIp}:${minioPort}", + "s3.region" = "us-east-1", + "s3.access_key" = "admin", + "s3.secret_key" = "password", + "use_path_style" = "true" + ) + """ + + def ctasSchema = spark_iceberg """describe demo.${dbName}.ctas_file_tvf_case""" + def payloadRow = ctasSchema.find { row -> row[0].toString() == "payload" } + assertNotNull(payloadRow, "payload column should exist in the Iceberg CTAS schema") + String payloadType = payloadRow[1].toString() + assertTrue(payloadType.contains("CaseSensitive"), payloadType) + assertTrue(payloadType.contains("NestedArray"), payloadType) + assertTrue(payloadType.contains("ArrayChild"), payloadType) + assertTrue(payloadType.contains("NestedMap"), payloadType) + assertTrue(payloadType.contains("MapChild"), payloadType) + + def nestedValues = sql """ + select element_at(payload, 'casesensitive'), + element_at(element_at(payload, 'nestedarray')[1], 'arraychild'), + element_at(element_at(payload, 'nestedmap')['k'], 'mapchild') + from ctas_file_tvf_case + """ + assertEquals([[7L, 8L, 9L]], nestedValues) + + // WC01-S04: Names displayed from external metadata must remain executable even when Java ROOT lowercasing + // changes their Unicode bytes or UTF-8 length before thrift reaches the BE. + spark_iceberg_multi """ + DROP TABLE IF EXISTS demo.${dbName}.unicode_struct_fields; + CREATE TABLE demo.${dbName}.unicode_struct_fields ( + id INT, + payload STRUCT<`Σ`:BIGINT, `ẞ`:BIGINT> + ) USING iceberg; + INSERT INTO demo.${dbName}.unicode_struct_fields VALUES ( + 1, NAMED_STRUCT('Σ', CAST(10 AS BIGINT), 'ẞ', CAST(11 AS BIGINT)) + ); + """ + sql """refresh catalog ${catalogName}""" + def unicodeSchema = sql """describe unicode_struct_fields""" + def unicodePayloadRow = unicodeSchema.find { row -> row[0].toString() == "payload" } + assertNotNull(unicodePayloadRow, "payload column should exist in the Unicode Iceberg schema") + String unicodePayloadType = unicodePayloadRow[1].toString() + assertTrue(unicodePayloadType.contains("Σ"), unicodePayloadType) + assertTrue(unicodePayloadType.contains("ẞ"), unicodePayloadType) + def unicodeValues = sql """ + select element_at(payload, 'Σ'), element_at(payload, 'ẞ') + from unicode_struct_fields + """ + assertEquals([[10L, 11L]], unicodeValues) + + // WC01-S05: Iceberg allows Avro, but the current Doris writer supports // Parquet and ORC only. Reject Avro explicitly instead of silently falling back. sql """drop table if exists avro_write_boundary""" sql """