diff --git a/be/test/format/table/iceberg/iceberg_reader_test.cpp b/be/test/format/table/iceberg/iceberg_reader_test.cpp index f639bc5b782ecc..297ea0848fb7be 100644 --- a/be/test/format/table/iceberg/iceberg_reader_test.cpp +++ b/be/test/format/table/iceberg/iceberg_reader_test.cpp @@ -1284,13 +1284,18 @@ TEST_F(IcebergReaderTest, v1_materializes_missing_equality_delete_initial_defaul return field_ptr; }; + static_assert(sizeof("0123456789abcdef0123456789abcdef") - 1 > StringView::kInlineSize); + const std::string binary_default = "0123456789abcdef0123456789abcdef"; + schema::external::TStructField root_field; - root_field.__set_fields({make_field("added_timestamp", 1, TPrimitiveType::DATETIMEV2, - "2024-01-01 00:00:00.123456", -1, 6, false), - make_field("added_binary", 2, TPrimitiveType::VARBINARY, - "Ej5FZ+ibEtOkVkJmFBdAAA==", 16, -1, true), - make_field("added_string_binary", 3, TPrimitiveType::STRING, - "AAEC/w==", -1, -1, true)}); + root_field.__set_fields( + {make_field("added_timestamp", 1, TPrimitiveType::DATETIMEV2, + "2024-01-01 00:00:00.123456", -1, 6, false), + make_field("added_binary", 2, TPrimitiveType::VARBINARY, + "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=", + static_cast(binary_default.size()), -1, true), + make_field("added_string_binary", 3, TPrimitiveType::STRING, + "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=", -1, -1, true)}); schema::external::TSchema current_schema; current_schema.__set_schema_id(-1); current_schema.__set_root_field(root_field); @@ -1312,7 +1317,8 @@ TEST_F(IcebergReaderTest, v1_materializes_missing_equality_delete_initial_defaul &runtime_state, cache.get()); const auto timestamp_type = make_nullable(std::make_shared(6)); - const auto varbinary_type = make_nullable(std::make_shared(16)); + const auto varbinary_type = make_nullable( + std::make_shared(static_cast(binary_default.size()))); const auto string_type = make_nullable(std::make_shared()); Block block; block.insert({timestamp_type->create_column(), timestamp_type, "added_timestamp"}); @@ -1335,10 +1341,8 @@ TEST_F(IcebergReaderTest, v1_materializes_missing_equality_delete_initial_defaul ASSERT_EQ(block.rows(), 3); EXPECT_EQ(timestamp_type->to_string(*block.get_by_position(0).column, 0), "2024-01-01 00:00:00.123456"); - EXPECT_EQ(varbinary_type->to_string(*block.get_by_position(1).column, 0), - std::string("\x12\x3e\x45\x67\xe8\x9b\x12\xd3\xa4\x56\x42\x66\x14\x17\x40\x00", 16)); - EXPECT_EQ(string_type->to_string(*block.get_by_position(2).column, 0), - std::string("\x00\x01\x02\xff", 4)); + EXPECT_EQ(varbinary_type->to_string(*block.get_by_position(1).column, 0), binary_default); + EXPECT_EQ(string_type->to_string(*block.get_by_position(2).column, 0), binary_default); EXPECT_FALSE(is_column_const(*block.get_by_position(0).column)); EXPECT_FALSE(is_column_const(*block.get_by_position(1).column)); EXPECT_FALSE(is_column_const(*block.get_by_position(2).column)); diff --git a/be/test/format_v2/table/iceberg_reader_test.cpp b/be/test/format_v2/table/iceberg_reader_test.cpp index e02ab1f082a3c0..d1aca7a1059098 100644 --- a/be/test/format_v2/table/iceberg_reader_test.cpp +++ b/be/test/format_v2/table/iceberg_reader_test.cpp @@ -2449,8 +2449,8 @@ TEST(IcebergV2ReaderTest, IcebergEqualityDeleteMatchesVarbinaryInitialDefaultFor const auto file_path = (test_dir / "split.parquet").string(); const auto delete_file_path = (test_dir / "equality-delete.parquet").string(); write_single_int_parquet_file(file_path, "id", {1, 2, 3}, 0); - const std::string binary_default( - "\x12\x3e\x45\x67\xe8\x9b\x12\xd3\xa4\x56\x42\x66\x14\x17\x40\x00", 16); + static_assert(sizeof("0123456789abcdef0123456789abcdef") - 1 > StringView::kInlineSize); + const std::string binary_default = "0123456789abcdef0123456789abcdef"; write_iceberg_binary_equality_delete_parquet_file(delete_file_path, 1, binary_default, "added_binary"); @@ -2460,9 +2460,11 @@ TEST(IcebergV2ReaderTest, IcebergEqualityDeleteMatchesVarbinaryInitialDefaultFor scan_params.__set_current_schema_id(100); scan_params.__set_history_schema_info({external_schema( 100, {external_schema_field("id", 0), - external_schema_field("added_binary", 1, {}, "Ej5FZ+ibEtOkVkJmFBdAAA==", - external_primitive_type(TPrimitiveType::VARBINARY, 16), - true)})}); + external_schema_field( + "added_binary", 1, {}, "MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY=", + external_primitive_type(TPrimitiveType::VARBINARY, + static_cast(binary_default.size())), + true)})}); RuntimeProfile profile("test_profile"); RuntimeState state {TQueryOptions(), TQueryGlobals()}; diff --git a/fe/fe-catalog/src/main/java/org/apache/doris/catalog/Column.java b/fe/fe-catalog/src/main/java/org/apache/doris/catalog/Column.java index aaf76f5c5da864..7cac33db8089bf 100644 --- a/fe/fe-catalog/src/main/java/org/apache/doris/catalog/Column.java +++ b/fe/fe-catalog/src/main/java/org/apache/doris/catalog/Column.java @@ -142,6 +142,10 @@ public static Column generateBeforeValueColumn(Column column) { private boolean isKey; @SerializedName(value = "isAllowNull") private boolean isAllowNull; + // Runtime-only schema change intent; do not persist it as part of the table schema. + private transient boolean nullableSpecified; + // Runtime-only schema change intent; do not persist it as part of the table schema. + private transient boolean commentSpecified; @SerializedName(value = "isAutoInc") private boolean isAutoInc; @@ -368,6 +372,8 @@ public Column(Column column) { this.isKey = column.isKey(); this.isCompoundKey = column.isCompoundKey(); this.isAllowNull = column.isAllowNull(); + this.nullableSpecified = column.isNullableSpecified(); + this.commentSpecified = column.isCommentSpecified(); this.isAutoInc = column.isAutoInc(); this.defaultValue = column.getDefaultValue(); this.realDefaultValue = column.realDefaultValue; @@ -583,6 +589,14 @@ public boolean isAllowNull() { return isAllowNull; } + public boolean isNullableSpecified() { + return nullableSpecified; + } + + public boolean isCommentSpecified() { + return commentSpecified; + } + public boolean isAutoInc() { return isAutoInc; } @@ -595,6 +609,14 @@ public void setIsAllowNull(boolean isAllowNull) { this.isAllowNull = isAllowNull; } + public void setNullableSpecified(boolean nullableSpecified) { + this.nullableSpecified = nullableSpecified; + } + + public void setCommentSpecified(boolean commentSpecified) { + this.commentSpecified = commentSpecified; + } + public String getDefaultValue() { return this.defaultValue; } diff --git a/fe/fe-catalog/src/main/java/org/apache/doris/catalog/ColumnType.java b/fe/fe-catalog/src/main/java/org/apache/doris/catalog/ColumnType.java index 015fecfd28a263..483f22e2425f16 100644 --- a/fe/fe-catalog/src/main/java/org/apache/doris/catalog/ColumnType.java +++ b/fe/fe-catalog/src/main/java/org/apache/doris/catalog/ColumnType.java @@ -322,11 +322,10 @@ private static void checkSupportSchemaChangeForComplexType(Type checkType, Type existingNames.add(originalField.getName()); } - // check new field name is not conflict with old field name + // check appended field names do not conflict with existing or earlier appended fields for (int i = originalFields.size(); i < otherStructType.getFields().size(); i++) { - // to check new field name is not conflict with old field name String newFieldName = otherStructType.getFields().get(i).getName(); - if (existingNames.contains(newFieldName)) { + if (!existingNames.add(newFieldName)) { throw new DdlException("Added struct field '" + newFieldName + "' conflicts with existing field"); } } diff --git a/fe/fe-catalog/src/main/java/org/apache/doris/catalog/info/ColumnPosition.java b/fe/fe-catalog/src/main/java/org/apache/doris/catalog/info/ColumnPosition.java index 7c30c725d6c3a4..5b6b53918ee59c 100644 --- a/fe/fe-catalog/src/main/java/org/apache/doris/catalog/info/ColumnPosition.java +++ b/fe/fe-catalog/src/main/java/org/apache/doris/catalog/info/ColumnPosition.java @@ -18,6 +18,7 @@ package org.apache.doris.catalog.info; import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.util.SqlUtils; import com.google.common.base.Strings; @@ -57,7 +58,7 @@ public String toSql() { if (this == FIRST) { sb.append("FIRST"); } else { - sb.append("AFTER `").append(lastCol).append("`"); + sb.append("AFTER ").append(SqlUtils.getIdentSql(lastCol)); } return sb.toString(); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java index 9f9eb03ceec676..85b2433092ecb8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java @@ -415,20 +415,26 @@ private void processAlterTableForExternalTable( table.getDbName(), table.getName(), renameTableOp.getNewTableName()); } else if (alterOp instanceof AddColumnOp) { AddColumnOp addColumn = (AddColumnOp) alterOp; - table.getCatalog().addColumn(table, addColumn.getColumn(), addColumn.getColPos()); + table.getCatalog().addColumn( + table, addColumn.getColumnPath(), addColumn.getColumn(), addColumn.getColPos()); } else if (alterOp instanceof AddColumnsOp) { AddColumnsOp addColumns = (AddColumnsOp) alterOp; table.getCatalog().addColumns(table, addColumns.getColumns()); } else if (alterOp instanceof DropColumnOp) { DropColumnOp dropColumn = (DropColumnOp) alterOp; - table.getCatalog().dropColumn(table, dropColumn.getColName()); + table.getCatalog().dropColumn(table, dropColumn.getColumnPath()); } else if (alterOp instanceof RenameColumnOp) { RenameColumnOp columnRename = (RenameColumnOp) alterOp; table.getCatalog().renameColumn( - table, columnRename.getColName(), columnRename.getNewColName()); + table, columnRename.getColumnPath(), columnRename.getNewColName()); } else if (alterOp instanceof ModifyColumnOp) { ModifyColumnOp modifyColumn = (ModifyColumnOp) alterOp; - table.getCatalog().modifyColumn(table, modifyColumn.getColumn(), modifyColumn.getColPos()); + table.getCatalog().modifyColumn( + table, modifyColumn.getColumnPath(), modifyColumn.getColumn(), modifyColumn.getColPos()); + } else if (alterOp instanceof ModifyColumnCommentOp) { + ModifyColumnCommentOp modifyColumnComment = (ModifyColumnCommentOp) alterOp; + table.getCatalog().modifyColumnComment( + table, modifyColumnComment.getColumnPath(), modifyColumnComment.getComment()); } else if (alterOp instanceof ReorderColumnsOp) { ReorderColumnsOp reorderColumns = (ReorderColumnsOp) alterOp; table.getCatalog().reorderColumns(table, reorderColumns.getColumnsByPos()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnPath.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnPath.java new file mode 100644 index 00000000000000..0e28f38de41ef5 --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnPath.java @@ -0,0 +1,92 @@ +// 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.analysis; + +import org.apache.doris.common.util.SqlUtils; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Represents a column path used by schema change statements. + */ +public class ColumnPath { + private final ImmutableList parts; + + private ColumnPath(List parts) { + Preconditions.checkArgument(parts != null && !parts.isEmpty(), "column path is empty"); + for (String part : parts) { + Preconditions.checkArgument(part != null && !part.isEmpty(), "column path contains empty part"); + } + this.parts = ImmutableList.copyOf(parts); + } + + public static ColumnPath of(List parts) { + return new ColumnPath(parts); + } + + public static ColumnPath of(String name) { + return new ColumnPath(ImmutableList.of(name)); + } + + public static ColumnPath fromDotName(String name) { + return new ColumnPath(Arrays.asList(name.split("\\."))); + } + + public List getParts() { + return parts; + } + + public boolean isNested() { + return parts.size() > 1; + } + + public String getTopLevelName() { + return parts.get(0); + } + + public String getLeafName() { + return parts.get(parts.size() - 1); + } + + public ColumnPath getParentPath() { + Preconditions.checkState(isNested(), "top-level column path has no parent"); + return new ColumnPath(parts.subList(0, parts.size() - 1)); + } + + public String getParentPathString() { + return getParentPath().getFullPath(); + } + + public String getFullPath() { + return String.join(".", parts); + } + + public String toSql() { + return parts.stream().map(SqlUtils::getIdentSql).collect(Collectors.joining(".")); + } + + @Override + public String toString() { + return getFullPath(); + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java b/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java index 0cf43bb1757cca..9e40042d32d754 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/FeNameFormat.java @@ -107,11 +107,18 @@ public static void checkColumnName(String columnName) throws AnalysisException { } public static void checkColumnNameBypassHiddenColumn(String columnName) throws AnalysisException { + checkColumnNameBypassSystemColumnPrefix(columnName); + checkColumnNamePrefix(columnName, Column.SHADOW_NAME_PREFIX); + } + + /** + * Check column name syntax without applying Doris top-level hidden/shadow column prefix rules. + */ + public static void checkColumnNameBypassSystemColumnPrefix(String columnName) throws AnalysisException { if (Strings.isNullOrEmpty(columnName) || !columnName.matches(getColumnNameRegex())) { ErrorReport.reportAnalysisException(ErrorCode.ERR_WRONG_COLUMN_NAME, columnName, getColumnNameRegex()); } - checkColumnNamePrefix(columnName, Column.SHADOW_NAME_PREFIX); } private static void checkColumnNamePrefix(String columnName, String prefix) throws AnalysisException { diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java index c598ef520f5ef6..5c4decd93a30b3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java @@ -17,6 +17,7 @@ package org.apache.doris.datasource; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.TableIf; @@ -249,6 +250,15 @@ default void addColumn(TableIf table, Column column, ColumnPosition columnPositi throw new UserException("Not support add column operation"); } + default void addColumn(TableIf table, ColumnPath columnPath, Column column, ColumnPosition columnPosition) + throws UserException { + if (!columnPath.isNested()) { + addColumn(table, column, columnPosition); + return; + } + throw new UserException("Not support nested add column operation"); + } + default void addColumns(TableIf table, List columns) throws UserException { throw new UserException("Not support add columns operation"); } @@ -257,14 +267,47 @@ default void dropColumn(TableIf table, String name) throws UserException { throw new UserException("Not support drop column operation"); } + default void dropColumn(TableIf table, ColumnPath columnPath) throws UserException { + if (!columnPath.isNested()) { + dropColumn(table, columnPath.getTopLevelName()); + return; + } + throw new UserException("Not support nested drop column operation"); + } + default void renameColumn(TableIf table, String oldName, String newName) throws UserException { throw new UserException("Not support rename column operation"); } + default void renameColumn(TableIf table, ColumnPath columnPath, String newName) throws UserException { + if (!columnPath.isNested()) { + renameColumn(table, columnPath.getTopLevelName(), newName); + return; + } + throw new UserException("Not support nested rename column operation"); + } + default void modifyColumn(TableIf table, Column column, ColumnPosition columnPosition) throws UserException { throw new UserException("Not support update column operation"); } + default void modifyColumn(TableIf table, ColumnPath columnPath, Column column, ColumnPosition columnPosition) + throws UserException { + if (!columnPath.isNested()) { + modifyColumn(table, column, columnPosition); + return; + } + throw new UserException("Not support nested modify column operation"); + } + + default void modifyColumnComment(TableIf table, String name, String comment) throws UserException { + modifyColumnComment(table, ColumnPath.of(name), comment); + } + + default void modifyColumnComment(TableIf table, ColumnPath columnPath, String comment) throws UserException { + throw new UserException("Not support modify column comment operation"); + } + default void reorderColumns(TableIf table, List newOrder) throws UserException { throw new UserException("Not support reorder columns operation"); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java index a742a50f9a62d6..341477c9911253 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java @@ -17,6 +17,7 @@ package org.apache.doris.datasource; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; @@ -1536,6 +1537,12 @@ private void logRefreshExternalTable(ExternalTable dorisTable, long updateTime) @Override public void addColumn(TableIf dorisTable, Column column, ColumnPosition position) throws UserException { + addColumn(dorisTable, ColumnPath.of(column.getName()), column, position); + } + + @Override + public void addColumn(TableIf dorisTable, ColumnPath columnPath, Column column, ColumnPosition position) + throws UserException { makeSureInitialized(); Preconditions.checkState(dorisTable instanceof ExternalTable, dorisTable.getName()); ExternalTable externalTable = (ExternalTable) dorisTable; @@ -1544,11 +1551,11 @@ public void addColumn(TableIf dorisTable, Column column, ColumnPosition position } try { long updateTime = System.currentTimeMillis(); - metadataOps.addColumn(externalTable, column, position, updateTime); + metadataOps.addColumn(externalTable, columnPath, column, position, updateTime); logRefreshExternalTable(externalTable, updateTime); } catch (Exception e) { LOG.warn("Failed to add column {} to table {}.{} in catalog {}", - column.getName(), externalTable.getDbName(), externalTable.getName(), getName(), e); + columnPath.getFullPath(), externalTable.getDbName(), externalTable.getName(), getName(), e); throw e; } } @@ -1574,6 +1581,11 @@ public void addColumns(TableIf dorisTable, List columns) throws UserExce @Override public void dropColumn(TableIf dorisTable, String columnName) throws UserException { + dropColumn(dorisTable, ColumnPath.of(columnName)); + } + + @Override + public void dropColumn(TableIf dorisTable, ColumnPath columnPath) throws UserException { makeSureInitialized(); Preconditions.checkState(dorisTable instanceof ExternalTable, dorisTable.getName()); ExternalTable externalTable = (ExternalTable) dorisTable; @@ -1582,17 +1594,22 @@ public void dropColumn(TableIf dorisTable, String columnName) throws UserExcepti } try { long updateTime = System.currentTimeMillis(); - metadataOps.dropColumn(externalTable, columnName, updateTime); + metadataOps.dropColumn(externalTable, columnPath, updateTime); logRefreshExternalTable(externalTable, updateTime); } catch (Exception e) { LOG.warn("Failed to drop column {} from table {}.{} in catalog {}", - columnName, externalTable.getDbName(), externalTable.getName(), getName(), e); + columnPath.getFullPath(), externalTable.getDbName(), externalTable.getName(), getName(), e); throw e; } } @Override public void renameColumn(TableIf dorisTable, String oldName, String newName) throws UserException { + renameColumn(dorisTable, ColumnPath.of(oldName), newName); + } + + @Override + public void renameColumn(TableIf dorisTable, ColumnPath columnPath, String newName) throws UserException { makeSureInitialized(); Preconditions.checkState(dorisTable instanceof ExternalTable, dorisTable.getName()); ExternalTable externalTable = (ExternalTable) dorisTable; @@ -1601,11 +1618,11 @@ public void renameColumn(TableIf dorisTable, String oldName, String newName) thr } try { long updateTime = System.currentTimeMillis(); - metadataOps.renameColumn(externalTable, oldName, newName, updateTime); + metadataOps.renameColumn(externalTable, columnPath, newName, updateTime); logRefreshExternalTable(externalTable, updateTime); } catch (Exception e) { - LOG.warn("Failed to rename column {} to {} in table {}.{} in catalog {}", - oldName, newName, externalTable.getDbName(), externalTable.getName(), getName(), e); + LOG.warn("Failed to rename column {} to {} in table {}.{} in catalog {}", columnPath.getFullPath(), + newName, externalTable.getDbName(), externalTable.getName(), getName(), e); throw e; } } @@ -1629,6 +1646,45 @@ public void modifyColumn(TableIf dorisTable, Column column, ColumnPosition colum } } + @Override + public void modifyColumn(TableIf dorisTable, ColumnPath columnPath, Column column, ColumnPosition columnPosition) + throws UserException { + makeSureInitialized(); + Preconditions.checkState(dorisTable instanceof ExternalTable, dorisTable.getName()); + ExternalTable externalTable = (ExternalTable) dorisTable; + if (metadataOps == null) { + throw new DdlException("Modify column operation is not supported for catalog: " + getName()); + } + try { + long updateTime = System.currentTimeMillis(); + metadataOps.modifyColumn(externalTable, columnPath, column, columnPosition, updateTime); + logRefreshExternalTable(externalTable, updateTime); + } catch (Exception e) { + LOG.warn("Failed to modify column {} in table {}.{} in catalog {}", + columnPath.getFullPath(), externalTable.getDbName(), externalTable.getName(), getName(), e); + throw e; + } + } + + @Override + public void modifyColumnComment(TableIf dorisTable, ColumnPath columnPath, String comment) throws UserException { + makeSureInitialized(); + Preconditions.checkState(dorisTable instanceof ExternalTable, dorisTable.getName()); + ExternalTable externalTable = (ExternalTable) dorisTable; + if (metadataOps == null) { + throw new DdlException("Modify column comment operation is not supported for catalog: " + getName()); + } + try { + long updateTime = System.currentTimeMillis(); + metadataOps.modifyColumnComment(externalTable, columnPath, comment, updateTime); + logRefreshExternalTable(externalTable, updateTime); + } catch (Exception e) { + LOG.warn("Failed to modify column comment {} in table {}.{} in catalog {}", + columnPath.getFullPath(), externalTable.getDbName(), externalTable.getName(), getName(), e); + throw e; + } + } + @Override public void reorderColumns(TableIf dorisTable, List newOrder) throws UserException { makeSureInitialized(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java index c8d9c50c21da6e..c6363c61cecd61 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java @@ -17,6 +17,7 @@ package org.apache.doris.datasource.iceberg; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.ArrayType; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.ColumnType; @@ -81,6 +82,7 @@ import org.apache.iceberg.expressions.Literal; import org.apache.iceberg.expressions.Term; import org.apache.iceberg.types.Type; +import org.apache.iceberg.types.TypeUtil; import org.apache.iceberg.types.Types; import org.apache.iceberg.types.Types.NestedField; import org.apache.logging.log4j.LogManager; @@ -95,6 +97,8 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.ThreadPoolExecutor; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -812,19 +816,54 @@ private void addOneColumn(UpdateSchema updateSchema, Column column) throws UserE if (!column.isAllowNull()) { throw new UserException("can't add a non-nullable column to an Iceberg table"); } - org.apache.iceberg.types.Type dorisType = IcebergUtils.dorisTypeToIcebergType(column.getType()); + org.apache.iceberg.types.Type dorisType = + toIcebergTypeForSchemaChange(column.getType(), column.getName()); Literal defaultValue = IcebergUtils.parseIcebergLiteral(column.getDefaultValue(), dorisType); updateSchema.addColumn(column.getName(), dorisType, column.getComment(), defaultValue); } - private void applyPosition(UpdateSchema updateSchema, ColumnPosition position, String columnName) { + private void applyPosition(UpdateSchema updateSchema, ColumnPosition position, ColumnPath columnPath, Schema schema, + String operation) throws UserException { + String columnName = columnPath.getFullPath(); if (position.isFirst()) { updateSchema.moveFirst(columnName); } else { - updateSchema.moveAfter(columnName, position.getLastCol()); + updateSchema.moveAfter(columnName, getPositionReferencePath(schema, columnPath, position, operation)); } } + private void validatePositionTarget(Schema schema, ColumnPath columnPath, String operation) + throws UserException { + if (!columnPath.isNested()) { + return; + } + ResolvedColumnPath parentPath = resolveColumnPath(schema, columnPath.getParentPath(), operation); + if (!parentPath.getType().isStructType()) { + throw new UserException("Cannot apply column position to '" + columnPath.getFullPath() + + "': parent column path '" + parentPath.getFullPath() + "' is not a struct"); + } + } + + @VisibleForTesting + String getPositionReferencePath(ColumnPath columnPath, ColumnPosition position) { + if (position == null || position.isFirst() || !columnPath.isNested()) { + return position == null || position.isFirst() ? null : position.getLastCol(); + } + return columnPath.getParentPathString() + "." + position.getLastCol(); + } + + @VisibleForTesting + String getPositionReferencePath(Schema schema, ColumnPath columnPath, ColumnPosition position, String operation) + throws UserException { + if (position == null || position.isFirst()) { + return null; + } + ColumnPath referencePath = columnPath.isNested() + ? childPath(columnPath.getParentPath(), position.getLastCol()) + : ColumnPath.of(position.getLastCol()); + return resolveColumnPath(schema, referencePath, operation).getFullPath(); + } + private void refreshTable(ExternalTable dorisTable, long updateTime) { Optional> db = dorisCatalog.getDbForReplay(dorisTable.getRemoteDbName()); if (db.isPresent()) { @@ -839,12 +878,16 @@ private void refreshTable(ExternalTable dorisTable, long updateTime) { @Override public void addColumn(ExternalTable dorisTable, Column column, ColumnPosition position, long updateTime) throws UserException { - validateCommonColumnInfo(column); + validateAddColumnMetadata(column, true); Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + validateRowLineageColumnMutation(icebergTable, column.getName(), "add"); + Schema schema = icebergTable.schema(); + validateNoCaseInsensitiveSiblingCollision( + schema.asStruct(), "", column.getName(), null, "add"); UpdateSchema updateSchema = icebergTable.updateSchema(); addOneColumn(updateSchema, column); if (position != null) { - applyPosition(updateSchema, position, column.getName()); + applyPosition(updateSchema, position, ColumnPath.of(column.getName()), schema, "add"); } try { executionAuthenticator.execute(() -> updateSchema.commit()); @@ -855,12 +898,55 @@ 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; + } + validateNestedAddColumnMetadata(column, columnPath); + 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 = + toIcebergTypeForSchemaChange(column.getType(), columnPath.getFullPath()); + updateSchema.addColumn(parentPath.getFullPath(), columnPath.getLeafName(), dorisType, + column.getComment()); + if (position != null) { + applyPosition(updateSchema, position, childPath(parentPath.getColumnPath(), columnPath.getLeafName()), + icebergTable.schema(), "add"); + } + try { + executionAuthenticator.execute(() -> updateSchema.commit()); + } catch (Exception e) { + throw new UserException("Failed to add nested column: " + columnPath.getFullPath() + " to table: " + + icebergTable.name() + ", error message is: " + e.getMessage(), e); + } + refreshTable(dorisTable, updateTime); + } + @Override public void addColumns(ExternalTable dorisTable, List columns, long updateTime) throws UserException { Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + for (Column column : columns) { + validateAddColumnMetadata(column, true); + validateRowLineageColumnMutation(icebergTable, column.getName(), "add"); + } + validateNoCaseInsensitiveTopLevelCollisions(icebergTable.schema(), columns); + UpdateSchema updateSchema = icebergTable.updateSchema(); for (Column column : columns) { - validateCommonColumnInfo(column); addOneColumn(updateSchema, column); } try { @@ -875,8 +961,10 @@ public void addColumns(ExternalTable dorisTable, List columns, long upda @Override public void dropColumn(ExternalTable dorisTable, String columnName, long updateTime) throws UserException { Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + validateRowLineageColumnMutation(icebergTable, columnName, "drop"); + ResolvedColumnPath columnPath = resolveColumnPath(icebergTable.schema(), ColumnPath.of(columnName), "drop"); UpdateSchema updateSchema = icebergTable.updateSchema(); - updateSchema.deleteColumn(columnName); + updateSchema.deleteColumn(columnPath.getFullPath()); try { executionAuthenticator.execute(() -> updateSchema.commit()); } catch (Exception e) { @@ -886,12 +974,38 @@ public void dropColumn(ExternalTable dorisTable, String columnName, long updateT refreshTable(dorisTable, updateTime); } + @Override + public void dropColumn(ExternalTable dorisTable, ColumnPath columnPath, long updateTime) throws UserException { + if (!columnPath.isNested()) { + dropColumn(dorisTable, columnPath.getTopLevelName(), updateTime); + return; + } + Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + ResolvedColumnPath resolvedPath = validateNestedStructFieldPath(icebergTable.schema(), columnPath, "drop"); + + UpdateSchema updateSchema = icebergTable.updateSchema(); + updateSchema.deleteColumn(resolvedPath.getFullPath()); + try { + executionAuthenticator.execute(() -> updateSchema.commit()); + } catch (Exception e) { + throw new UserException("Failed to drop nested column: " + columnPath.getFullPath() + " from table: " + + icebergTable.name() + ", error message is: " + e.getMessage(), e); + } + refreshTable(dorisTable, updateTime); + } + @Override public void renameColumn(ExternalTable dorisTable, String oldName, String newName, long updateTime) throws UserException { Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + validateRowLineageColumnMutation(icebergTable, oldName, "rename"); + validateRowLineageColumnMutation(icebergTable, newName, "rename to"); + Schema schema = icebergTable.schema(); + ResolvedColumnPath oldPath = resolveColumnPath(schema, ColumnPath.of(oldName), "rename"); + validateNoCaseInsensitiveSiblingCollision( + schema.asStruct(), "", newName, oldPath.getField(), "rename"); UpdateSchema updateSchema = icebergTable.updateSchema(); - updateSchema.renameColumn(oldName, newName); + applyRenameColumn(schema, updateSchema, oldPath, newName); try { executionAuthenticator.execute(() -> updateSchema.commit()); } catch (Exception e) { @@ -901,42 +1015,112 @@ 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(); + applyRenameColumn(icebergTable.schema(), updateSchema, resolvedPath, 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); + } + + private void applyRenameColumn(Schema schema, UpdateSchema updateSchema, + ResolvedColumnPath oldPath, String newName) { + String oldFullPath = oldPath.getFullPath(); + ColumnPath renamedPath = oldPath.getColumnPath().isNested() + ? childPath(oldPath.getColumnPath().getParentPath(), newName) + : ColumnPath.of(newName); + String renamedFullPath = renamedPath.getFullPath(); + boolean identifierFieldRenamed = false; + Set renamedIdentifierFields = new TreeSet<>(); + int renamedFieldId = oldPath.getField().fieldId(); + // Iceberg 1.10.1 does not preserve full identifier paths when an identifier field or one + // of its ancestors is renamed. Use field identity so dotted sibling names are not mistaken for descendants. + for (int identifierFieldId : schema.identifierFieldIds()) { + String identifierField = schema.findColumnName(identifierFieldId); + boolean isRenamedField = identifierFieldId == renamedFieldId; + boolean isDescendant = TypeUtil.ancestorFields(schema, identifierFieldId).stream() + .anyMatch(field -> field.fieldId() == renamedFieldId); + if (isRenamedField || isDescendant) { + renamedIdentifierFields.add(renamedFullPath + identifierField.substring(oldFullPath.length())); + identifierFieldRenamed = true; + } else { + renamedIdentifierFields.add(identifierField); + } + } + + updateSchema.renameColumn(oldFullPath, newName); + if (identifierFieldRenamed) { + updateSchema.setIdentifierFields(renamedIdentifierFields); + } + } + @Override public void modifyColumn(ExternalTable dorisTable, Column column, ColumnPosition position, long updateTime) throws UserException { + // This overload predates nullableSpecified/commentSpecified. Keep its top-level values + // explicit while delegating to the path-aware implementation. + Column explicitColumn = new Column(column); + explicitColumn.setIsKey(false); + explicitColumn.setNullableSpecified(true); + explicitColumn.setCommentSpecified(true); + modifyColumn(dorisTable, ColumnPath.of(column.getName()), explicitColumn, position, updateTime); + } + + private void modifyTopLevelColumn(ExternalTable dorisTable, ColumnPath columnPath, Column column, + ColumnPosition position, long updateTime) throws UserException { Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); - NestedField currentCol = icebergTable.schema().findField(column.getName()); + validateRowLineageColumnMutation(icebergTable, columnPath.getTopLevelName(), "modify"); + NestedField currentCol = icebergTable.schema().asStruct() + .caseInsensitiveField(columnPath.getTopLevelName()); if (currentCol == null) { - throw new UserException("Column " + column.getName() + " does not exist"); + throw new UserException("Column " + columnPath.getTopLevelName() + " does not exist"); } + ResolvedColumnPath resolvedPath = new ResolvedColumnPath(ColumnPath.of(currentCol.name()), + currentCol.type(), currentCol); - validateCommonColumnInfo(column); - UpdateSchema updateSchema = icebergTable.updateSchema(); - + validateModifyColumnMetadata(column, resolvedPath.getFullPath(), true); + org.apache.iceberg.types.Type targetType; if (column.getType().isComplexType()) { - // Complex type processing branch validateForModifyComplexColumn(column, currentCol); - applyComplexTypeChange(updateSchema, column.getName(), currentCol.type(), column.getType()); - if (column.isAllowNull()) { - updateSchema.makeColumnOptional(column.getName()); - } - if (!Objects.equals(currentCol.doc(), column.getComment())) { - updateSchema.updateColumnDoc(column.getName(), column.getComment()); - } + targetType = currentCol.type(); } else { - // Primitive type processing (existing logic) validateForModifyColumn(column, currentCol); - Type icebergType = IcebergUtils.dorisTypeToIcebergType(column.getType()); - updateSchema.updateColumn(column.getName(), icebergType.asPrimitiveType(), column.getComment()); - if (column.isAllowNull()) { - // we can change a required column to optional, but not the other way around - // because we don't know whether there is existing data with null values. - updateSchema.makeColumnOptional(column.getName()); + targetType = resolvePrimitiveTypeForModify( + currentCol.type(), column.getType(), resolvedPath.getFullPath()); + } + + UpdateSchema updateSchema = icebergTable.updateSchema(); + String targetComment = resolveTargetComment(currentCol, column); + if (column.getType().isComplexType()) { + applyComplexTypeChange(updateSchema, resolvedPath.getFullPath(), currentCol.type(), + column.getType()); + if (!Objects.equals(currentCol.doc(), targetComment)) { + updateSchema.updateColumnDoc(resolvedPath.getFullPath(), targetComment); } + } else { + applyPrimitiveColumnChange(updateSchema, resolvedPath.getFullPath(), currentCol, + targetType.asPrimitiveType(), targetComment); } + applyExplicitNullableChange(updateSchema, resolvedPath.getFullPath(), column); if (position != null) { - applyPosition(updateSchema, position, column.getName()); + applyPosition(updateSchema, position, resolvedPath.getColumnPath(), icebergTable.schema(), "modify"); } try { executionAuthenticator.execute(() -> updateSchema.commit()); @@ -947,18 +1131,158 @@ public void modifyColumn(ExternalTable dorisTable, Column column, ColumnPosition refreshTable(dorisTable, updateTime); } + @Override + public void modifyColumn(ExternalTable dorisTable, ColumnPath columnPath, Column column, ColumnPosition position, + long updateTime) throws UserException { + if (!columnPath.isNested()) { + modifyTopLevelColumn(dorisTable, columnPath, column, position, updateTime); + return; + } + + Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + ResolvedColumnPath resolvedPath = resolveColumnPath(icebergTable.schema(), columnPath, "modify"); + NestedField currentCol = resolvedPath.getField(); + validateCollectionPseudoFieldComment( + icebergTable.schema(), resolvedPath, column.getComment(), column.isCommentSpecified()); + if (position != null) { + validatePositionTarget(icebergTable.schema(), resolvedPath.getColumnPath(), "modify"); + } + + validateNestedModifyColumnMetadata(column, resolvedPath.getFullPath()); + org.apache.iceberg.types.Type targetType; + if (column.getType().isComplexType()) { + validateForModifyComplexColumn(column, currentCol, columnPath.getFullPath()); + targetType = currentCol.type(); + } else { + validateForModifyColumn(column, currentCol, columnPath.getFullPath()); + targetType = resolvePrimitiveTypeForModify( + currentCol.type(), column.getType(), resolvedPath.getFullPath()); + } + + UpdateSchema updateSchema = icebergTable.updateSchema(); + String targetComment = resolveTargetComment(currentCol, column); + if (column.getType().isComplexType()) { + applyComplexTypeChange(updateSchema, resolvedPath.getFullPath(), currentCol.type(), + column.getType()); + if (!Objects.equals(currentCol.doc(), targetComment)) { + updateSchema.updateColumnDoc(resolvedPath.getFullPath(), targetComment); + } + } else { + applyPrimitiveColumnChange(updateSchema, resolvedPath.getFullPath(), currentCol, + targetType.asPrimitiveType(), targetComment); + } + applyExplicitNullableChange(updateSchema, resolvedPath.getFullPath(), column); + + if (position != null) { + applyPosition(updateSchema, position, resolvedPath.getColumnPath(), icebergTable.schema(), "modify"); + } + + try { + executionAuthenticator.execute(() -> updateSchema.commit()); + } catch (Exception e) { + throw new UserException("Failed to modify nested column: " + columnPath.getFullPath() + " in table: " + + icebergTable.name() + ", error message is: " + e.getMessage(), e); + } + refreshTable(dorisTable, updateTime); + } + + @Override + public void modifyColumnComment(ExternalTable dorisTable, ColumnPath columnPath, String comment, long updateTime) + throws UserException { + Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + if (!columnPath.isNested()) { + validateRowLineageColumnMutation(icebergTable, columnPath.getTopLevelName(), "modify comment for"); + } + ResolvedColumnPath resolvedPath = resolveColumnPath( + icebergTable.schema(), columnPath, "modify comment"); + validateCollectionPseudoFieldComment(icebergTable.schema(), resolvedPath, comment, true); + + UpdateSchema updateSchema = icebergTable.updateSchema(); + updateSchema.updateColumnDoc(resolvedPath.getFullPath(), StringUtils.defaultString(comment)); + try { + executionAuthenticator.execute(() -> updateSchema.commit()); + } catch (Exception e) { + throw new UserException("Failed to modify column comment: " + columnPath.getFullPath() + " in table: " + + icebergTable.name() + ", error message is: " + e.getMessage(), e); + } + refreshTable(dorisTable, updateTime); + } + + private void validateCollectionPseudoFieldComment(Schema schema, ResolvedColumnPath resolvedPath, + String comment, boolean commentSpecified) throws UserException { + if (!resolvedPath.getColumnPath().isNested() + || (!commentSpecified && StringUtils.isEmpty(comment))) { + return; + } + ResolvedColumnPath parentPath = resolveColumnPath( + schema, resolvedPath.getColumnPath().getParentPath(), "modify comment"); + if (parentPath.getType().isListType() || parentPath.getType().isMapType()) { + throw new UserException("Iceberg does not support comments on collection element or value fields: " + + resolvedPath.getFullPath()); + } + } + + private void applyExplicitNullableChange(UpdateSchema updateSchema, String columnPath, Column column) { + if (column.isNullableSpecified() && column.isAllowNull()) { + updateSchema.makeColumnOptional(columnPath); + } + } + + private String resolveTargetComment(NestedField currentCol, Column column) { + return column.isCommentSpecified() ? column.getComment() : currentCol.doc(); + } + + private org.apache.iceberg.types.Type.PrimitiveType resolvePrimitiveTypeForModify( + org.apache.iceberg.types.Type currentIcebergType, + org.apache.doris.catalog.Type requestedDorisType, String columnPath) throws UserException { + if (isSameMappedDorisType(mappedDorisType(currentIcebergType), requestedDorisType)) { + return currentIcebergType.asPrimitiveType(); + } + org.apache.iceberg.types.Type.PrimitiveType currentType = currentIcebergType.asPrimitiveType(); + org.apache.iceberg.types.Type.PrimitiveType targetType = + toIcebergTypeForSchemaChange(requestedDorisType, columnPath).asPrimitiveType(); + if (!currentType.equals(targetType) && !TypeUtil.isPromotionAllowed(currentType, targetType)) { + throw new UserException("Cannot change column type: " + columnPath + ": " + + currentType + " -> " + targetType); + } + return targetType; + } + + private void applyPrimitiveColumnChange(UpdateSchema updateSchema, String columnPath, + NestedField currentCol, org.apache.iceberg.types.Type.PrimitiveType targetType, + String targetComment) { + if (!currentCol.type().equals(targetType)) { + updateSchema.updateColumn(columnPath, targetType, targetComment); + } else if (!Objects.equals(currentCol.doc(), targetComment)) { + updateSchema.updateColumnDoc(columnPath, targetComment); + } + } + private void validateForModifyColumn(Column column, NestedField currentCol) throws UserException { + validateForModifyColumn(column, currentCol, column.getName()); + } + + private void validateForModifyColumn(Column column, NestedField currentCol, String columnPath) + throws UserException { // check complex type if (column.getType().isComplexType()) { throw new UserException("Modify column type to non-primitive type is not supported: " + column.getType()); } + if (!currentCol.type().isPrimitiveType()) { + throw new UserException("Modify column type from complex to primitive is not supported: " + columnPath); + } // check nullable if (currentCol.isOptional() && !column.isAllowNull()) { - throw new UserException("Can not change nullable column " + column.getName() + " to not null"); + throw new UserException("Can not change nullable column " + columnPath + " to not null"); } } private void validateForModifyComplexColumn(Column column, NestedField currentCol) throws UserException { + validateForModifyComplexColumn(column, currentCol, column.getName()); + } + + private void validateForModifyComplexColumn(Column column, NestedField currentCol, String columnPath) + throws UserException { if (!column.getType().isComplexType()) { throw new UserException("Modify column type to non-complex type is not supported: " + column.getType()); } @@ -968,7 +1292,7 @@ private void validateForModifyComplexColumn(Column column, NestedField currentCo + column.getName()); } - org.apache.doris.catalog.Type oldDorisType = IcebergUtils.icebergTypeToDorisType(oldIcebergType, false, false); + org.apache.doris.catalog.Type oldDorisType = mappedDorisType(oldIcebergType); org.apache.doris.catalog.Type newDorisType = column.getType(); if (!isSameComplexCategory(oldIcebergType, newDorisType)) { throw new UserException("Cannot change complex column type category from " @@ -979,14 +1303,169 @@ private void validateForModifyComplexColumn(Column column, NestedField currentCo } catch (DdlException e) { throw new UserException(e.getMessage(), e); } + validateComplexTypeChanges(oldIcebergType, newDorisType, columnPath); if (currentCol.isOptional() && !column.isAllowNull()) { - throw new UserException("Cannot change nullable column " + column.getName() + " to not null"); + throw new UserException("Cannot change nullable column " + columnPath + " to not null"); } if (column.getDefaultValue() != null || column.getDefaultValueExprDef() != null) { throw new UserException("Complex type default value only supports NULL"); } } + @VisibleForTesting + org.apache.iceberg.types.Type resolveNestedColumnPath(Schema schema, ColumnPath columnPath, String operation) + throws UserException { + return resolveColumnPath(schema, columnPath, operation).getType(); + } + + @VisibleForTesting + String getCanonicalColumnPath(Schema schema, ColumnPath columnPath, String operation) throws UserException { + return resolveColumnPath(schema, columnPath, operation).getFullPath(); + } + + private ResolvedColumnPath resolveColumnPath(Schema schema, ColumnPath columnPath, String operation) + throws UserException { + org.apache.iceberg.types.Type currentType = schema.asStruct(); + NestedField currentField = null; + String currentPath = ""; + List canonicalParts = new ArrayList<>(); + for (String part : columnPath.getParts()) { + if (!currentPath.isEmpty()) { + currentPath += "."; + } + currentPath += part; + + if (currentType.isStructType()) { + NestedField field = currentType.asStructType().caseInsensitiveField(part); + if (field == null) { + throw new UserException("Column path does not exist in Iceberg schema: " + + columnPath.getFullPath()); + } + canonicalParts.add(field.name()); + currentField = field; + currentType = field.type(); + } else if (currentType.isListType()) { + Types.ListType listType = currentType.asListType(); + NestedField elementField = listType.field(listType.elementId()); + if (!elementField.name().equalsIgnoreCase(part)) { + throw new UserException("Expected array element path at '" + currentPath + + "' for Iceberg column path: " + columnPath.getFullPath()); + } + canonicalParts.add(elementField.name()); + currentField = elementField; + currentType = listType.elementType(); + } else if (currentType.isMapType()) { + Types.MapType mapType = currentType.asMapType(); + NestedField keyField = mapType.field(mapType.keyId()); + if (keyField.name().equalsIgnoreCase(part)) { + throw new UserException("Cannot " + operation + " MAP key nested column: " + + columnPath.getFullPath()); + } + NestedField valueField = mapType.field(mapType.valueId()); + if (!valueField.name().equalsIgnoreCase(part)) { + throw new UserException("Expected map value path at '" + currentPath + + "' for Iceberg column path: " + columnPath.getFullPath()); + } + canonicalParts.add(valueField.name()); + currentField = valueField; + currentType = mapType.valueType(); + } else { + throw new UserException("Cannot resolve nested field under primitive column path: " + + columnPath.getFullPath()); + } + } + return new ResolvedColumnPath(ColumnPath.of(canonicalParts), currentType, currentField); + } + + @VisibleForTesting + org.apache.iceberg.types.Type validateNestedStructField(Schema schema, ColumnPath columnPath, String operation) + throws UserException { + return validateNestedStructFieldPath(schema, columnPath, operation).getType(); + } + + private ResolvedColumnPath validateNestedStructFieldPath(Schema schema, ColumnPath columnPath, String operation) + throws UserException { + ResolvedColumnPath parentPath = resolveColumnPath(schema, columnPath.getParentPath(), operation); + org.apache.iceberg.types.Type parentType = parentPath.getType(); + if (!parentType.isStructType()) { + throw new UserException("Parent column path '" + columnPath.getParentPathString() + + "' is not a struct for Iceberg nested " + operation + ": " + columnPath.getFullPath()); + } + NestedField field = parentType.asStructType().caseInsensitiveField(columnPath.getLeafName()); + if (field == null) { + throw new UserException("Column path does not exist in Iceberg schema: " + columnPath.getFullPath()); + } + return new ResolvedColumnPath(childPath(parentPath.getColumnPath(), field.name()), field.type(), field); + } + + private ColumnPath childPath(ColumnPath parentPath, String childName) { + List parts = new ArrayList<>(parentPath.getParts()); + parts.add(childName); + return ColumnPath.of(parts); + } + + @VisibleForTesting + void validateNoCaseInsensitiveSiblingCollision(Types.StructType parentType, ColumnPath parentPath, + String targetName, NestedField sourceField, String operation) throws UserException { + validateNoCaseInsensitiveSiblingCollision( + parentType, parentPath.getFullPath(), targetName, sourceField, operation); + } + + private void validateNoCaseInsensitiveSiblingCollision(Types.StructType parentType, String parentPath, + String targetName, NestedField sourceField, String operation) throws UserException { + NestedField conflictingField = parentType.caseInsensitiveField(targetName); + if (conflictingField != null + && (sourceField == null || conflictingField.fieldId() != sourceField.fieldId())) { + String targetPath = parentPath.isEmpty() ? targetName : parentPath + "." + targetName; + String conflictingPath = parentPath.isEmpty() + ? conflictingField.name() : parentPath + "." + conflictingField.name(); + String columnDescription = parentPath.isEmpty() ? "column" : "nested column"; + throw new UserException("Cannot " + operation + " " + columnDescription + " '" + targetPath + + "': conflicts with existing Iceberg field '" + conflictingPath + "' (case-insensitive)"); + } + } + + private void validateNoCaseInsensitiveTopLevelCollisions(Schema schema, List columns) + throws UserException { + Set requestedNames = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + for (Column column : columns) { + validateNoCaseInsensitiveSiblingCollision( + schema.asStruct(), "", column.getName(), null, "add"); + if (!requestedNames.add(column.getName())) { + throw new UserException("Cannot add column '" + column.getName() + + "': conflicts with another requested column (case-insensitive)"); + } + } + } + + private static class ResolvedColumnPath { + private final ColumnPath columnPath; + private final org.apache.iceberg.types.Type type; + private final NestedField field; + + private ResolvedColumnPath(ColumnPath columnPath, org.apache.iceberg.types.Type type, NestedField field) { + this.columnPath = columnPath; + this.type = type; + this.field = field; + } + + private ColumnPath getColumnPath() { + return columnPath; + } + + private String getFullPath() { + return columnPath.getFullPath(); + } + + private org.apache.iceberg.types.Type getType() { + return type; + } + + private NestedField getField() { + return field; + } + } + private boolean isSameComplexCategory(Type oldIcebergType, org.apache.doris.catalog.Type newDorisType) { switch (oldIcebergType.typeId()) { case STRUCT: @@ -1000,6 +1479,59 @@ private boolean isSameComplexCategory(Type oldIcebergType, org.apache.doris.cata } } + private void validateComplexTypeChanges(org.apache.iceberg.types.Type oldIcebergType, + org.apache.doris.catalog.Type newDorisType, String path) throws UserException { + switch (oldIcebergType.typeId()) { + case STRUCT: + validateStructTypeChanges(oldIcebergType.asStructType(), (StructType) newDorisType, path); + break; + case LIST: + Types.ListType oldListType = oldIcebergType.asListType(); + validateExistingTypeChange(oldListType.elementType(), ((ArrayType) newDorisType).getItemType(), + path + "." + oldListType.field(oldListType.elementId()).name()); + break; + case MAP: + Types.MapType oldMapType = oldIcebergType.asMapType(); + MapType newMapType = (MapType) newDorisType; + org.apache.doris.catalog.Type oldDorisKeyType = mappedDorisType(oldMapType.keyType()); + if (!isSameMappedDorisType(oldDorisKeyType, newMapType.getKeyType())) { + throw new UserException("Cannot change MAP key type from " + + oldDorisKeyType.toSql() + " to " + newMapType.getKeyType().toSql()); + } + validateExistingTypeChange(oldMapType.valueType(), newMapType.getValueType(), + path + "." + oldMapType.field(oldMapType.valueId()).name()); + break; + default: + throw new UserException("Unsupported complex type for modify: " + oldIcebergType); + } + } + + private void validateStructTypeChanges(Types.StructType oldStructType, + StructType newStructType, String path) throws UserException { + List oldFields = oldStructType.fields(); + List newFields = newStructType.getFields(); + for (int i = 0; i < oldFields.size(); i++) { + NestedField oldField = oldFields.get(i); + validateExistingTypeChange(oldField.type(), newFields.get(i).getType(), + path + "." + oldField.name()); + } + for (int i = oldFields.size(); i < newFields.size(); i++) { + StructField newField = newFields.get(i); + toIcebergTypeForSchemaChange(newField.getType(), path + "." + newField.getName()); + } + } + + private void validateExistingTypeChange(org.apache.iceberg.types.Type oldIcebergType, + org.apache.doris.catalog.Type newDorisType, String path) throws UserException { + if (oldIcebergType.isPrimitiveType()) { + if (!isSameMappedDorisType(mappedDorisType(oldIcebergType), newDorisType)) { + toIcebergTypeForSchemaChange(newDorisType, path); + } + return; + } + validateComplexTypeChanges(oldIcebergType, newDorisType, path); + } + private void applyComplexTypeChange(UpdateSchema updateSchema, String path, org.apache.iceberg.types.Type oldIcebergType, org.apache.doris.catalog.Type newDorisType) throws UserException { @@ -1019,7 +1551,8 @@ private void applyComplexTypeChange(UpdateSchema updateSchema, String path, } private void applyStructChange(UpdateSchema updateSchema, String path, - Types.StructType oldStructType, StructType newStructType) throws UserException { + Types.StructType oldStructType, StructType newStructType) + throws UserException { List oldFields = oldStructType.fields(); List newFields = newStructType.getFields(); @@ -1034,27 +1567,26 @@ private void applyStructChange(UpdateSchema updateSchema, String path, org.apache.iceberg.types.Type oldFieldType = oldField.type(); org.apache.doris.catalog.Type newFieldType = newField.getType(); + String targetComment = newField.isCommentSpecified() + ? newField.getComment() : oldField.doc(); if (oldFieldType.isPrimitiveType()) { - org.apache.doris.catalog.Type oldDorisFieldType = - IcebergUtils.icebergTypeToDorisType(oldFieldType, false, false); - boolean typeChanged = !oldDorisFieldType.equals(newFieldType); - boolean commentChanged = !Objects.equals(oldField.doc(), newField.getComment()); - if (typeChanged || commentChanged) { + org.apache.doris.catalog.Type oldDorisFieldType = mappedDorisType(oldFieldType); + boolean typeChanged = !isSameMappedDorisType(oldDorisFieldType, newFieldType); + boolean commentChanged = !Objects.equals(oldField.doc(), targetComment); + if (typeChanged) { org.apache.iceberg.types.Type newIcebergFieldType = - IcebergUtils.dorisTypeToIcebergType(newFieldType); + toIcebergTypeForSchemaChange(newFieldType, fieldPath); updateSchema.updateColumn(fieldPath, newIcebergFieldType.asPrimitiveType(), - newField.getComment()); + targetComment); + } else if (commentChanged) { + updateSchema.updateColumnDoc(fieldPath, targetComment); } } else { applyComplexTypeChange(updateSchema, fieldPath, oldFieldType, newFieldType); - if (!Objects.equals(oldField.doc(), newField.getComment())) { - updateSchema.updateColumnDoc(fieldPath, newField.getComment()); + if (!Objects.equals(oldField.doc(), targetComment)) { + updateSchema.updateColumnDoc(fieldPath, targetComment); } } - - if (!oldField.isOptional() && newField.getContainsNull()) { - updateSchema.makeColumnOptional(fieldPath); - } } for (int i = oldFields.size(); i < newFields.size(); i++) { @@ -1063,13 +1595,14 @@ private void applyStructChange(UpdateSchema updateSchema, String path, throw new UserException("New struct field '" + newField.getName() + "' must be nullable"); } org.apache.iceberg.types.Type newFieldIcebergType = - IcebergUtils.dorisTypeToIcebergType(newField.getType()); + toIcebergTypeForSchemaChange(newField.getType(), path + "." + newField.getName()); updateSchema.addColumn(path, newField.getName(), newFieldIcebergType, newField.getComment()); } } private void applyListChange(UpdateSchema updateSchema, String path, - Types.ListType oldListType, ArrayType newArrayType) throws UserException { + Types.ListType oldListType, ArrayType newArrayType) + throws UserException { String elementPath = path + "." + oldListType.field(oldListType.elementId()).name(); if (oldListType.isElementOptional() && !newArrayType.getContainsNull()) { throw new UserException("Cannot change nullable column " + elementPath + " to not null"); @@ -1077,28 +1610,24 @@ private void applyListChange(UpdateSchema updateSchema, String path, org.apache.iceberg.types.Type oldElementType = oldListType.elementType(); org.apache.doris.catalog.Type newElementType = newArrayType.getItemType(); if (oldElementType.isPrimitiveType()) { - org.apache.doris.catalog.Type oldDorisElementType = - IcebergUtils.icebergTypeToDorisType(oldElementType, false, false); - if (!oldDorisElementType.equals(newElementType)) { + org.apache.doris.catalog.Type oldDorisElementType = mappedDorisType(oldElementType); + if (!isSameMappedDorisType(oldDorisElementType, newElementType)) { org.apache.iceberg.types.Type newIcebergElementType = - IcebergUtils.dorisTypeToIcebergType(newElementType); + toIcebergTypeForSchemaChange(newElementType, elementPath); updateSchema.updateColumn(elementPath, newIcebergElementType.asPrimitiveType(), null); } } else { applyComplexTypeChange(updateSchema, elementPath, oldElementType, newElementType); } - if (!oldListType.isElementOptional() && newArrayType.getContainsNull()) { - updateSchema.makeColumnOptional(elementPath); - } } private void applyMapChange(UpdateSchema updateSchema, String path, - Types.MapType oldMapType, MapType newMapType) throws UserException { + Types.MapType oldMapType, MapType newMapType) + throws UserException { org.apache.iceberg.types.Type oldKeyType = oldMapType.keyType(); org.apache.doris.catalog.Type newKeyType = newMapType.getKeyType(); - org.apache.doris.catalog.Type oldDorisKeyType = - IcebergUtils.icebergTypeToDorisType(oldKeyType, false, false); - if (!oldDorisKeyType.equals(newKeyType)) { + org.apache.doris.catalog.Type oldDorisKeyType = mappedDorisType(oldKeyType); + if (!isSameMappedDorisType(oldDorisKeyType, newKeyType)) { throw new UserException("Cannot change MAP key type from " + oldDorisKeyType.toSql() + " to " + newKeyType.toSql()); } @@ -1110,22 +1639,29 @@ private void applyMapChange(UpdateSchema updateSchema, String path, org.apache.iceberg.types.Type oldValueType = oldMapType.valueType(); org.apache.doris.catalog.Type newValueType = newMapType.getValueType(); if (oldValueType.isPrimitiveType()) { - org.apache.doris.catalog.Type oldDorisValueType = - IcebergUtils.icebergTypeToDorisType(oldValueType, false, false); - if (!oldDorisValueType.equals(newValueType)) { + org.apache.doris.catalog.Type oldDorisValueType = mappedDorisType(oldValueType); + if (!isSameMappedDorisType(oldDorisValueType, newValueType)) { org.apache.iceberg.types.Type newIcebergValueType = - IcebergUtils.dorisTypeToIcebergType(newValueType); + toIcebergTypeForSchemaChange(newValueType, valuePath); updateSchema.updateColumn(valuePath, newIcebergValueType.asPrimitiveType(), null); } } else { applyComplexTypeChange(updateSchema, valuePath, oldValueType, newValueType); } - if (!oldMapType.isValueOptional() && newMapType.getIsValueContainsNull()) { - updateSchema.makeColumnOptional(valuePath); - } } - private void validateCommonColumnInfo(Column column) throws UserException { + private void validateCommonColumnInfo(Column column, boolean rejectKey) throws UserException { + validateCommonColumnMetadata(column, rejectKey); + toIcebergTypeForSchemaChange(column.getType(), column.getName()); + } + + private void validateCommonColumnMetadata(Column column, boolean rejectKey) throws UserException { + if (rejectKey && column.isKey()) { + throw new UserException("KEY is not supported for Iceberg ADD/MODIFY COLUMN"); + } + if (column.isGeneratedColumn()) { + throw new UserException("Generated columns are not supported for Iceberg ADD/MODIFY COLUMN"); + } // check aggregation method if (column.isAggregated()) { throw new UserException("Can not specify aggregation method for iceberg table column"); @@ -1136,16 +1672,87 @@ private void validateCommonColumnInfo(Column column) throws UserException { } } + private org.apache.doris.catalog.Type mappedDorisType(org.apache.iceberg.types.Type icebergType) { + return IcebergUtils.icebergTypeToDorisType(icebergType, + dorisCatalog.getEnableMappingVarbinary(), dorisCatalog.getEnableMappingTimestampTz()); + } + + private boolean isSameMappedDorisType(org.apache.doris.catalog.Type mappedType, + org.apache.doris.catalog.Type requestedType) { + // ScalarType.equals does not compare VARBINARY length, but Iceberg FIXED uses the + // mapped length to distinguish an unchanged view from a requested type change. + return mappedType.equals(requestedType) + && (!mappedType.isVarbinaryType() || mappedType.getLength() == requestedType.getLength()); + } + + private org.apache.iceberg.types.Type toIcebergTypeForSchemaChange( + org.apache.doris.catalog.Type dorisType, String columnPath) throws UserException { + try { + return IcebergUtils.dorisTypeToIcebergType(dorisType); + } catch (UnsupportedOperationException | IllegalArgumentException e) { + throw new UserException("Type " + dorisType.toSql() + + " is not supported for Iceberg column " + columnPath, e); + } + } + + private void validateNestedAddColumnMetadata(Column column, ColumnPath columnPath) throws UserException { + validateCommonColumnInfo(column, true); + if (column.hasDefaultValue() || column.hasOnUpdateDefaultValue()) { + throw new UserException("DEFAULT and ON UPDATE are not supported for Iceberg nested ADD COLUMN: " + + columnPath.getFullPath()); + } + } + + private void validateNestedModifyColumnMetadata(Column column, String columnPath) throws UserException { + validateModifyColumnMetadata(column, columnPath, true); + } + + private void validateAddColumnMetadata(Column column, boolean rejectKey) throws UserException { + validateCommonColumnInfo(column, rejectKey); + if (column.hasOnUpdateDefaultValue()) { + throw new UserException("ON UPDATE is not supported for Iceberg ADD COLUMN: " + column.getName()); + } + } + + private void validateModifyColumnMetadata(Column column, String columnPath, boolean rejectKey) + throws UserException { + validateCommonColumnMetadata(column, rejectKey); + if (column.hasDefaultValue() || column.hasOnUpdateDefaultValue()) { + throw new UserException("Modifying default values is not supported for Iceberg columns: " + columnPath); + } + } + + private void validateRowLineageColumnMutation(Table icebergTable, String columnName, String operation) + throws UserException { + int formatVersion = IcebergUtils.getFormatVersion(icebergTable); + if (formatVersion >= IcebergUtils.ICEBERG_ROW_LINEAGE_MIN_VERSION + && IcebergUtils.isIcebergRowLineageColumn(columnName)) { + throw new UserException("Cannot " + operation + " Iceberg v" + formatVersion + + " reserved row lineage column: " + columnName); + } + } + @Override public void reorderColumns(ExternalTable dorisTable, List newOrder, long updateTime) throws UserException { if (newOrder == null || newOrder.isEmpty()) { throw new UserException("Reorder column failed, new order is empty."); } Table icebergTable = IcebergUtils.getIcebergTable(dorisTable); + List canonicalOrder = new ArrayList<>(newOrder.size()); + Set canonicalNames = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); + for (String columnName : newOrder) { + validateRowLineageColumnMutation(icebergTable, columnName, "reorder"); + String canonicalName = resolveColumnPath( + icebergTable.schema(), ColumnPath.of(columnName), "reorder").getFullPath(); + if (!canonicalNames.add(canonicalName)) { + throw new UserException("Duplicate column in reorder columns: " + columnName); + } + canonicalOrder.add(canonicalName); + } UpdateSchema updateSchema = icebergTable.updateSchema(); - updateSchema.moveFirst(newOrder.get(0)); - for (int i = 1; i < newOrder.size(); i++) { - updateSchema.moveAfter(newOrder.get(i), newOrder.get(i - 1)); + updateSchema.moveFirst(canonicalOrder.get(0)); + for (int i = 1; i < canonicalOrder.size(); i++) { + updateSchema.moveAfter(canonicalOrder.get(i), canonicalOrder.get(i - 1)); } try { executionAuthenticator.execute(() -> updateSchema.commit()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/operations/ExternalMetadataOps.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/operations/ExternalMetadataOps.java index b7f6331306861c..f513eb10fca7a2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/operations/ExternalMetadataOps.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/operations/ExternalMetadataOps.java @@ -17,6 +17,7 @@ package org.apache.doris.datasource.operations; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.info.ColumnPosition; import org.apache.doris.catalog.info.CreateOrReplaceBranchInfo; @@ -226,6 +227,15 @@ default void addColumn(ExternalTable dorisTable, Column column, ColumnPosition p throw new UnsupportedOperationException("Add column operation is not supported for this table type."); } + default void addColumn(ExternalTable dorisTable, ColumnPath columnPath, Column column, ColumnPosition position, + long updateTime) throws UserException { + if (!columnPath.isNested()) { + addColumn(dorisTable, column, position, updateTime); + return; + } + throw new UnsupportedOperationException("Nested add column operation is not supported for this table type."); + } + /** * add columns for external table * @@ -250,6 +260,15 @@ default void dropColumn(ExternalTable dorisTable, String columnName, long update throw new UnsupportedOperationException("Drop column operation is not supported for this table type."); } + default void dropColumn(ExternalTable dorisTable, ColumnPath columnPath, long updateTime) + throws UserException { + if (!columnPath.isNested()) { + dropColumn(dorisTable, columnPath.getTopLevelName(), updateTime); + return; + } + throw new UnsupportedOperationException("Nested drop column operation is not supported for this table type."); + } + /** * rename column for external table * @@ -263,6 +282,15 @@ default void renameColumn(ExternalTable dorisTable, String oldName, String newNa throw new UnsupportedOperationException("Rename column operation is not supported for this table type."); } + default void renameColumn(ExternalTable dorisTable, ColumnPath columnPath, String newName, long updateTime) + throws UserException { + if (!columnPath.isNested()) { + renameColumn(dorisTable, columnPath.getTopLevelName(), newName, updateTime); + return; + } + throw new UnsupportedOperationException("Nested rename column operation is not supported for this table type."); + } + /** * update column for external table * @@ -276,6 +304,30 @@ default void modifyColumn(ExternalTable dorisTable, Column column, ColumnPositio throw new UnsupportedOperationException("Modify column operation is not supported for this table type."); } + default void modifyColumn(ExternalTable dorisTable, ColumnPath columnPath, Column column, ColumnPosition position, + long updateTime) throws UserException { + if (!columnPath.isNested()) { + modifyColumn(dorisTable, column, position, updateTime); + return; + } + throw new UnsupportedOperationException("Nested modify column operation is not supported for this table type."); + } + + /** + * modify column comment for external table + * + * @param dorisTable + * @param columnPath + * @param comment + * @param updateTime + * @throws UserException + */ + default void modifyColumnComment(ExternalTable dorisTable, ColumnPath columnPath, String comment, long updateTime) + throws UserException { + throw new UnsupportedOperationException( + "Modify column comment operation is not supported for this table type."); + } + /** * reorder columns for external table * diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java index c470bc789d9e78..42695f85a8d34f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java @@ -22,6 +22,7 @@ import org.apache.doris.analysis.ArithmeticExpr.Operator; import org.apache.doris.analysis.BrokerDesc; import org.apache.doris.analysis.ColumnNullableType; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.analysis.DbName; import org.apache.doris.analysis.EncryptKeyName; import org.apache.doris.analysis.PassVar; @@ -140,6 +141,7 @@ import org.apache.doris.nereids.DorisParser.CleanLabelContext; import org.apache.doris.nereids.DorisParser.CollateContext; import org.apache.doris.nereids.DorisParser.ColumnDefContext; +import org.apache.doris.nereids.DorisParser.ColumnDefWithPathContext; import org.apache.doris.nereids.DorisParser.ColumnDefsContext; import org.apache.doris.nereids.DorisParser.ColumnReferenceContext; import org.apache.doris.nereids.DorisParser.CommentRelationHintContext; @@ -1094,6 +1096,7 @@ import org.apache.doris.nereids.types.coercion.CharacterType; import org.apache.doris.nereids.util.ExpressionUtils; import org.apache.doris.nereids.util.RelationUtil; +import org.apache.doris.nereids.util.SqlLiteralUtils; import org.apache.doris.nereids.util.Utils; import org.apache.doris.policy.FilterType; import org.apache.doris.policy.PolicyTypeEnum; @@ -1154,6 +1157,16 @@ public class LogicalPlanBuilder extends DorisParserBaseVisitor { private static String DEFAULT_NESTED_COLUMN_NAME = "unnest"; private static String DEFAULT_ORDINALITY_COLUMN_NAME = "ordinality"; + private static class ColumnDefinitionWithPath { + private final ColumnDefinition columnDefinition; + private final ColumnPath columnPath; + + private ColumnDefinitionWithPath(ColumnDefinition columnDefinition, ColumnPath columnPath) { + this.columnDefinition = columnDefinition; + this.columnPath = columnPath; + } + } + // Sort the parameters with token position to keep the order with original placeholders // in prepared statement.Otherwise, the order maybe broken private final Map tokenPosToParameters = Maps.newTreeMap((pos1, pos2) -> { @@ -1174,6 +1187,18 @@ public LogicalPlanBuilder(Map selectHintMap) { this.selectHintMap = selectHintMap; } + private static String requireNonEmptyColumnIdentifier(ParserRuleContext ctx, String identifier) { + if (identifier.isEmpty()) { + throw new ParseException("Quoted identifier cannot be empty", ctx); + } + return identifier; + } + + private static ColumnPath parseColumnPath(ParserRuleContext ctx, List parts) { + parts.forEach(part -> requireNonEmptyColumnIdentifier(ctx, part)); + return ColumnPath.of(parts); + } + @SuppressWarnings("unchecked") protected T typedVisit(ParseTree ctx) { return (T) ctx.accept(this); @@ -3728,18 +3753,7 @@ public Literal visitIntegerLiteral(IntegerLiteralContext ctx) { @Override public Literal visitStringLiteral(StringLiteralContext ctx) { - String txt = ctx.STRING_LITERAL().getText(); - String s = txt.substring(1, txt.length() - 1); - if (txt.charAt(0) == '\'') { - // for single quote string, '' should be converted to ' - s = s.replace("''", "'"); - } else if (txt.charAt(0) == '"') { - // for double quote string, "" should be converted to " - s = s.replace("\"\"", "\""); - } - if (!SqlModeHelper.hasNoBackSlashEscapes()) { - s = LogicalPlanBuilderAssistant.escapeBackSlash(s); - } + String s = SqlLiteralUtils.parseStringLiteral(ctx.STRING_LITERAL().getText()); int strLength = Utils.containChinese(s) ? s.length() * StringLikeLiteral.CHINESE_CHAR_BYTE_LENGTH : s.length(); if (strLength > ScalarType.MAX_VARCHAR_LENGTH) { return new StringLiteral(s); @@ -3906,6 +3920,13 @@ public List visitIdentifierSeq(IdentifierSeqContext ctx) { .collect(ImmutableList.toImmutableList()); } + @Override + public List visitQualifiedName(QualifiedNameContext ctx) { + return ctx.identifier().stream() + .map(RuleContext::getText) + .collect(ImmutableList.toImmutableList()); + } + @Override public EqualTo visitUpdateAssignment(UpdateAssignmentContext ctx) { return new EqualTo(new UnboundSlot( @@ -4211,9 +4232,8 @@ public ColumnDefinition visitColumnDef(ColumnDefContext ctx) { e.getCause()); } } - //comment should remove '\' and '(") at the beginning and end - String comment = ctx.comment != null ? ctx.comment.getText().substring(1, ctx.comment.getText().length() - 1) - .replace("\\", "") : ""; + String comment = ctx.comment != null + ? SqlLiteralUtils.parseStringLiteral(ctx.comment.getText()) : ""; long autoIncInitValue = -1; if (ctx.AUTO_INCREMENT() != null) { if (ctx.autoIncInitValue != null) { @@ -4231,7 +4251,65 @@ public ColumnDefinition visitColumnDef(ColumnDefContext ctx) { ? Optional.of(new GeneratedColumnDesc(ctx.generatedExpr.getText(), getExpression(ctx.generatedExpr))) : Optional.empty(); return new ColumnDefinition(colName, colType, isKey, aggType, nullableType, autoIncInitValue, defaultValue, - onUpdateDefaultValue, comment, desc); + onUpdateDefaultValue, comment, ctx.comment != null, true, desc); + } + + @Override + public ColumnDefinitionWithPath visitColumnDefWithPath(ColumnDefWithPathContext ctx) { + if (ctx.columnDef() != null) { + ColumnDefinition columnDefinition = visitColumnDef(ctx.columnDef()); + ColumnPath columnPath = parseColumnPath(ctx, Collections.singletonList(columnDefinition.getName())); + return new ColumnDefinitionWithPath(columnDefinition, columnPath); + } + + ColumnPath columnPath = parseColumnPath(ctx, ctx.colNames.stream() + .map(RuleContext::getText) + .collect(Collectors.toList())); + 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; + AggregateType aggType = null; + if (aggTypeString != null) { + try { + aggType = AggregateType.valueOf(aggTypeString.toUpperCase()); + } catch (Exception e) { + throw new AnalysisException(String.format("Aggregate type %s is unsupported", aggTypeString), + e.getCause()); + } + } + String comment = ctx.comment != null + ? SqlLiteralUtils.parseStringLiteral(ctx.comment.getText()) : ""; + long autoIncInitValue = -1; + if (ctx.AUTO_INCREMENT() != null) { + if (ctx.autoIncInitValue != null) { + autoIncInitValue = Long.valueOf(ctx.autoIncInitValue.getText()); + if (autoIncInitValue < 0) { + throw new AnalysisException("AUTO_INCREMENT start value can not be negative."); + } + } else { + autoIncInitValue = Long.valueOf(1); + } + } + Optional desc = ctx.generatedExpr != null + ? Optional.of(new GeneratedColumnDesc(ctx.generatedExpr.getText(), getExpression(ctx.generatedExpr))) + : Optional.empty(); + ColumnDefinition columnDefinition = new ColumnDefinition(colName, colType, isKey, aggType, nullableType, + autoIncInitValue, Optional.empty(), Optional.empty(), comment, ctx.comment != null, true, desc); + return new ColumnDefinitionWithPath(columnDefinition, columnPath); } @Override @@ -5417,14 +5495,11 @@ public List visitComplexColTypeList(ComplexColTypeListContext ctx) @Override public StructField visitComplexColType(ComplexColTypeContext ctx) { - String comment; - if (ctx.commentSpec() != null) { - comment = ctx.commentSpec().STRING_LITERAL().getText(); - comment = LogicalPlanBuilderAssistant.escapeBackSlash(comment.substring(1, comment.length() - 1)); - } else { - comment = ""; - } - return new StructField(ctx.identifier().getText(), typedVisit(ctx.dataType()), true, comment); + String comment = ctx.commentSpec() == null ? "" + : SqlLiteralUtils.parseStringLiteral( + ctx.commentSpec().STRING_LITERAL().getText()); + return new StructField(ctx.identifier().getText(), typedVisit(ctx.dataType()), true, + comment, ctx.commentSpec() != null); } private String parseConstant(ConstantContext context) { @@ -6105,7 +6180,7 @@ public AlterTableCommand visitAlterTableProperties(DorisParser.AlterTablePropert @Override public AlterTableOp visitAddColumnClause(AddColumnClauseContext ctx) { - ColumnDefinition columnDefinition = visitColumnDef(ctx.columnDef()); + ColumnDefinitionWithPath columnDefinitionWithPath = visitColumnDefWithPath(ctx.columnDefWithPath()); ColumnPosition columnPosition = null; if (ctx.columnPosition() != null) { if (ctx.columnPosition().FIRST() != null) { @@ -6118,7 +6193,8 @@ public AlterTableOp visitAddColumnClause(AddColumnClauseContext ctx) { Map properties = ctx.properties != null ? Maps.newHashMap(visitPropertyClause(ctx.properties)) : Maps.newHashMap(); - return new AddColumnOp(columnDefinition, columnPosition, rollupName, properties); + return new AddColumnOp(columnDefinitionWithPath.columnDefinition, columnDefinitionWithPath.columnPath, + columnPosition, rollupName, properties); } @Override @@ -6133,17 +6209,17 @@ public AlterTableOp visitAddColumnsClause(AddColumnsClauseContext ctx) { @Override public AlterTableOp visitDropColumnClause(DropColumnClauseContext ctx) { - String columnName = ctx.name.getText(); + ColumnPath columnPath = parseColumnPath(ctx.name, visitQualifiedName(ctx.name)); String rollupName = ctx.fromRollup() != null ? ctx.fromRollup().rollup.getText() : null; Map properties = ctx.properties != null ? Maps.newHashMap(visitPropertyClause(ctx.properties)) : Maps.newHashMap(); - return new DropColumnOp(columnName, rollupName, properties); + return new DropColumnOp(columnPath, rollupName, properties); } @Override public AlterTableOp visitModifyColumnClause(ModifyColumnClauseContext ctx) { - ColumnDefinition columnDefinition = visitColumnDef(ctx.columnDef()); + ColumnDefinitionWithPath columnDefinitionWithPath = visitColumnDefWithPath(ctx.columnDefWithPath()); ColumnPosition columnPosition = null; if (ctx.columnPosition() != null) { if (ctx.columnPosition().FIRST() != null) { @@ -6156,12 +6232,14 @@ public AlterTableOp visitModifyColumnClause(ModifyColumnClauseContext ctx) { Map properties = ctx.properties != null ? Maps.newHashMap(visitPropertyClause(ctx.properties)) : Maps.newHashMap(); - return new ModifyColumnOp(columnDefinition, columnPosition, rollupName, properties); + return new ModifyColumnOp(columnDefinitionWithPath.columnDefinition, columnDefinitionWithPath.columnPath, + columnPosition, rollupName, properties); } @Override public AlterTableOp visitReorderColumnsClause(ReorderColumnsClauseContext ctx) { List columnsByPos = visitIdentifierList(ctx.identifierList()); + columnsByPos.forEach(column -> requireNonEmptyColumnIdentifier(ctx.identifierList(), column)); String rollupName = ctx.fromRollup() != null ? ctx.fromRollup().rollup.getText() : null; Map properties = ctx.properties != null ? Maps.newHashMap(visitPropertyClause(ctx.properties)) @@ -6359,7 +6437,7 @@ public AlterTableOp visitRenamePartitionClause(RenamePartitionClauseContext ctx) @Override public AlterTableOp visitRenameColumnClause(RenameColumnClauseContext ctx) { - return new RenameColumnOp(ctx.name.getText(), ctx.newName.getText()); + return new RenameColumnOp(parseColumnPath(ctx.name, visitQualifiedName(ctx.name)), ctx.newName.getText()); } @Override @@ -6467,9 +6545,9 @@ public AlterTableOp visitModifyTableCommentClause(ModifyTableCommentClauseContex @Override public AlterTableOp visitModifyColumnCommentClause(ModifyColumnCommentClauseContext ctx) { - String columnName = ctx.name.getText(); - String comment = stripQuotes(ctx.STRING_LITERAL().getText()); - return new ModifyColumnCommentOp(columnName, comment); + ColumnPath columnPath = parseColumnPath(ctx.name, visitQualifiedName(ctx.name)); + String comment = SqlLiteralUtils.parseStringLiteral(ctx.STRING_LITERAL().getText()); + return new ModifyColumnCommentOp(columnPath, comment); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderAssistant.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderAssistant.java index 7806201a75dbbd..63a8e9557b1144 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderAssistant.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilderAssistant.java @@ -19,6 +19,7 @@ import org.apache.doris.nereids.trees.plans.logical.LogicalCheckPolicy; import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; +import org.apache.doris.nereids.util.SqlLiteralUtils; import com.google.common.collect.ImmutableSet; @@ -42,45 +43,7 @@ private LogicalPlanBuilderAssistant() { * EscapeBackSlash such \n, \t */ public static String escapeBackSlash(String str) { - StringBuilder sb = new StringBuilder(); - int strLen = str.length(); - for (int i = 0; i < strLen; ++i) { - char c = str.charAt(i); - if (c == '\\' && (i + 1) < strLen) { - switch (str.charAt(i + 1)) { - case 'n': - sb.append('\n'); - break; - case 't': - sb.append('\t'); - break; - case 'r': - sb.append('\r'); - break; - case 'b': - sb.append('\b'); - break; - case '0': - sb.append('\0'); // Ascii null - break; - case 'Z': // ^Z must be escaped on Win32 - sb.append('\032'); - break; - case '_': - case '%': - sb.append('\\'); // remember prefix for wildcard - sb.append(str.charAt(i + 1)); - break; - default: - sb.append(str.charAt(i + 1)); - break; - } - i++; - } else { - sb.append(c); - } - } - return sb.toString(); + return SqlLiteralUtils.unescapeBackSlash(str); } /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java index 83f978fff948b9..c7a7bed074e9f7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/NereidsParser.java @@ -59,6 +59,7 @@ import java.util.BitSet; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -300,13 +301,14 @@ public List> parseMultiple(String sql, } public Expression parseExpression(String expression) { - if (isSimpleIdentifier(expression)) { + if (isValidUnquotedIdentifier(expression)) { return new UnboundSlot(expression); } return parse(expression, DorisParser::expressionWithEof); } - private static boolean isSimpleIdentifier(String expression) { + /** Return whether the text can be emitted as an unquoted Nereids identifier. */ + public static boolean isValidUnquotedIdentifier(String expression) { if (expression == null || expression.isEmpty()) { return false; } @@ -323,7 +325,7 @@ private static boolean isSimpleIdentifier(String expression) { if (!hasLetter) { return false; } - String upperCase = expression.toUpperCase(); + String upperCase = expression.toUpperCase(Locale.ROOT); return (NON_RESERVED_KEYWORDS.contains(upperCase) || !LITERAL_TOKENS.containsKey(upperCase)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommand.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommand.java index 86b085740e2925..91fb9bf9739019 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommand.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommand.java @@ -17,6 +17,7 @@ package org.apache.doris.nereids.trees.plans.commands; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.analysis.StmtType; import org.apache.doris.catalog.AggregateType; import org.apache.doris.catalog.Column; @@ -36,6 +37,7 @@ import org.apache.doris.common.UserException; import org.apache.doris.common.util.InternalDatabaseUtil; import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.datasource.iceberg.IcebergExternalTable; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.nereids.trees.plans.commands.info.AddColumnOp; @@ -52,6 +54,7 @@ import org.apache.doris.nereids.trees.plans.commands.info.DropRollupOp; import org.apache.doris.nereids.trees.plans.commands.info.DropTagOp; import org.apache.doris.nereids.trees.plans.commands.info.EnableFeatureOp; +import org.apache.doris.nereids.trees.plans.commands.info.ModifyColumnCommentOp; import org.apache.doris.nereids.trees.plans.commands.info.ModifyColumnOp; import org.apache.doris.nereids.trees.plans.commands.info.ModifyEngineOp; import org.apache.doris.nereids.trees.plans.commands.info.ModifyTablePropertiesOp; @@ -115,10 +118,6 @@ private void validate(ConnectContext ctx) throws UserException { if (ops == null || ops.isEmpty()) { ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_ALTER_OPERATION); } - for (AlterTableOp op : ops) { - op.setTableName(tbl); - op.validate(ctx); - } String ctlName = tbl.getCtl(); String dbName = tbl.getDb(); String tableName = tbl.getTbl(); @@ -129,6 +128,11 @@ private void validate(ConnectContext ctx) throws UserException { if (tableIf.isTemporary()) { throw new AnalysisException("Do not support alter temporary table[" + tableName + "]"); } + checkColumnOperationsSupported(tableIf, ops); + for (AlterTableOp op : ops) { + op.setTableName(tbl); + op.validate(ctx); + } if (tableIf instanceof OlapTable) { rewriteAlterOpForOlapTable(ctx, (OlapTable) tableIf); } else { @@ -136,6 +140,140 @@ private void validate(ConnectContext ctx) throws UserException { } } + static void checkColumnOperationsSupported(TableIf table, List alterTableOps) + throws AnalysisException { + if (table instanceof IcebergExternalTable) { + checkIcebergCompoundColumnOperations(alterTableOps); + for (AlterTableOp alterTableOp : alterTableOps) { + ColumnDefinition columnDefinition = getColumnDefinition(alterTableOp); + ColumnPath nestedColumnPath = getNestedColumnPath(alterTableOp); + // Keep this before AddColumnOp.validate(), whose generic NOT NULL check would mask + // the Iceberg nested-field invariant. Metadata validation retains the same guard for other callers. + if (alterTableOp instanceof AddColumnOp && columnDefinition != null && nestedColumnPath != null + && !columnDefinition.isNullable()) { + throw new AnalysisException("New nested field '" + nestedColumnPath.getFullPath() + + "' must be nullable"); + } + if (!isIcebergColumnSchemaOperation(alterTableOp)) { + continue; + } + if (getRollupName(alterTableOp) != null) { + throw new AnalysisException("Rollup is not supported for Iceberg column operations"); + } + Map properties = alterTableOp.getProperties(); + if (properties != null && !properties.isEmpty()) { + throw new AnalysisException("PROPERTIES are not supported for Iceberg column operations"); + } + checkIcebergColumnDefinition(alterTableOp, columnDefinition); + if (alterTableOp instanceof AddColumnsOp) { + for (ColumnDefinition definition : ((AddColumnsOp) alterTableOp).getColumnDefinitions()) { + checkIcebergColumnDefinition(alterTableOp, definition); + } + } + } + return; + } + for (AlterTableOp alterTableOp : alterTableOps) { + ColumnPath columnPath = getNestedColumnPath(alterTableOp); + if (columnPath != null) { + throw new AnalysisException("Nested column path is only supported for Iceberg tables: " + + columnPath.getFullPath()); + } + } + } + + private static void checkIcebergCompoundColumnOperations(List alterTableOps) + throws AnalysisException { + if (alterTableOps.size() <= 1) { + return; + } + for (AlterTableOp alterTableOp : alterTableOps) { + if (isIcebergColumnSchemaOperation(alterTableOp)) { + throw new AnalysisException("Multiple Iceberg ALTER clauses are not supported when a statement " + + "contains a column operation"); + } + } + } + + private static ColumnPath getNestedColumnPath(AlterTableOp alterTableOp) { + ColumnPath columnPath = null; + if (alterTableOp instanceof AddColumnOp) { + columnPath = ((AddColumnOp) alterTableOp).getColumnPath(); + } else if (alterTableOp instanceof DropColumnOp) { + columnPath = ((DropColumnOp) alterTableOp).getColumnPath(); + } else if (alterTableOp instanceof RenameColumnOp) { + columnPath = ((RenameColumnOp) alterTableOp).getColumnPath(); + } else if (alterTableOp instanceof ModifyColumnOp) { + columnPath = ((ModifyColumnOp) alterTableOp).getColumnPath(); + } else if (alterTableOp instanceof ModifyColumnCommentOp) { + columnPath = ((ModifyColumnCommentOp) alterTableOp).getColumnPath(); + } + return columnPath != null && columnPath.isNested() ? columnPath : null; + } + + private static ColumnDefinition getColumnDefinition(AlterTableOp alterTableOp) { + if (alterTableOp instanceof AddColumnOp) { + return ((AddColumnOp) alterTableOp).getColumnDef(); + } + if (alterTableOp instanceof ModifyColumnOp) { + return ((ModifyColumnOp) alterTableOp).getColumnDef(); + } + return null; + } + + private static boolean isIcebergColumnSchemaOperation(AlterTableOp alterTableOp) { + return alterTableOp instanceof AddColumnOp + || alterTableOp instanceof AddColumnsOp + || alterTableOp instanceof DropColumnOp + || alterTableOp instanceof RenameColumnOp + || alterTableOp instanceof ModifyColumnOp + || alterTableOp instanceof ModifyColumnCommentOp + || alterTableOp instanceof ReorderColumnsOp; + } + + private static void checkIcebergColumnDefinition(AlterTableOp alterTableOp, ColumnDefinition columnDefinition) + throws AnalysisException { + if (columnDefinition == null) { + return; + } + if (columnDefinition.isKey()) { + throw new AnalysisException("KEY is not supported for Iceberg ADD/MODIFY COLUMN"); + } + if (columnDefinition.getGeneratedColumnDesc().isPresent()) { + throw new AnalysisException("Generated columns are not supported for Iceberg ADD/MODIFY COLUMN"); + } + if (alterTableOp instanceof ModifyColumnOp + && (columnDefinition.hasDefaultValue() || columnDefinition.hasOnUpdateDefaultValue())) { + columnDefinition.validateComplexTypeDefaultValue(); + throw new AnalysisException("Modifying default values is not supported for Iceberg columns: " + + ((ModifyColumnOp) alterTableOp).getColumnPath().getFullPath()); + } + if ((alterTableOp instanceof AddColumnOp || alterTableOp instanceof AddColumnsOp) + && columnDefinition.hasOnUpdateDefaultValue()) { + throw new AnalysisException("ON UPDATE is not supported for Iceberg ADD COLUMN: " + + columnDefinition.getName()); + } + } + + private static String getRollupName(AlterTableOp alterTableOp) { + if (alterTableOp instanceof AddColumnOp) { + return ((AddColumnOp) alterTableOp).getRollupName(); + } + if (alterTableOp instanceof AddColumnsOp) { + return ((AddColumnsOp) alterTableOp).getRollupName(); + } + if (alterTableOp instanceof DropColumnOp) { + return ((DropColumnOp) alterTableOp).getRollupName(); + } + if (alterTableOp instanceof ModifyColumnOp) { + return ((ModifyColumnOp) alterTableOp).getRollupName(); + } + if (alterTableOp instanceof ReorderColumnsOp) { + return ((ReorderColumnsOp) alterTableOp).getRollupName(); + } + return null; + } + private void rewriteAlterOpForOlapTable(ConnectContext ctx, OlapTable table) throws UserException { List alterTableOps = new ArrayList<>(); for (AlterTableOp alterClause : ops) { @@ -242,6 +380,7 @@ private void checkExternalTableOperationAllow(TableIf table) throws UserExceptio || alterTableOp instanceof DropColumnOp || alterTableOp instanceof RenameColumnOp || alterTableOp instanceof ModifyColumnOp + || (alterTableOp instanceof ModifyColumnCommentOp && table instanceof IcebergExternalTable) || alterTableOp instanceof ReorderColumnsOp || alterTableOp instanceof ModifyEngineOp || alterTableOp instanceof ModifyTablePropertiesOp diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnOp.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnOp.java index 09c686779a862c..2e6c5ab3727820 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnOp.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnOp.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.trees.plans.commands.info; import org.apache.doris.alter.AlterOpType; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.AggregateType; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Env; @@ -44,6 +45,7 @@ */ public class AddColumnOp extends AlterTableOp { private ColumnDefinition columnDef; + private ColumnPath columnPath; // Column position private ColumnPosition colPos; // if rollupName is null, add to column to base index. @@ -56,8 +58,17 @@ public class AddColumnOp extends AlterTableOp { public AddColumnOp(ColumnDefinition columnDef, ColumnPosition colPos, String rollupName, Map properties) { + this(columnDef, ColumnPath.of(columnDef.getName()), colPos, rollupName, properties); + } + + /** + * Create add-column operation with the original nested column path. + */ + public AddColumnOp(ColumnDefinition columnDef, ColumnPath columnPath, ColumnPosition colPos, + String rollupName, Map properties) { super(AlterOpType.SCHEMA_CHANGE); this.columnDef = columnDef; + this.columnPath = columnPath; this.colPos = colPos; this.rollupName = rollupName; this.properties = properties; @@ -67,6 +78,14 @@ public Column getColumn() { return column; } + public ColumnDefinition getColumnDef() { + return columnDef; + } + + public ColumnPath getColumnPath() { + return columnPath; + } + public ColumnPosition getColPos() { return colPos; } @@ -84,7 +103,7 @@ public void validate(ConnectContext ctx) throws UserException { if (colPos != null) { colPos.analyze(); } - validateColumnDef(tableName, columnDef, colPos, rollupName); + validateColumnDef(tableName, columnDef, colPos, rollupName, columnPath.isNested()); column = columnDef.translateToCatalogStyleForSchemaChange(); } @@ -111,7 +130,7 @@ public boolean allowOpRowBinlog() { @Override public String toSql() { StringBuilder sb = new StringBuilder(); - sb.append("ADD COLUMN ").append(columnDef.toSql()); + sb.append("ADD COLUMN ").append(columnDef.toSql(columnPath.toSql())); if (colPos != null) { sb.append(" ").append(colPos.toSql()); } @@ -132,6 +151,11 @@ public String toString() { public static void validateColumnDef(TableNameInfo tableName, ColumnDefinition columnDef, ColumnPosition colPos, String rollupName) throws UserException { + validateColumnDef(tableName, columnDef, colPos, rollupName, false); + } + + private static void validateColumnDef(TableNameInfo tableName, ColumnDefinition columnDef, ColumnPosition colPos, + String rollupName, boolean nestedColumn) throws UserException { if (columnDef == null) { throw new AnalysisException("No column definition in add column clause."); } @@ -182,7 +206,11 @@ public static void validateColumnDef(TableNameInfo tableName, ColumnDefinition c } } - columnDef.validate(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType); + if (nestedColumn) { + columnDef.validateNestedColumn(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType); + } else { + columnDef.validate(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType); + } if (!columnDef.isNullable() && !columnDef.hasDefaultValue()) { ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_DEFAULT_FOR_FIELD, columnDef.getName()); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnsOp.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnsOp.java index 418185ed7557f2..9ff326ffb385de 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnsOp.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AddColumnsOp.java @@ -62,6 +62,10 @@ public List getColumns() { return columns; } + public List getColumnDefinitions() { + return columnDefs == null ? Collections.emptyList() : columnDefs; + } + public String getRollupName() { return rollupName; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java index 4be6315f079fdb..6c386489139492 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java @@ -39,6 +39,7 @@ import org.apache.doris.nereids.types.TinyIntType; import org.apache.doris.nereids.types.VarcharType; import org.apache.doris.nereids.types.coercion.CharacterType; +import org.apache.doris.nereids.util.SqlLiteralUtils; import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.ConnectContextUtil; import org.apache.doris.qe.SessionVariable; @@ -61,9 +62,13 @@ public class ColumnDefinition { private boolean isKey; private AggregateType aggType; private boolean isNullable; + // Distinguishes an explicit NULL/NOT NULL clause from the parser's default nullability. + private final boolean nullableSpecified; private Optional defaultValue; private Optional onUpdateDefaultValue = Optional.empty(); private final String comment; + // Distinguishes an explicit COMMENT '' clause from an omitted COMMENT clause. + private final boolean commentSpecified; private final boolean isVisible; private boolean aggTypeImplicit = false; private long autoIncInitValue = -1; @@ -83,7 +88,7 @@ public ColumnDefinition(String name, DataType type, boolean isKey, AggregateType Optional onUpdateDefaultValue, String comment, Optional generatedColumnDesc) { this(name, type, isKey, aggType, nullableType, autoIncInitValue, defaultValue, onUpdateDefaultValue, - comment, true, generatedColumnDesc); + comment, comment != null && !comment.isEmpty(), true, generatedColumnDesc); } /** @@ -96,8 +101,10 @@ public ColumnDefinition(String name, DataType type, boolean isKey, AggregateType this.isKey = isKey; this.aggType = aggType; this.isNullable = isNullable; + this.nullableSpecified = true; this.defaultValue = defaultValue; this.comment = comment; + this.commentSpecified = comment != null && !comment.isEmpty(); this.isVisible = isVisible; } @@ -112,10 +119,12 @@ private ColumnDefinition(String name, DataType type, boolean isKey, AggregateTyp this.isKey = isKey; this.aggType = aggType; this.isNullable = isNullable; + this.nullableSpecified = true; this.autoIncInitValue = autoIncInitValue; this.defaultValue = defaultValue; this.onUpdateDefaultValue = onUpdateDefaultValue; this.comment = comment; + this.commentSpecified = comment != null && !comment.isEmpty(); this.isVisible = isVisible; } @@ -126,15 +135,30 @@ public ColumnDefinition(String name, DataType type, boolean isKey, AggregateType ColumnNullableType nullableType, long autoIncInitValue, Optional defaultValue, Optional onUpdateDefaultValue, String comment, boolean isVisible, Optional generatedColumnDesc) { + this(name, type, isKey, aggType, nullableType, autoIncInitValue, defaultValue, onUpdateDefaultValue, + comment, comment != null && !comment.isEmpty(), isVisible, generatedColumnDesc); + } + + /** + * constructor + */ + public ColumnDefinition(String name, DataType type, boolean isKey, AggregateType aggType, + ColumnNullableType nullableType, long autoIncInitValue, Optional defaultValue, + Optional onUpdateDefaultValue, String comment, boolean commentSpecified, + boolean isVisible, + Optional generatedColumnDesc) { this.name = name; this.type = type; this.isKey = isKey; this.aggType = aggType; this.isNullable = nullableType.getNullable(type.toCatalogDataType().getPrimitiveType()); + this.nullableSpecified = nullableType == ColumnNullableType.NULLABLE + || nullableType == ColumnNullableType.NOT_NULLABLE; this.autoIncInitValue = autoIncInitValue; this.defaultValue = defaultValue; this.onUpdateDefaultValue = onUpdateDefaultValue; this.comment = comment; + this.commentSpecified = commentSpecified; this.isVisible = isVisible; this.generatedColumnDesc = generatedColumnDesc; } @@ -183,6 +207,10 @@ public boolean hasDefaultValue() { return defaultValue.isPresent(); } + public boolean hasOnUpdateDefaultValue() { + return onUpdateDefaultValue.isPresent(); + } + public boolean isVisible() { return isVisible; } @@ -203,23 +231,40 @@ public String getComment(boolean escapeQuota) { return SqlUtils.escapeQuota(comment); } + public boolean isCommentSpecified() { + return commentSpecified; + } + /** * toSql */ public String toSql() { + return toSql("`" + name + "`", true); + } + + /** + * Convert this column definition to schema-change SQL with a caller-provided column name. + * Unlike {@link #toSql()}, this overload emits COMMENT only when it was explicitly specified. + */ + public String toSql(String columnNameSql) { + return toSql(columnNameSql, commentSpecified); + } + + private String toSql(String columnNameSql, boolean includeComment) { StringBuilder sb = new StringBuilder(); - sb.append("`").append(name).append("` "); + sb.append(columnNameSql).append(" "); sb.append(type.toSql()).append(" "); if (aggType != null && aggType != AggregateType.NONE) { sb.append(aggType.name()).append(" "); } - if (!isNullable) { - sb.append("NOT NULL "); - } else { - // should append NULL to make result can be executed right. - sb.append("NULL "); + if (nullableSpecified) { + if (!isNullable) { + sb.append("NOT NULL "); + } else { + sb.append("NULL "); + } } if (autoIncInitValue != -1) { @@ -247,7 +292,9 @@ public String toSql() { sb.append("DEFAULT ").append("NULL").append(" "); } } - sb.append("COMMENT \"").append(SqlUtils.escapeQuota(comment)).append("\""); + if (includeComment) { + sb.append("COMMENT ").append(SqlLiteralUtils.quoteStringLiteral(getComment())); + } return sb.toString(); } @@ -306,10 +353,25 @@ private void checkKeyColumnType(boolean isOlap) { */ public void validate(boolean isOlap, Set keysSet, Set clusterKeySet, boolean isEnableMergeOnWrite, KeysType keysType) { + validateInternal(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType, false); + } + + /** + * Validate a nested field whose name is scoped by its parent path rather than the Doris top-level column namespace. + */ + public void validateNestedColumn(boolean isOlap, Set keysSet, Set clusterKeySet, + boolean isEnableMergeOnWrite, KeysType keysType) { + validateInternal(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType, true); + } + + private void validateInternal(boolean isOlap, Set keysSet, Set clusterKeySet, + boolean isEnableMergeOnWrite, KeysType keysType, boolean nestedColumn) { try { // if enableAddHiddenColumn is true, can add hidden column. // So does not check if the column name starts with __DORIS_ - if (enableAddHiddenColumn) { + if (nestedColumn) { + FeNameFormat.checkColumnNameBypassSystemColumnPrefix(name); + } else if (enableAddHiddenColumn) { FeNameFormat.checkColumnNameBypassHiddenColumn(name); } else { FeNameFormat.checkColumnName(name); @@ -428,18 +490,8 @@ public void validate(boolean isOlap, Set keysSet, Set clusterKey .getValue().equals(DefaultValue.ARRAY_EMPTY_DEFAULT_VALUE.getValue())) { throw new AnalysisException("Array type column default value only support null or " + DefaultValue.ARRAY_EMPTY_DEFAULT_VALUE); - } else if (type.isMapType()) { - if (defaultValue.isPresent() && defaultValue.get() != DefaultValue.NULL_DEFAULT_VALUE) { - throw new AnalysisException("Map type column default value just support null"); - } - } else if (type.isStructType()) { - if (defaultValue.isPresent() && defaultValue.get() != DefaultValue.NULL_DEFAULT_VALUE) { - throw new AnalysisException("Struct type column default value just support null"); - } - } else if (type.isJsonType() || type.isVariantType()) { - if (defaultValue.isPresent() && defaultValue.get() != DefaultValue.NULL_DEFAULT_VALUE) { - throw new AnalysisException("Json or Variant type column default value just support null"); - } + } else { + validateComplexTypeDefaultValue(); } if (!isNullable && defaultValue.isPresent() @@ -521,6 +573,22 @@ public void validate(boolean isOlap, Set keysSet, Set clusterKey validateGeneratedColumnInfo(); } + /** + * Validate non-null defaults for complex types before connector-specific validation. + */ + public void validateComplexTypeDefaultValue() throws AnalysisException { + if (!defaultValue.isPresent() || defaultValue.get() == DefaultValue.NULL_DEFAULT_VALUE) { + return; + } + if (type.isMapType()) { + throw new AnalysisException("Map type column default value just support null"); + } else if (type.isStructType()) { + throw new AnalysisException("Struct type column default value just support null"); + } else if (type.isJsonType() || type.isVariantType()) { + throw new AnalysisException("Json or Variant type column default value just support null"); + } + } + /** * translate to catalog create table stmt */ @@ -554,6 +622,8 @@ public Column translateToCatalogStyleForSchemaChange() { generatedColumnDesc.map(desc -> ConnectContextUtil.getAffectQueryResultInPlanVariables(ConnectContext.get())) .orElse(null)); + column.setNullableSpecified(nullableSpecified); + column.setCommentSpecified(commentSpecified); column.setAggregationTypeImplicit(aggTypeImplicit); return column; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/DropColumnOp.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/DropColumnOp.java index be5e91c8a0b7d4..f9a3444cbe0394 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/DropColumnOp.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/DropColumnOp.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.trees.plans.commands.info; import org.apache.doris.alter.AlterOpType; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.KeysType; @@ -42,6 +43,7 @@ */ public class DropColumnOp extends AlterTableOp { private String colName; + private ColumnPath columnPath; private String rollupName; private Map properties; @@ -50,8 +52,13 @@ public class DropColumnOp extends AlterTableOp { * DropColumnOp */ public DropColumnOp(String colName, String rollupName, Map properties) { + this(ColumnPath.of(colName), rollupName, properties); + } + + public DropColumnOp(ColumnPath columnPath, String rollupName, Map properties) { super(AlterOpType.SCHEMA_CHANGE); - this.colName = colName; + this.colName = columnPath.getLeafName(); + this.columnPath = columnPath; this.rollupName = rollupName; this.properties = properties; } @@ -60,6 +67,10 @@ public String getColName() { return colName; } + public ColumnPath getColumnPath() { + return columnPath; + } + public String getRollupName() { return rollupName; } @@ -70,7 +81,7 @@ public void validate(ConnectContext ctx) throws UserException { ErrorReport.reportAnalysisException(ErrorCode.ERR_WRONG_COLUMN_NAME, colName, FeNameFormat.getColumnNameRegex()); } - if (colName.startsWith(Column.HIDDEN_COLUMN_PREFIX)) { + if (!columnPath.isNested() && colName.startsWith(Column.HIDDEN_COLUMN_PREFIX)) { throw new AnalysisException("Do not support drop hidden column"); } @@ -171,7 +182,7 @@ public boolean allowOpRowBinlog() { @Override public String toSql() { StringBuilder sb = new StringBuilder(); - sb.append("DROP COLUMN `").append(colName).append("`"); + sb.append("DROP COLUMN ").append(columnPath.toSql()); if (rollupName != null) { sb.append(" FROM `").append(rollupName).append("`"); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnCommentOp.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnCommentOp.java index 0b1c6ac49558e2..9b0e4cadab3514 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnCommentOp.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnCommentOp.java @@ -18,8 +18,10 @@ package org.apache.doris.nereids.trees.plans.commands.info; import org.apache.doris.alter.AlterOpType; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.UserException; +import org.apache.doris.nereids.util.SqlLiteralUtils; import org.apache.doris.qe.ConnectContext; import com.google.common.base.Strings; @@ -31,17 +33,25 @@ * ModifyColumnCommentOp */ public class ModifyColumnCommentOp extends AlterTableOp { - private String colName; + private ColumnPath columnPath; private String comment; public ModifyColumnCommentOp(String colName, String comment) { + this(ColumnPath.of(colName), comment); + } + + public ModifyColumnCommentOp(ColumnPath columnPath, String comment) { super(AlterOpType.MODIFY_COLUMN_COMMENT); - this.colName = colName; + this.columnPath = columnPath; this.comment = Strings.nullToEmpty(comment); } public String getColName() { - return colName; + return columnPath.getFullPath(); + } + + public ColumnPath getColumnPath() { + return columnPath; } public String getComment() { @@ -55,7 +65,7 @@ public Map getProperties() { @Override public void validate(ConnectContext ctx) throws UserException { - if (Strings.isNullOrEmpty(colName)) { + if (columnPath == null || Strings.isNullOrEmpty(columnPath.getFullPath())) { throw new AnalysisException("Empty column name"); } } @@ -79,8 +89,8 @@ public boolean allowOpRowBinlog() { @Override public String toSql() { StringBuilder sb = new StringBuilder(); - sb.append("MODIFY COLUMN COMMENT ").append(colName); - sb.append(" '").append(comment).append("'"); + sb.append("MODIFY COLUMN ").append(columnPath.toSql()); + sb.append(" COMMENT ").append(SqlLiteralUtils.quoteStringLiteral(comment)); return sb.toString(); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnOp.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnOp.java index 049d12bc325ae4..f5acfbed9df9cf 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnOp.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ModifyColumnOp.java @@ -18,6 +18,7 @@ package org.apache.doris.nereids.trees.plans.commands.info; import org.apache.doris.alter.AlterOpType; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.KeysType; @@ -43,6 +44,7 @@ */ public class ModifyColumnOp extends AlterTableOp { private ColumnDefinition columnDef; + private ColumnPath columnPath; private ColumnPosition colPos; // which rollup is to be modify, if rollup is null, modify base table. private String rollupName; @@ -54,8 +56,17 @@ public class ModifyColumnOp extends AlterTableOp { public ModifyColumnOp(ColumnDefinition columnDef, ColumnPosition colPos, String rollup, Map properties) { + this(columnDef, ColumnPath.of(columnDef.getName()), colPos, rollup, properties); + } + + /** + * Create modify-column operation with the original nested column path. + */ + public ModifyColumnOp(ColumnDefinition columnDef, ColumnPath columnPath, ColumnPosition colPos, + String rollup, Map properties) { super(AlterOpType.SCHEMA_CHANGE); this.columnDef = columnDef; + this.columnPath = columnPath; this.colPos = colPos; this.rollupName = rollup; this.properties = properties; @@ -65,6 +76,10 @@ public Column getColumn() { return column; } + public ColumnPath getColumnPath() { + return columnPath; + } + public ColumnPosition getColPos() { return colPos; } @@ -122,7 +137,11 @@ public void validate(ConnectContext ctx) throws UserException { } } } - columnDef.validate(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType); + if (columnPath.isNested()) { + columnDef.validateNestedColumn(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType); + } else { + columnDef.validate(isOlap, keysSet, clusterKeySet, isEnableMergeOnWrite, keysType); + } if (colPos != null) { colPos.analyze(); if (olapTable != null) { @@ -187,7 +206,7 @@ public boolean needChangeMTMVState() { @Override public String toSql() { StringBuilder sb = new StringBuilder(); - sb.append("MODIFY COLUMN ").append(columnDef.toSql()); + sb.append("MODIFY COLUMN ").append(columnDef.toSql(columnPath.toSql())); if (colPos != null) { sb.append(" ").append(colPos); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/RenameColumnOp.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/RenameColumnOp.java index e66ae02c15337b..15270360535d85 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/RenameColumnOp.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/RenameColumnOp.java @@ -18,10 +18,12 @@ package org.apache.doris.nereids.trees.plans.commands.info; import org.apache.doris.alter.AlterOpType; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.Column; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.FeNameFormat; import org.apache.doris.common.UserException; +import org.apache.doris.common.util.SqlUtils; import org.apache.doris.qe.ConnectContext; import com.google.common.base.Strings; @@ -33,11 +35,17 @@ */ public class RenameColumnOp extends AlterTableOp { private String colName; + private ColumnPath columnPath; private String newColName; public RenameColumnOp(String colName, String newColName) { + this(ColumnPath.of(colName), newColName); + } + + public RenameColumnOp(ColumnPath columnPath, String newColName) { super(AlterOpType.RENAME); - this.colName = colName; + this.colName = columnPath.getLeafName(); + this.columnPath = columnPath; this.newColName = newColName; } @@ -45,6 +53,10 @@ public String getColName() { return colName; } + public ColumnPath getColumnPath() { + return columnPath; + } + public String getNewColName() { return newColName; } @@ -59,11 +71,15 @@ public void validate(ConnectContext ctx) throws UserException { throw new AnalysisException("New column name is not set"); } - if (colName.startsWith(Column.HIDDEN_COLUMN_PREFIX)) { + if (!columnPath.isNested() && colName.startsWith(Column.HIDDEN_COLUMN_PREFIX)) { throw new AnalysisException("Do not support rename hidden column"); } - FeNameFormat.checkColumnName(newColName); + if (columnPath.isNested()) { + FeNameFormat.checkColumnNameBypassSystemColumnPrefix(newColName); + } else { + FeNameFormat.checkColumnName(newColName); + } } @Override @@ -83,7 +99,7 @@ public boolean needChangeMTMVState() { @Override public String toSql() { - return "RENAME COLUMN " + colName + " " + newColName; + return "RENAME COLUMN " + columnPath.toSql() + " " + SqlUtils.getIdentSql(newColName); } @Override 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 a47352a29e60cc..c6a5dc1b0285ad 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 @@ -468,7 +468,8 @@ 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()), - cf.getContainsNull(), cf.getComment() == null ? "" : cf.getComment())) + cf.getContainsNull(), cf.getComment() == null ? "" : cf.getComment(), + cf.isCommentSpecified())) .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 63e94bc369bb0c..aefcf20f227ff1 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 @@ -17,6 +17,9 @@ package org.apache.doris.nereids.types; +import org.apache.doris.common.util.SqlUtils; +import org.apache.doris.nereids.parser.NereidsParser; +import org.apache.doris.nereids.util.SqlLiteralUtils; import org.apache.doris.nereids.util.Utils; import java.util.Objects; @@ -32,6 +35,7 @@ public class StructField { private final DataType dataType; private final boolean nullable; private final String comment; + private final boolean commentSpecified; /** * StructField Constructor @@ -40,10 +44,16 @@ public class StructField { * @param nullable Indicates if values of this field can be `null` values */ public StructField(String name, DataType dataType, boolean nullable, String comment) { + this(name, dataType, nullable, comment, comment != null && !comment.isEmpty()); + } + + public StructField(String name, DataType dataType, boolean nullable, String comment, + boolean commentSpecified) { this.name = Objects.requireNonNull(name, "name should not be null").toLowerCase(); 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; } public String getName() { @@ -62,6 +72,10 @@ public String getComment() { return comment; } + public boolean isCommentSpecified() { + return commentSpecified; + } + public StructField conversion() { if (this.dataType.equals(dataType.conversion())) { return this; @@ -70,22 +84,24 @@ public StructField conversion() { } public StructField withDataType(DataType dataType) { - return new StructField(name, dataType, nullable, comment); + return new StructField(name, dataType, nullable, comment, commentSpecified); } public StructField withDataTypeAndNullable(DataType dataType, boolean nullable) { - return new StructField(name, dataType, nullable, comment); + return new StructField(name, dataType, nullable, comment, commentSpecified); } public org.apache.doris.catalog.StructField toCatalogDataType() { return new org.apache.doris.catalog.StructField( - name, dataType.toCatalogDataType(), comment, nullable); + name, dataType.toCatalogDataType(), comment, nullable, commentSpecified); } public String toSql() { - return name + ":" + dataType.toSql() + String nameSql = NereidsParser.isValidUnquotedIdentifier(name) ? name : SqlUtils.getIdentSql(name); + return nameSql + ":" + dataType.toSql() + (nullable ? "" : " NOT NULL") - + (comment.isEmpty() ? "" : " COMMENT " + comment); + + (!commentSpecified ? "" : " COMMENT " + + SqlLiteralUtils.quoteStringLiteral(comment)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/SqlLiteralUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/SqlLiteralUtils.java new file mode 100644 index 00000000000000..5d87bc08f5f56d --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/SqlLiteralUtils.java @@ -0,0 +1,96 @@ +// 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.util; + +import org.apache.doris.qe.SqlModeHelper; + +/** + * Utilities for decoding and rendering SQL string literals. + */ +public final class SqlLiteralUtils { + + private SqlLiteralUtils() { + } + + /** + * Decode backslash escape sequences used in SQL string literals. + */ + public static String unescapeBackSlash(String value) { + StringBuilder result = new StringBuilder(); + int length = value.length(); + for (int i = 0; i < length; ++i) { + char current = value.charAt(i); + if (current == '\\' && (i + 1) < length) { + switch (value.charAt(i + 1)) { + case 'n': + result.append('\n'); + break; + case 't': + result.append('\t'); + break; + case 'r': + result.append('\r'); + break; + case 'b': + result.append('\b'); + break; + case '0': + result.append('\0'); + break; + case 'Z': + result.append('\032'); + break; + case '_': + case '%': + result.append('\\'); + result.append(value.charAt(i + 1)); + break; + default: + result.append(value.charAt(i + 1)); + break; + } + i++; + } else { + result.append(current); + } + } + return result.toString(); + } + + /** + * Decode a STRING_LITERAL token according to the current SQL mode. + */ + public static String parseStringLiteral(String text) { + String value = text.substring(1, text.length() - 1); + if (text.charAt(0) == '\'') { + value = value.replace("''", "'"); + } else { + value = value.replace("\"\"", "\""); + } + return SqlModeHelper.hasNoBackSlashEscapes() ? value : unescapeBackSlash(value); + } + + /** + * Quote a value as a STRING_LITERAL that can be parsed under the current SQL mode. + */ + public static String quoteStringLiteral(String value) { + String escaped = SqlModeHelper.hasNoBackSlashEscapes() + ? value : value.replace("\\", "\\\\"); + return "\"" + escaped.replace("\"", "\"\"") + "\""; + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java index 59d7cf0a9fc571..7c910231f6a16c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java @@ -918,6 +918,7 @@ public void testRowBinlogSchemaChangeKeepsHiddenKeyColumn() throws Exception { List columnNames = rowBinlogSchema.stream().map(Column::getName).collect(Collectors.toList()); Assertions.assertEquals(1, columnNames.indexOf("__DORIS_TEST_HIDDEN_KEY__")); + Assertions.assertEquals(2, columnNames.indexOf(Column.BINLOG_TSO_COL)); Assertions.assertEquals(3, columnNames.indexOf(Column.BINLOG_LSN_COL)); Column hiddenValue = new Column("__DORIS_TEST_HIDDEN_VALUE__", PrimitiveType.INT); diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java index 937167c34c351d..f78c186d0d1b5c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergMetadataOpsValidationTest.java @@ -17,6 +17,7 @@ package org.apache.doris.datasource.iceberg; +import org.apache.doris.analysis.ColumnPath; import org.apache.doris.catalog.ArrayType; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.MapType; @@ -24,37 +25,61 @@ import org.apache.doris.catalog.StructField; import org.apache.doris.catalog.StructType; import org.apache.doris.catalog.Type; +import org.apache.doris.catalog.info.ColumnPosition; import org.apache.doris.common.UserException; import org.apache.doris.common.security.authentication.ExecutionAuthenticator; import org.apache.doris.datasource.ExternalCatalog; +import org.apache.doris.datasource.ExternalTable; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.UpdateSchema; import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.CommitFailedException; +import org.apache.iceberg.hadoop.HadoopCatalog; import org.apache.iceberg.types.Types; import org.apache.iceberg.types.Types.NestedField; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.mockito.MockedStatic; import org.mockito.Mockito; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; public class IcebergMetadataOpsValidationTest { private IcebergMetadataOps ops; + private ExternalCatalog dorisCatalog; private Method validateForModifyColumnMethod; private Method validateForModifyComplexColumnMethod; + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Before public void setUp() throws Exception { - ExternalCatalog dorisCatalog = Mockito.mock(ExternalCatalog.class); + dorisCatalog = Mockito.mock(ExternalCatalog.class); Catalog icebergCatalog = Mockito.mock(Catalog.class, Mockito.withSettings().extraInterfaces(SupportsNamespaces.class)); Mockito.when(dorisCatalog.getExecutionAuthenticator()).thenReturn(new ExecutionAuthenticator() { }); Mockito.when(dorisCatalog.getProperties()).thenReturn(Collections.emptyMap()); + Mockito.doReturn(Optional.empty()).when(dorisCatalog).getDbForReplay(Mockito.anyString()); ops = new IcebergMetadataOps(dorisCatalog, icebergCatalog); validateForModifyColumnMethod = IcebergMetadataOps.class.getDeclaredMethod( @@ -88,6 +113,15 @@ public void testValidateForModifyColumnSuccess() throws Throwable { invokeValidateForModifyColumn(column, currentCol); } + @Test + public void testValidateForModifyColumnRejectsComplexToPrimitive() { + Column column = new Column("struct_col", Type.INT, true); + NestedField currentCol = Types.NestedField.required(1, "struct_col", Types.StructType.of( + Types.NestedField.required(2, "value", Types.IntegerType.get()))); + assertUserException(() -> invokeValidateForModifyColumn(column, currentCol), + "Modify column type from complex to primitive is not supported: struct_col"); + } + @Test public void testValidateForModifyComplexColumnRejectsPrimitiveType() { Column column = new Column("arr_i", Type.INT, true); @@ -182,6 +216,1193 @@ public void testValidateForModifyComplexColumnSuccess() throws Throwable { invokeValidateForModifyComplexColumn(column, currentCol); } + @Test + public void testRejectUnsupportedIcebergTargetTypesBeforeUpdateSchema() { + Schema schema = requiredNestedSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + StructType unsupportedStruct = new StructType(new StructField("value", Type.LARGEINT)); + ArrayType unsupportedArray = ArrayType.create(Type.LARGEINT, true); + MapType unsupportedMap = new MapType(Type.STRING, Type.LARGEINT); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.addColumn(dorisTable, ColumnPath.fromDotName("info.new_field"), + new Column("new_field", Type.LARGEINT, true), null, 1L), + "is not supported for Iceberg column new_field"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.metric"), + new Column("metric", Type.LARGEINT, true), null, 1L), + "is not supported for Iceberg column info.metric"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.child"), + new Column("child", unsupportedStruct, false), null, 1L), + "is not supported for Iceberg column info.child.value"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.events"), + new Column("events", unsupportedArray, false), null, 1L), + "is not supported for Iceberg column info.events.element"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.attrs"), + new Column("attrs", unsupportedMap, false), null, 1L), + "is not supported for Iceberg column info.attrs.value"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testComplexModifyPreservesRequiredNestedFields() throws Throwable { + Schema schema = requiredNestedSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.child"), + new Column("child", new StructType(new StructField("value", Type.BIGINT)), true), null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.events"), + new Column("events", ArrayType.create(Type.BIGINT, true), true), null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.attrs"), + new Column("attrs", new MapType(Type.STRING, Type.BIGINT), true), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn("info.child.value", Types.LongType.get(), null); + Mockito.verify(updateSchema).updateColumn("info.events.element", Types.LongType.get(), null); + Mockito.verify(updateSchema).updateColumn("info.attrs.value", Types.LongType.get(), null); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional(Mockito.anyString()); + Mockito.verify(updateSchema, Mockito.times(3)).commit(); + } + + @Test + public void testComplexModifyPersistsDecodedStructMemberComment() throws Throwable { + Schema schema = new Schema(Types.NestedField.optional(1, "info", Types.StructType.of( + Types.NestedField.optional(2, "payload", Types.StructType.of( + Types.NestedField.optional(3, "name", Types.StringType.get(), "old comment")))))); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + String decodedComment = "owner's \"field\" C:\\tmp\\"; + Column column = new Column("payload", new StructType( + new StructField("name", Type.STRING, decodedComment, true)), true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.payload"), column, null, 1L); + } + + Mockito.verify(updateSchema).updateColumnDoc("info.payload.name", decodedComment); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testPrimitiveModifyPreservesOmittedCommentAndClearsExplicitEmptyComment() throws Throwable { + Schema schema = new Schema( + Types.NestedField.optional(1, "info", Types.StructType.of( + Types.NestedField.optional(2, "metric", Types.IntegerType.get(), "metric doc"), + Types.NestedField.optional(3, "clear_me", Types.StringType.get(), "clear doc"))), + Types.NestedField.optional(4, "top_metric", Types.IntegerType.get(), "top metric doc")); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + Column clearComment = new Column("clear_me", Type.STRING, true, ""); + clearComment.setCommentSpecified(true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.metric"), + new Column("metric", Type.BIGINT, true), null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.of("top_metric"), + new Column("top_metric", Type.BIGINT, true), null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.clear_me"), + clearComment, null, 1L); + } + + Mockito.verify(updateSchema).updateColumn("info.metric", Types.LongType.get(), "metric doc"); + Mockito.verify(updateSchema).updateColumn("top_metric", Types.LongType.get(), "top metric doc"); + Mockito.verify(updateSchema).updateColumnDoc("info.clear_me", ""); + Mockito.verify(updateSchema, Mockito.times(3)).commit(); + } + + @Test + public void testFullStructModifyPreservesOmittedChildComments() throws Throwable { + Schema schema = new Schema(Types.NestedField.optional(1, "info", Types.StructType.of( + Types.NestedField.optional(2, "payload", Types.StructType.of( + Types.NestedField.optional(3, "metric", Types.IntegerType.get(), "metric doc"), + Types.NestedField.optional(4, "clear_me", Types.StringType.get(), "clear doc"), + Types.NestedField.optional(5, "details", Types.StructType.of( + Types.NestedField.optional( + 6, "count", Types.IntegerType.get(), "count doc")), "details doc")), + "payload doc")))); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + StructType detailsType = new StructType( + new StructField("count", Type.BIGINT, "", true, false)); + StructType payloadType = new StructType( + new StructField("metric", Type.BIGINT, "", true, false), + new StructField("clear_me", Type.STRING, "", true, true), + new StructField("details", detailsType, "", true, false)); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.payload"), + new Column("payload", payloadType, true), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn( + "info.payload.metric", Types.LongType.get(), "metric doc"); + Mockito.verify(updateSchema).updateColumnDoc("info.payload.clear_me", ""); + Mockito.verify(updateSchema).updateColumn( + "info.payload.details.count", Types.LongType.get(), "count doc"); + Mockito.verify(updateSchema, Mockito.never()).updateColumnDoc( + Mockito.eq("info.payload"), Mockito.nullable(String.class)); + Mockito.verify(updateSchema, Mockito.never()).updateColumnDoc( + Mockito.eq("info.payload.details"), Mockito.nullable(String.class)); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testPrimitiveModifyPreservesRequiredNestedField() throws Throwable { + Schema schema = requiredNestedSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.metric"), + new Column("metric", Type.BIGINT, true), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn("info.metric", Types.LongType.get(), null); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional(Mockito.anyString()); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testTopLevelModifyPreservesRequiredMixedCaseFields() throws Throwable { + Schema schema = new Schema( + Types.NestedField.required(1, "Id", Types.IntegerType.get()), + Types.NestedField.required(2, "Payload", Types.StructType.of( + Types.NestedField.required(3, "Value", Types.IntegerType.get())))); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.of("id"), + new Column("id", Type.BIGINT, true), null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.of("payload"), new Column("payload", + new StructType(new StructField("Value", Type.BIGINT)), true), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn("Id", Types.LongType.get(), null); + Mockito.verify(updateSchema).updateColumn("Payload.Value", Types.LongType.get(), null); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional(Mockito.anyString()); + Mockito.verify(updateSchema, Mockito.times(2)).commit(); + } + + @Test + public void testTopLevelModifyDoesNotResolveQuotedComponentAsNestedPath() { + Schema schema = new Schema( + Types.NestedField.optional(1, "a", Types.StructType.of( + Types.NestedField.optional(2, "b", Types.IntegerType.get()))), + Types.NestedField.optional(3, "b", Types.IntegerType.get())); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.of("a.b"), + new Column("a.b", Type.BIGINT, true), null, 1L), + "Column a.b does not exist"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testTopLevelModifyPreservesDottedTopLevelName() throws Throwable { + Schema schema = new Schema(Types.NestedField.optional(1, "a.b", Types.IntegerType.get())); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.of("a.b"), + new Column("a.b", Type.BIGINT, true), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn("a.b", Types.LongType.get(), null); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testPrimitiveModifyPreservesActualTypeWhenMappingDisabled() throws Throwable { + Schema schema = mappedPrimitiveSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + Mockito.when(dorisCatalog.getEnableMappingVarbinary()).thenReturn(false); + Mockito.when(dorisCatalog.getEnableMappingTimestampTz()).thenReturn(false); + + Column topUuid = new Column("top_uuid", Type.STRING, true); + topUuid.setNullableSpecified(true); + Column nestedUuid = new Column("uuid_value", Type.STRING, true); + nestedUuid.setNullableSpecified(true); + Column nestedTimestamp = new Column( + "tz_value", ScalarType.createDatetimeV2Type(6), true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.of("top_uuid"), topUuid, ColumnPosition.FIRST, 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.uuid_value"), nestedUuid, + new ColumnPosition("other"), 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.tz_value"), + nestedTimestamp, null, 1L); + } + + Mockito.verify(updateSchema, Mockito.never()).updateColumnDoc( + Mockito.anyString(), Mockito.nullable(String.class)); + Mockito.verify(updateSchema).makeColumnOptional("top_uuid"); + Mockito.verify(updateSchema).makeColumnOptional("info.uuid_value"); + Mockito.verify(updateSchema).moveFirst("top_uuid"); + Mockito.verify(updateSchema).moveAfter("info.uuid_value", "info.other"); + Mockito.verify(updateSchema, Mockito.never()).updateColumn( + Mockito.anyString(), Mockito.any(org.apache.iceberg.types.Type.PrimitiveType.class), + Mockito.nullable(String.class)); + Mockito.verify(updateSchema, Mockito.times(3)).commit(); + } + + @Test + public void testPrimitiveModifyPreservesActualTypeWhenMappingEnabled() throws Throwable { + Schema schema = mappedPrimitiveSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + Mockito.when(dorisCatalog.getEnableMappingVarbinary()).thenReturn(true); + Mockito.when(dorisCatalog.getEnableMappingTimestampTz()).thenReturn(true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.of("top_uuid"), + new Column("top_uuid", ScalarType.createVarbinaryType(16), true), null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.tz_value"), + new Column("tz_value", ScalarType.createTimeStampTzType(6), true), null, 1L); + } + + Mockito.verify(updateSchema, Mockito.never()).updateColumnDoc( + Mockito.anyString(), Mockito.nullable(String.class)); + Mockito.verify(updateSchema, Mockito.never()).updateColumn( + Mockito.anyString(), Mockito.any(org.apache.iceberg.types.Type.PrimitiveType.class), + Mockito.nullable(String.class)); + Mockito.verify(updateSchema, Mockito.times(2)).commit(); + } + + @Test + public void testComplexModifyIgnoresUnchangedMappedChildren() throws Throwable { + Schema schema = mappedComplexSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + Mockito.when(dorisCatalog.getEnableMappingVarbinary()).thenReturn(true); + Mockito.when(dorisCatalog.getEnableMappingTimestampTz()).thenReturn(true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("outer.payload"), + new Column("payload", mappedPayloadDorisType(Type.BIGINT, 8, + ScalarType.createVarbinaryType(4)), true), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn( + "outer.payload.metric", Types.LongType.get(), null); + Mockito.verify(updateSchema, Mockito.times(1)).updateColumn( + Mockito.anyString(), Mockito.any(org.apache.iceberg.types.Type.PrimitiveType.class), + Mockito.nullable(String.class)); + Mockito.verify(updateSchema, Mockito.never()).updateColumnDoc( + Mockito.anyString(), Mockito.nullable(String.class)); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testComplexModifyRejectsChangedUnsupportedMappedChildrenBeforeUpdateSchema() { + Schema schema = mappedComplexSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(dorisCatalog.getEnableMappingVarbinary()).thenReturn(true); + Mockito.when(dorisCatalog.getEnableMappingTimestampTz()).thenReturn(true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("outer.payload"), + new Column("payload", mappedPayloadDorisType(Type.LARGEINT, 8, + ScalarType.createVarbinaryType(4)), true), null, 1L), + "Type largeint is not supported for Iceberg column outer.payload.metric"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("outer.payload"), + new Column("payload", mappedPayloadDorisType(Type.INT, 16, + ScalarType.createVarbinaryType(4)), true), null, 1L), + "Type varbinary(16) is not supported for Iceberg column outer.payload.fixed_value"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("outer.payload"), + new Column("payload", mappedPayloadDorisType(Type.INT, 8, + ScalarType.createVarbinaryType(8)), true), null, 1L), + "Cannot change MAP key type from varbinary(4) to varbinary(8)"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testLegacyModifyColumnTreatsNullabilityAsExplicit() throws Throwable { + Schema schema = new Schema(Types.NestedField.required(1, "id", Types.IntegerType.get())); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + // Iceberg schema columns are represented as keys in Doris, so the legacy API must not + // interpret isKey as an explicit KEY clause. + ops.modifyColumn(dorisTable, + new Column("id", Type.BIGINT, true, null, true, null, ""), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn("id", Types.LongType.get(), ""); + Mockito.verify(updateSchema).makeColumnOptional("id"); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testLegacyComplexModifyDoesNotInferRecursiveNullableChanges() throws Throwable { + Schema schema = requiredNestedSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + Column column = new Column("info", new StructType( + new StructField("metric", Type.INT), + new StructField("child", new StructType(new StructField("value", Type.INT))), + new StructField("events", ArrayType.create(Type.INT, true)), + new StructField("attrs", new MapType(Type.STRING, Type.INT))), true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, column, null, 1L); + } + + Mockito.verify(updateSchema).makeColumnOptional("info"); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional("info.metric"); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional("info.child.value"); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional("info.events.element"); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional("info.attrs.value"); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testExplicitNullableModifyMakesRequiredFieldsOptional() throws Throwable { + Schema schema = requiredNestedSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + Column topLevelColumn = new Column("info", new StructType( + new StructField("metric", Type.BIGINT), + new StructField("child", new StructType(new StructField("value", Type.INT))), + new StructField("events", ArrayType.create(Type.INT, true)), + new StructField("attrs", new MapType(Type.STRING, Type.INT))), true); + topLevelColumn.setNullableSpecified(true); + Column nestedColumn = new Column("metric", Type.BIGINT, true); + nestedColumn.setNullableSpecified(true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.of("info"), topLevelColumn, null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.metric"), nestedColumn, null, 1L); + } + + Mockito.verify(updateSchema).makeColumnOptional("info"); + Mockito.verify(updateSchema).makeColumnOptional("info.metric"); + Mockito.verify(updateSchema, Mockito.times(2)).commit(); + } + + @Test + public void testSchemaCommitConflictDoesNotPartiallyApplyComplexModify() throws Exception { + String dbName = "db"; + String tableName = "conflict_table"; + TableIdentifier tableIdentifier = TableIdentifier.of(dbName, tableName); + Map properties = new HashMap<>(); + properties.put(CatalogProperties.WAREHOUSE_LOCATION, + temporaryFolder.newFolder("iceberg_warehouse").toURI().toString()); + + HadoopCatalog icebergCatalog = new HadoopCatalog(); + icebergCatalog.setConf(new Configuration()); + icebergCatalog.initialize("conflict_catalog", properties); + icebergCatalog.createNamespace(Namespace.of(dbName)); + Table staleTable = icebergCatalog.createTable(tableIdentifier, new Schema( + Types.NestedField.optional(1, "info", Types.StructType.of( + Types.NestedField.optional(2, "a", Types.IntegerType.get()), + Types.NestedField.optional(3, "b", Types.IntegerType.get()))))); + Table concurrentTable = icebergCatalog.loadTable(tableIdentifier); + + ExternalCatalog conflictDorisCatalog = Mockito.mock(ExternalCatalog.class); + AtomicBoolean conflictInjected = new AtomicBoolean(false); + Mockito.when(conflictDorisCatalog.getExecutionAuthenticator()).thenReturn(new ExecutionAuthenticator() { + @Override + public void execute(Runnable task) { + Assert.assertTrue("schema commit should execute only once", conflictInjected.compareAndSet(false, true)); + concurrentTable.updateSchema().renameColumn("info.a", "concurrent_a").commit(); + task.run(); + } + }); + Mockito.when(conflictDorisCatalog.getProperties()).thenReturn(Collections.emptyMap()); + Mockito.doReturn(Optional.empty()).when(conflictDorisCatalog).getDbForReplay(Mockito.anyString()); + IcebergMetadataOps conflictOps = new IcebergMetadataOps(conflictDorisCatalog, icebergCatalog); + + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn(dbName); + Mockito.when(dorisTable.getRemoteName()).thenReturn(tableName); + StructType promotedInfo = new StructType( + new StructField("a", Type.BIGINT), + new StructField("b", Type.BIGINT)); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(staleTable); + + try { + conflictOps.modifyColumn(dorisTable, ColumnPath.of("info"), + new Column("info", promotedInfo, true), null, 1L); + Assert.fail("expected schema commit conflict"); + } catch (UserException e) { + Assert.assertTrue(e.getMessage().contains("Failed to modify column: info")); + Assert.assertTrue("expected Iceberg commit conflict but got " + e.getCause(), + e.getCause() instanceof CommitFailedException); + } + } + + Schema committedSchema = icebergCatalog.loadTable(tableIdentifier).schema(); + Assert.assertNull(committedSchema.findField("info.a")); + Assert.assertEquals(Types.IntegerType.get(), committedSchema.findType("info.concurrent_a")); + Assert.assertEquals(Types.IntegerType.get(), committedSchema.findType("info.b")); + Mockito.verify(conflictDorisCatalog, Mockito.never()).getDbForReplay(Mockito.anyString()); + + icebergCatalog.dropTable(tableIdentifier); + icebergCatalog.dropNamespace(Namespace.of(dbName)); + icebergCatalog.close(); + } + + @Test + public void testRenamePreservesNestedIdentifierFieldPaths() throws Exception { + String dbName = "db"; + String tableName = "identifier_table"; + TableIdentifier tableIdentifier = TableIdentifier.of(dbName, tableName); + Map properties = new HashMap<>(); + properties.put(CatalogProperties.WAREHOUSE_LOCATION, + temporaryFolder.newFolder("identifier_warehouse").toURI().toString()); + + HadoopCatalog icebergCatalog = new HadoopCatalog(); + icebergCatalog.setConf(new Configuration()); + icebergCatalog.initialize("identifier_catalog", properties); + icebergCatalog.createNamespace(Namespace.of(dbName)); + Schema schema = new Schema(Arrays.asList( + Types.NestedField.required(1, "root", Types.StructType.of( + Types.NestedField.required(2, "child", Types.StructType.of( + Types.NestedField.required(3, "id", Types.IntegerType.get()), + Types.NestedField.optional(4, "value", Types.StringType.get())))))), + Collections.singleton(3)); + Table icebergTable = icebergCatalog.createTable(tableIdentifier, schema); + + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn(dbName); + Mockito.when(dorisTable.getRemoteName()).thenReturn(tableName); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.renameColumn(dorisTable, ColumnPath.fromDotName("root.child.id"), "renamed_id", 1L); + icebergTable.refresh(); + Assert.assertEquals(Collections.singleton("root.child.renamed_id"), + icebergTable.schema().identifierFieldNames()); + + ops.renameColumn(dorisTable, ColumnPath.fromDotName("root.child"), "renamed_child", 1L); + icebergTable.refresh(); + Assert.assertEquals(Collections.singleton("root.renamed_child.renamed_id"), + icebergTable.schema().identifierFieldNames()); + + ops.renameColumn(dorisTable, "root", "renamed_root", 1L); + icebergTable.refresh(); + Assert.assertEquals(Collections.singleton("renamed_root.renamed_child.renamed_id"), + icebergTable.schema().identifierFieldNames()); + Assert.assertEquals(3, icebergTable.schema().findField( + "renamed_root.renamed_child.renamed_id").fieldId()); + } + + icebergCatalog.dropTable(tableIdentifier); + icebergCatalog.dropNamespace(Namespace.of(dbName)); + icebergCatalog.close(); + } + + @Test + public void testRenameDoesNotRewriteDottedIdentifierSibling() throws Exception { + String dbName = "db"; + String tableName = "dotted_identifier_table"; + TableIdentifier tableIdentifier = TableIdentifier.of(dbName, tableName); + Map properties = new HashMap<>(); + properties.put(CatalogProperties.WAREHOUSE_LOCATION, + temporaryFolder.newFolder("dotted_identifier_warehouse").toURI().toString()); + + HadoopCatalog icebergCatalog = new HadoopCatalog(); + icebergCatalog.setConf(new Configuration()); + icebergCatalog.initialize("dotted_identifier_catalog", properties); + icebergCatalog.createNamespace(Namespace.of(dbName)); + Schema schema = new Schema(Arrays.asList( + Types.NestedField.required(1, "a", Types.IntegerType.get()), + Types.NestedField.required(2, "a.b", Types.IntegerType.get())), + Collections.singleton(2)); + Table icebergTable = icebergCatalog.createTable(tableIdentifier, schema); + + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn(dbName); + Mockito.when(dorisTable.getRemoteName()).thenReturn(tableName); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.renameColumn(dorisTable, "a", "renamed", 1L); + icebergTable.refresh(); + Assert.assertNotNull(icebergTable.schema().findField("renamed")); + Assert.assertEquals(Collections.singleton("a.b"), icebergTable.schema().identifierFieldNames()); + Assert.assertEquals(2, icebergTable.schema().findField("a.b").fieldId()); + } + + icebergCatalog.dropTable(tableIdentifier); + icebergCatalog.dropNamespace(Namespace.of(dbName)); + icebergCatalog.close(); + } + + @Test + public void testNestedColumnOperationsRejectDefaultMetadata() { + Schema schema = new Schema(Types.NestedField.optional(1, "s", Types.StructType.of( + Types.NestedField.optional(2, "existing", Types.IntegerType.get())))); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + Column nestedAddDefaultColumn = new Column("new_col", Type.BIGINT, false, null, true, "7", ""); + Column nestedDefaultColumn = new Column("existing", Type.BIGINT, false, null, true, "7", ""); + Column nestedOnUpdateColumn = Mockito.spy(new Column("existing", Type.BIGINT, true)); + Mockito.doReturn(true).when(nestedOnUpdateColumn).hasOnUpdateDefaultValue(); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.addColumn(dorisTable, ColumnPath.fromDotName("s.new_col"), + nestedAddDefaultColumn, null, 1L), + "DEFAULT and ON UPDATE are not supported for Iceberg nested ADD COLUMN: s.new_col"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("s.existing"), + nestedDefaultColumn, null, 1L), + "Modifying default values is not supported for Iceberg columns: s.existing"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("s.existing"), + nestedOnUpdateColumn, null, 1L), + "Modifying default values is not supported for Iceberg columns: s.existing"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testTopLevelColumnOperationsRejectUnsupportedDefaultMetadata() { + Schema schema = new Schema(Types.NestedField.optional(1, "id", Types.IntegerType.get())); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + Column defaultColumn = new Column("id", Type.BIGINT, false, null, true, "7", ""); + Column onUpdateColumn = Mockito.spy(new Column("id", Type.BIGINT, true)); + Column onUpdateAddColumn = Mockito.spy(new Column("new_col", Type.BIGINT, true)); + Mockito.doReturn(true).when(onUpdateColumn).hasOnUpdateDefaultValue(); + Mockito.doReturn(true).when(onUpdateAddColumn).hasOnUpdateDefaultValue(); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.modifyColumn(dorisTable, defaultColumn, null, 1L), + "Modifying default values is not supported for Iceberg columns: id"); + assertUserException(() -> ops.modifyColumn( + dorisTable, ColumnPath.of("id"), onUpdateColumn, null, 1L), + "Modifying default values is not supported for Iceberg columns: id"); + assertUserException(() -> ops.addColumn(dorisTable, onUpdateAddColumn, null, 1L), + "ON UPDATE is not supported for Iceberg ADD COLUMN: new_col"); + assertUserException(() -> ops.addColumns( + dorisTable, Collections.singletonList(onUpdateAddColumn), 1L), + "ON UPDATE is not supported for Iceberg ADD COLUMN: new_col"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testUnsupportedPrimitiveModifyFailsBeforeUpdateSchema() { + Schema schema = new Schema( + Types.NestedField.required(1, "top_long", Types.LongType.get()), + Types.NestedField.required(2, "info", Types.StructType.of( + Types.NestedField.required(3, "metric", Types.LongType.get()), + Types.NestedField.required(4, "child", Types.StructType.of( + Types.NestedField.required(5, "value", Types.IntegerType.get())))))); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.modifyColumn( + dorisTable, ColumnPath.of("info"), new Column("info", Type.INT, true), null, 1L), + "Modify column type from complex to primitive is not supported: info"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.child"), + new Column("child", Type.INT, true), null, 1L), + "Modify column type from complex to primitive is not supported: info.child"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.of("top_long"), + new Column("top_long", Type.INT, true), null, 1L), + "Cannot change column type: top_long: long -> int"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("info.metric"), + new Column("metric", Type.INT, true), null, 1L), + "Cannot change column type: info.metric: long -> int"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testRejectKeyAndGeneratedMetadataBeforeUpdateSchema() { + Schema schema = new Schema(Types.NestedField.optional(1, "id", Types.IntegerType.get())); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + Column keyColumn = new Column("id", Type.BIGINT, true, null, true, null, ""); + Column generatedColumn = Mockito.mock(Column.class); + Mockito.when(generatedColumn.getName()).thenReturn("id"); + Mockito.when(generatedColumn.isGeneratedColumn()).thenReturn(true); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.addColumn(dorisTable, keyColumn, null, 1L), + "KEY is not supported for Iceberg ADD/MODIFY COLUMN"); + assertUserException(() -> ops.addColumns(dorisTable, Collections.singletonList(keyColumn), 1L), + "KEY is not supported for Iceberg ADD/MODIFY COLUMN"); + assertUserException(() -> ops.modifyColumn( + dorisTable, ColumnPath.of("id"), keyColumn, null, 1L), + "KEY is not supported for Iceberg ADD/MODIFY COLUMN"); + assertUserException(() -> ops.addColumns(dorisTable, + Collections.singletonList(generatedColumn), 1L), + "Generated columns are not supported for Iceberg ADD/MODIFY COLUMN"); + assertUserException(() -> ops.modifyColumn( + dorisTable, ColumnPath.of("id"), generatedColumn, null, 1L), + "Generated columns are not supported for Iceberg ADD/MODIFY COLUMN"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testModifyComplexColumnRejectsCaseInsensitiveStructFieldAdditions() { + Schema schema = mixedCaseNestedSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + StructType infoType = new StructType( + new StructField("Metric", Type.INT), + new StructField("Label", Type.STRING), + new StructField("metric", Type.INT)); + ArrayType eventsType = ArrayType.create(new StructType( + new StructField("Score", Type.INT), + new StructField("score", Type.INT)), true); + MapType attrsType = new MapType(Type.STRING, new StructType( + new StructField("Code", Type.INT), + new StructField("code", Type.INT))); + StructType duplicateNewFieldsType = new StructType( + new StructField("Metric", Type.INT), + new StructField("Label", Type.STRING), + new StructField("Extra", Type.INT), + new StructField("EXTRA", Type.STRING)); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.modifyColumn( + dorisTable, new Column("info", infoType, true), null, 1L), + "Added struct field 'metric' conflicts with existing field"); + assertUserException(() -> ops.modifyColumn( + dorisTable, new Column("events", eventsType, true), null, 1L), + "Added struct field 'score' conflicts with existing field"); + assertUserException(() -> ops.modifyColumn( + dorisTable, new Column("attrs", attrsType, true), null, 1L), + "Added struct field 'code' conflicts with existing field"); + assertUserException(() -> ops.modifyColumn( + dorisTable, new Column("info", duplicateNewFieldsType, true), null, 1L), + "Added struct field 'extra' conflicts with existing field"); + } + + Mockito.verifyNoInteractions(updateSchema); + } + + @Test + public void testResolveNestedColumnPathSupportsStructArrayElementAndMapValue() throws Throwable { + Schema schema = nestedSchema(); + Assert.assertTrue(ops.resolveNestedColumnPath(schema, ColumnPath.fromDotName("s"), "add").isStructType()); + Assert.assertTrue(ops.resolveNestedColumnPath(schema, ColumnPath.fromDotName("arr.element"), "add") + .isStructType()); + Assert.assertTrue(ops.resolveNestedColumnPath(schema, ColumnPath.fromDotName("m.value"), "add") + .isStructType()); + } + + @Test + public void testResolveNestedColumnPathUsesCaseInsensitiveCanonicalIcebergPath() throws Throwable { + Schema schema = mixedCaseNestedSchema(); + Assert.assertEquals("Info.Metric", + ops.getCanonicalColumnPath(schema, ColumnPath.fromDotName("info.metric"), "modify")); + Assert.assertEquals("Events.element.Score", + ops.getCanonicalColumnPath(schema, ColumnPath.fromDotName("events.element.score"), "modify")); + Assert.assertEquals("Attrs.value.Code", + ops.getCanonicalColumnPath(schema, ColumnPath.fromDotName("attrs.value.code"), "modify")); + Assert.assertEquals("Info.Label", + ops.getPositionReferencePath(schema, ColumnPath.fromDotName("Info.NewField"), + new ColumnPosition("label"), "add")); + } + + @Test + public void testValidateNoCaseInsensitiveSiblingCollisionRejectsAddAndRenameTargets() { + Types.StructType parentType = mixedCaseNestedSchema().findField("Info").type().asStructType(); + ColumnPath parentPath = ColumnPath.fromDotName("Info"); + assertUserException(() -> ops.validateNoCaseInsensitiveSiblingCollision( + parentType, parentPath, "metric", null, "add"), + "Cannot add nested column 'Info.metric': conflicts with existing Iceberg field 'Info.Metric'"); + assertUserException(() -> ops.validateNoCaseInsensitiveSiblingCollision( + parentType, parentPath, "metric", parentType.field("Label"), "rename"), + "Cannot rename nested column 'Info.metric': conflicts with existing Iceberg field 'Info.Metric'"); + } + + @Test + public void testValidateNoCaseInsensitiveSiblingCollisionAllowsCaseOnlyRename() throws Throwable { + Types.StructType parentType = mixedCaseNestedSchema().findField("Info").type().asStructType(); + ops.validateNoCaseInsensitiveSiblingCollision(parentType, ColumnPath.fromDotName("Info"), + "metric", parentType.field("Metric"), "rename"); + } + + @Test + public void testTopLevelCaseInsensitiveCollisionsAndCaseOnlyRename() throws Throwable { + Schema schema = new Schema( + Types.NestedField.optional(1, "Id", Types.IntegerType.get()), + Types.NestedField.optional(2, "Label", Types.StringType.get())); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.addColumn( + dorisTable, new Column("id", Type.STRING, true), null, 1L), + "Cannot add column 'id': conflicts with existing Iceberg field 'Id'"); + assertUserException(() -> ops.addColumns(dorisTable, + Collections.singletonList(new Column("id", Type.STRING, true)), 1L), + "Cannot add column 'id': conflicts with existing Iceberg field 'Id'"); + assertUserException(() -> ops.addColumns(dorisTable, Arrays.asList( + new Column("new_field", Type.STRING, true), + new Column("NEW_FIELD", Type.STRING, true)), 1L), + "conflicts with another requested column (case-insensitive)"); + assertUserException(() -> ops.renameColumn(dorisTable, "label", "id", 1L), + "Cannot rename column 'id': conflicts with existing Iceberg field 'Id'"); + + ops.renameColumn(dorisTable, "id", "id", 1L); + } + + Mockito.verify(updateSchema).renameColumn("Id", "id"); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testReorderColumnsUsesCanonicalIcebergNames() throws Throwable { + Schema schema = new Schema( + Types.NestedField.optional(1, "Id", Types.IntegerType.get()), + Types.NestedField.optional(2, "Label", Types.StringType.get())); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.reorderColumns(dorisTable, Arrays.asList("label", "id"), 1L); + } + + Mockito.verify(updateSchema).moveFirst("Label"); + Mockito.verify(updateSchema).moveAfter("Id", "Label"); + Mockito.verify(updateSchema).commit(); + } + + @Test + public void testModifyColumnSupportsDirectArrayElementAndMapValue() throws Throwable { + Schema schema = primitiveContainerSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("arr.element"), + new Column("element", Type.BIGINT, true), null, 1L); + ops.modifyColumn(dorisTable, ColumnPath.fromDotName("m.value"), + new Column("value", Type.BIGINT, true), null, 1L); + } + + Mockito.verify(updateSchema).updateColumn("arr.element", Types.LongType.get(), null); + Mockito.verify(updateSchema).updateColumn("m.value", Types.LongType.get(), null); + Mockito.verify(updateSchema, Mockito.never()).makeColumnOptional(Mockito.anyString()); + Mockito.verify(updateSchema, Mockito.times(2)).commit(); + } + + @Test + public void testModifyColumnRejectsPositionForDirectArrayElementAndMapValue() { + Schema schema = primitiveContainerSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("arr.element"), + new Column("element", Type.BIGINT, true), ColumnPosition.FIRST, 1L), + "Cannot apply column position to 'arr.element': parent column path 'arr' is not a struct"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("arr.element"), + new Column("element", Type.BIGINT, true), new ColumnPosition("element"), 1L), + "Cannot apply column position to 'arr.element': parent column path 'arr' is not a struct"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("m.value"), + new Column("value", Type.BIGINT, true), ColumnPosition.FIRST, 1L), + "Cannot apply column position to 'm.value': parent column path 'm' is not a struct"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("m.value"), + new Column("value", Type.BIGINT, true), new ColumnPosition("value"), 1L), + "Cannot apply column position to 'm.value': parent column path 'm' is not a struct"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testModifyColumnCommentUsesCanonicalNestedPaths() throws Throwable { + Schema schema = mixedCaseNestedSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + UpdateSchema updateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + Mockito.when(icebergTable.updateSchema()).thenReturn(updateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + ops.modifyColumnComment(dorisTable, ColumnPath.fromDotName("info.metric"), + "struct comment", 1L); + ops.modifyColumnComment(dorisTable, ColumnPath.fromDotName("events.element.score"), + "array element comment", 1L); + ops.modifyColumnComment(dorisTable, ColumnPath.fromDotName("attrs.value.code"), + "map value comment", 1L); + } + + Mockito.verify(updateSchema).updateColumnDoc("Info.Metric", "struct comment"); + Mockito.verify(updateSchema).updateColumnDoc("Events.element.Score", "array element comment"); + Mockito.verify(updateSchema).updateColumnDoc("Attrs.value.Code", "map value comment"); + Mockito.verify(updateSchema, Mockito.times(3)).commit(); + } + + @Test + public void testRejectsCommentsOnDirectArrayElementAndMapValue() { + Schema schema = primitiveContainerSchema(); + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(dorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(icebergTable.schema()).thenReturn(schema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.modifyColumnComment( + dorisTable, ColumnPath.fromDotName("arr.element"), "array element comment", 1L), + "Iceberg does not support comments on collection element or value fields: arr.element"); + assertUserException(() -> ops.modifyColumnComment( + dorisTable, ColumnPath.fromDotName("m.value"), "map value comment", 1L), + "Iceberg does not support comments on collection element or value fields: m.value"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("arr.element"), + new Column("element", Type.BIGINT, true, "array element comment"), null, 1L), + "Iceberg does not support comments on collection element or value fields: arr.element"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("m.value"), + new Column("value", Type.BIGINT, true, "map value comment"), null, 1L), + "Iceberg does not support comments on collection element or value fields: m.value"); + Column arrayElementWithEmptyComment = new Column("element", Type.BIGINT, true, ""); + arrayElementWithEmptyComment.setCommentSpecified(true); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("arr.element"), + arrayElementWithEmptyComment, null, 1L), + "Iceberg does not support comments on collection element or value fields: arr.element"); + Column mapValueWithEmptyComment = new Column("value", Type.BIGINT, true, ""); + mapValueWithEmptyComment.setCommentSpecified(true); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.fromDotName("m.value"), + mapValueWithEmptyComment, null, 1L), + "Iceberg does not support comments on collection element or value fields: m.value"); + assertUserException(() -> ops.modifyColumnComment( + dorisTable, ColumnPath.fromDotName("m.key"), "map key comment", 1L), + "Cannot modify comment MAP key nested column"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testRejectsTopLevelRowLineageMutationsForV3Tables() { + ExternalTable dorisTable = Mockito.mock(ExternalTable.class); + Table icebergTable = Mockito.mock(Table.class); + Mockito.when(icebergTable.properties()).thenReturn(Collections.singletonMap("format-version", "3")); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(dorisTable)).thenReturn(icebergTable); + + assertUserException(() -> ops.addColumn(dorisTable, + new Column("_row_id", Type.BIGINT, true), null, 1L), + "Cannot add Iceberg v3 reserved row lineage column: _row_id"); + assertUserException(() -> ops.addColumns(dorisTable, Collections.singletonList( + new Column("_last_updated_sequence_number", Type.BIGINT, true)), 1L), + "Cannot add Iceberg v3 reserved row lineage column: _last_updated_sequence_number"); + assertUserException(() -> ops.dropColumn(dorisTable, "_ROW_ID", 1L), + "Cannot drop Iceberg v3 reserved row lineage column: _ROW_ID"); + assertUserException(() -> ops.renameColumn(dorisTable, "_row_id", "renamed", 1L), + "Cannot rename Iceberg v3 reserved row lineage column: _row_id"); + assertUserException(() -> ops.renameColumn( + dorisTable, "id", "_last_updated_sequence_number", 1L), + "Cannot rename to Iceberg v3 reserved row lineage column: _last_updated_sequence_number"); + assertUserException(() -> ops.modifyColumn(dorisTable, ColumnPath.of("_row_id"), + new Column("_row_id", Type.BIGINT, true), null, 1L), + "Cannot modify Iceberg v3 reserved row lineage column: _row_id"); + assertUserException(() -> ops.modifyColumnComment(dorisTable, + ColumnPath.of("_last_updated_sequence_number"), "comment", 1L), + "Cannot modify comment for Iceberg v3 reserved row lineage column: " + + "_last_updated_sequence_number"); + assertUserException(() -> ops.reorderColumns(dorisTable, + Arrays.asList("_row_id", "id"), 1L), + "Cannot reorder Iceberg v3 reserved row lineage column: _row_id"); + } + + Mockito.verify(icebergTable, Mockito.never()).updateSchema(); + } + + @Test + public void testAllowsV3NestedAndV2TopLevelRowLineageNames() throws Throwable { + Schema v3Schema = new Schema( + Types.NestedField.optional(1, "id", Types.IntegerType.get()), + Types.NestedField.optional(2, "s", Types.StructType.of( + Types.NestedField.optional(3, "source", Types.LongType.get()), + Types.NestedField.optional(4, "_row_id", Types.LongType.get())))); + ExternalTable v3DorisTable = Mockito.mock(ExternalTable.class); + Table v3IcebergTable = Mockito.mock(Table.class); + UpdateSchema v3UpdateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(v3DorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(v3IcebergTable.properties()).thenReturn(Collections.singletonMap("format-version", "3")); + Mockito.when(v3IcebergTable.schema()).thenReturn(v3Schema); + Mockito.when(v3IcebergTable.updateSchema()).thenReturn(v3UpdateSchema); + + Schema v2Schema = new Schema(Types.NestedField.optional(1, "id", Types.IntegerType.get())); + ExternalTable v2DorisTable = Mockito.mock(ExternalTable.class); + Table v2IcebergTable = Mockito.mock(Table.class); + UpdateSchema v2UpdateSchema = Mockito.mock(UpdateSchema.class); + Mockito.when(v2DorisTable.getRemoteDbName()).thenReturn("db"); + Mockito.when(v2IcebergTable.properties()).thenReturn(Collections.singletonMap("format-version", "2")); + Mockito.when(v2IcebergTable.schema()).thenReturn(v2Schema); + Mockito.when(v2IcebergTable.updateSchema()).thenReturn(v2UpdateSchema); + + try (MockedStatic mockedIcebergUtils = + Mockito.mockStatic(IcebergUtils.class, Mockito.CALLS_REAL_METHODS)) { + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(v3DorisTable)).thenReturn(v3IcebergTable); + mockedIcebergUtils.when(() -> IcebergUtils.getIcebergTable(v2DorisTable)).thenReturn(v2IcebergTable); + + ops.addColumn(v3DorisTable, ColumnPath.fromDotName("s._last_updated_sequence_number"), + new Column("_last_updated_sequence_number", Type.BIGINT, true), null, 1L); + ops.renameColumn(v3DorisTable, ColumnPath.fromDotName("s.source"), "_last_updated_sequence_number", 1L); + ops.modifyColumn(v3DorisTable, ColumnPath.fromDotName("s._row_id"), + new Column("_row_id", Type.BIGINT, true), null, 1L); + ops.modifyColumnComment(v3DorisTable, ColumnPath.fromDotName("s._row_id"), "comment", 1L); + ops.dropColumn(v3DorisTable, ColumnPath.fromDotName("s._row_id"), 1L); + + ops.addColumn(v2DorisTable, new Column("_row_id", Type.BIGINT, true), null, 1L); + ops.renameColumn(v2DorisTable, "id", "_last_updated_sequence_number", 1L); + } + + Mockito.verify(v3UpdateSchema, Mockito.times(5)).commit(); + Mockito.verify(v2UpdateSchema, Mockito.times(2)).commit(); + } + + @Test + public void testResolveNestedColumnPathRejectsMapKey() { + assertUserException(() -> ops.resolveNestedColumnPath(nestedSchema(), ColumnPath.fromDotName("m.key.x"), + "modify"), + "Cannot modify MAP key nested column"); + } + + @Test + public void testResolveNestedColumnPathRejectsPrimitiveParent() { + assertUserException(() -> ops.resolveNestedColumnPath(nestedSchema(), ColumnPath.fromDotName("id.x"), + "modify"), + "Cannot resolve nested field under primitive column path"); + } + + @Test + public void testGetPositionReferencePathForNestedColumn() { + Assert.assertEquals("s.a", ops.getPositionReferencePath(ColumnPath.fromDotName("s.new_col"), + new ColumnPosition("a"))); + Assert.assertEquals("arr.element.x", ops.getPositionReferencePath( + ColumnPath.fromDotName("arr.element.new_col"), new ColumnPosition("x"))); + Assert.assertEquals("m.value.v", ops.getPositionReferencePath( + ColumnPath.fromDotName("m.value.new_col"), new ColumnPosition("v"))); + Assert.assertEquals("id", ops.getPositionReferencePath(ColumnPath.fromDotName("new_col"), + new ColumnPosition("id"))); + } + + @Test + public void testValidateNestedStructFieldSupportsStructArrayElementAndMapValueFields() throws Throwable { + Schema schema = nestedSchema(); + Assert.assertTrue(ops.validateNestedStructField(schema, ColumnPath.fromDotName("s.a"), "drop") + .isPrimitiveType()); + Assert.assertTrue(ops.validateNestedStructField(schema, ColumnPath.fromDotName("arr.element.x"), "drop") + .isPrimitiveType()); + Assert.assertTrue(ops.validateNestedStructField(schema, ColumnPath.fromDotName("m.value.v"), "rename") + .isPrimitiveType()); + } + + @Test + public void testValidateNestedStructFieldRejectsArrayElementAndMapValuePseudoFields() { + assertUserException(() -> ops.validateNestedStructField(nestedSchema(), ColumnPath.fromDotName("arr.element"), + "drop"), + "Parent column path 'arr' is not a struct"); + assertUserException(() -> ops.validateNestedStructField(nestedSchema(), ColumnPath.fromDotName("m.value"), + "rename"), + "Parent column path 'm' is not a struct"); + } + + @Test + public void testValidateNestedStructFieldRejectsMapKeyAndMissingField() { + assertUserException(() -> ops.validateNestedStructField(nestedSchema(), ColumnPath.fromDotName("m.key.k"), + "drop"), + "Cannot drop MAP key nested column"); + assertUserException(() -> ops.validateNestedStructField(nestedSchema(), ColumnPath.fromDotName("s.missing"), + "rename"), + "Column path does not exist in Iceberg schema"); + } + private void invokeValidateForModifyColumn(Column column, NestedField currentCol) throws Throwable { invokeValidationMethod(validateForModifyColumnMethod, column, currentCol); } @@ -203,8 +1424,9 @@ private void assertUserException(ThrowingRunnable runnable, String expectedMessa runnable.run(); Assert.fail("expected UserException"); } catch (Throwable t) { - Assert.assertTrue(t instanceof UserException); - Assert.assertTrue(t.getMessage().contains(expectedMessage)); + Assert.assertTrue("expected UserException but got " + t, t instanceof UserException); + Assert.assertTrue("expected message containing '" + expectedMessage + "' but was '" + + t.getMessage() + "'", t.getMessage().contains(expectedMessage)); } } @@ -212,4 +1434,86 @@ private void assertUserException(ThrowingRunnable runnable, String expectedMessa private interface ThrowingRunnable { void run() throws Throwable; } + + private Schema nestedSchema() { + return new Schema( + Types.NestedField.optional(1, "id", Types.IntegerType.get()), + Types.NestedField.optional(2, "s", Types.StructType.of( + Types.NestedField.optional(3, "a", Types.IntegerType.get()))), + Types.NestedField.optional(4, "arr", Types.ListType.ofOptional(5, + Types.StructType.of(Types.NestedField.optional(6, "x", Types.IntegerType.get())))), + Types.NestedField.optional(7, "m", Types.MapType.ofOptional(8, 9, + Types.StringType.get(), + Types.StructType.of(Types.NestedField.optional(10, "v", Types.IntegerType.get()))))); + } + + private Schema mixedCaseNestedSchema() { + return new Schema( + Types.NestedField.optional(1, "Id", Types.IntegerType.get()), + Types.NestedField.optional(2, "Info", Types.StructType.of( + Types.NestedField.optional(3, "Metric", Types.IntegerType.get()), + Types.NestedField.optional(4, "Label", Types.StringType.get()))), + Types.NestedField.optional(5, "Events", Types.ListType.ofOptional(6, + Types.StructType.of(Types.NestedField.optional(7, "Score", Types.IntegerType.get())))), + Types.NestedField.optional(8, "Attrs", Types.MapType.ofOptional(9, 10, + Types.StringType.get(), + Types.StructType.of(Types.NestedField.optional(11, "Code", Types.IntegerType.get()))))); + } + + private Schema primitiveContainerSchema() { + return new Schema( + Types.NestedField.optional(1, "arr", + Types.ListType.ofOptional(2, Types.IntegerType.get())), + Types.NestedField.optional(3, "m", Types.MapType.ofOptional( + 4, 5, Types.StringType.get(), Types.IntegerType.get()))); + } + + private Schema mappedPrimitiveSchema() { + return new Schema( + Types.NestedField.required(1, "top_uuid", Types.UUIDType.get()), + Types.NestedField.required(2, "top_other", Types.IntegerType.get()), + Types.NestedField.optional(3, "info", Types.StructType.of( + Types.NestedField.required(4, "uuid_value", Types.UUIDType.get()), + Types.NestedField.required(5, "tz_value", Types.TimestampType.withZone()), + Types.NestedField.required(6, "other", Types.IntegerType.get())))); + } + + private Schema mappedComplexSchema() { + return new Schema(Types.NestedField.optional(1, "outer", Types.StructType.of( + Types.NestedField.optional(2, "payload", Types.StructType.of( + Types.NestedField.optional(3, "uuid_value", Types.UUIDType.get()), + Types.NestedField.optional(4, "binary_value", Types.BinaryType.get()), + Types.NestedField.optional(5, "fixed_value", Types.FixedType.ofLength(8)), + Types.NestedField.optional(6, "tz_value", Types.TimestampType.withZone()), + Types.NestedField.optional(7, "metric", Types.IntegerType.get()), + Types.NestedField.optional(8, "events", + Types.ListType.ofOptional(9, Types.UUIDType.get())), + Types.NestedField.optional(10, "attrs", Types.MapType.ofOptional( + 11, 12, Types.FixedType.ofLength(4), Types.TimestampType.withZone()))))))); + } + + private StructType mappedPayloadDorisType(Type metricType, int fixedLength, Type mapKeyType) { + return new StructType( + new StructField("uuid_value", ScalarType.createVarbinaryType(16)), + new StructField("binary_value", + ScalarType.createVarbinaryType(ScalarType.MAX_VARBINARY_LENGTH)), + new StructField("fixed_value", ScalarType.createVarbinaryType(fixedLength)), + new StructField("tz_value", ScalarType.createTimeStampTzType(6)), + new StructField("metric", metricType), + new StructField("events", ArrayType.create( + ScalarType.createVarbinaryType(16), true)), + new StructField("attrs", new MapType( + mapKeyType, ScalarType.createTimeStampTzType(6)))); + } + + private Schema requiredNestedSchema() { + return new Schema(Types.NestedField.required(1, "info", Types.StructType.of( + Types.NestedField.required(2, "metric", Types.IntegerType.get()), + Types.NestedField.required(3, "child", Types.StructType.of( + Types.NestedField.required(4, "value", Types.IntegerType.get()))), + Types.NestedField.required(5, "events", Types.ListType.ofRequired( + 6, Types.IntegerType.get())), + Types.NestedField.required(7, "attrs", Types.MapType.ofRequired( + 8, 9, Types.StringType.get(), Types.IntegerType.get()))))); + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/IcebergNestedSchemaEvolutionParserTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/IcebergNestedSchemaEvolutionParserTest.java new file mode 100644 index 00000000000000..3b3355c06079e1 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/parser/IcebergNestedSchemaEvolutionParserTest.java @@ -0,0 +1,462 @@ +// 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.parser; + +import org.apache.doris.analysis.ColumnPath; +import org.apache.doris.nereids.exceptions.ParseException; +import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.commands.AlterTableCommand; +import org.apache.doris.nereids.trees.plans.commands.info.AddColumnOp; +import org.apache.doris.nereids.trees.plans.commands.info.AddColumnsOp; +import org.apache.doris.nereids.trees.plans.commands.info.AlterTableOp; +import org.apache.doris.nereids.trees.plans.commands.info.DropColumnOp; +import org.apache.doris.nereids.trees.plans.commands.info.ModifyColumnCommentOp; +import org.apache.doris.nereids.trees.plans.commands.info.ModifyColumnOp; +import org.apache.doris.nereids.trees.plans.commands.info.RenameColumnOp; +import org.apache.doris.nereids.types.StructType; +import org.apache.doris.qe.SqlModeHelper; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.List; +import java.util.Locale; + +public class IcebergNestedSchemaEvolutionParserTest { + + private final NereidsParser parser = new NereidsParser(); + + @Test + public void testParseNestedAddColumnPaths() { + assertSingleClausePath("ALTER TABLE t ADD COLUMN s.c STRING NULL", + AddColumnOp.class, "s.c"); + assertSingleClausePath("ALTER TABLE t ADD COLUMN s.first_col INT NULL FIRST", + AddColumnOp.class, "s.first_col"); + assertSingleClausePath("ALTER TABLE t ADD COLUMN s.after_col INT NULL AFTER a", + AddColumnOp.class, "s.after_col"); + assertSingleClausePath("ALTER TABLE t ADD COLUMN arr.element.y INT NULL", + AddColumnOp.class, "arr.element.y"); + assertSingleClausePath("ALTER TABLE t ADD COLUMN m.value.y INT NULL", + AddColumnOp.class, "m.value.y"); + } + + @Test + public void testParseNestedModifyDropRenamePaths() { + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN s.a BIGINT", + ModifyColumnOp.class, "s.a"); + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN s.a BIGINT AFTER b", + ModifyColumnOp.class, "s.a"); + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN arr.element BIGINT", + ModifyColumnOp.class, "arr.element"); + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN m.value BIGINT", + ModifyColumnOp.class, "m.value"); + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN s.a COMMENT 'nested comment'", + ModifyColumnCommentOp.class, "s.a"); + assertSingleClausePath("ALTER TABLE t DROP COLUMN s.c", + DropColumnOp.class, "s.c"); + assertSingleClausePath("ALTER TABLE t DROP COLUMN arr.element.y", + DropColumnOp.class, "arr.element.y"); + assertSingleClausePath("ALTER TABLE t DROP COLUMN m.value.y", + DropColumnOp.class, "m.value.y"); + assertSingleClausePath("ALTER TABLE t RENAME COLUMN s.c TO c2", + RenameColumnOp.class, "s.c"); + assertSingleClausePath("ALTER TABLE t RENAME COLUMN arr.element.y TO y2", + RenameColumnOp.class, "arr.element.y"); + assertSingleClausePath("ALTER TABLE t RENAME COLUMN m.value.y TO y2", + RenameColumnOp.class, "m.value.y"); + } + + @Test + public void testNestedColumnDefaultClausesAreNotInGrammar() { + for (String sql : List.of( + "ALTER TABLE t ADD COLUMN s.b BIGINT NULL DEFAULT 7", + "ALTER TABLE t ADD COLUMN s.c BIGINT NULL DEFAULT NULL", + "ALTER TABLE t ADD COLUMN s.ts DATETIME NULL DEFAULT CURRENT_TIMESTAMP " + + "ON UPDATE CURRENT_TIMESTAMP", + "ALTER TABLE t MODIFY COLUMN s.a BIGINT DEFAULT 7", + "ALTER TABLE t MODIFY COLUMN s.a BIGINT DEFAULT NULL", + "ALTER TABLE t MODIFY COLUMN s.ts DATETIME DEFAULT CURRENT_TIMESTAMP " + + "ON UPDATE CURRENT_TIMESTAMP")) { + Assertions.assertThrows(ParseException.class, () -> parser.parseSingle(sql), sql); + } + } + + @Test + public void testTopLevelColumnKeepsExistingDefaultGrammar() { + AddColumnOp add = assertSingleClausePath( + "ALTER TABLE t ADD COLUMN b BIGINT NULL DEFAULT 7", AddColumnOp.class, "b"); + Assertions.assertTrue(add.getColumnDef().hasDefaultValue()); + } + + @Test + public void testTopLevelColumnRoundTripPreservesOmittedIntent() { + AddColumnOp add = assertSingleClausePath( + "ALTER TABLE t ADD COLUMN added INT", AddColumnOp.class, "added"); + String renderedAdd = add.toSql(); + Assertions.assertFalse(renderedAdd.contains(" NULL")); + Assertions.assertFalse(renderedAdd.contains(" COMMENT ")); + AddColumnOp reparsedAdd = assertSingleClausePath( + "ALTER TABLE t " + renderedAdd, AddColumnOp.class, "added"); + Assertions.assertFalse(reparsedAdd.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + Assertions.assertFalse(reparsedAdd.getColumnDef() + .translateToCatalogStyleForSchemaChange().isCommentSpecified()); + + ModifyColumnOp modify = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN existing BIGINT", ModifyColumnOp.class, "existing"); + String renderedModify = modify.toSql(); + Assertions.assertFalse(renderedModify.contains(" NULL")); + Assertions.assertFalse(renderedModify.contains(" COMMENT ")); + ModifyColumnOp reparsedModify = assertSingleClausePath( + "ALTER TABLE t " + renderedModify, ModifyColumnOp.class, "existing"); + Assertions.assertFalse(reparsedModify.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + Assertions.assertFalse(reparsedModify.getColumnDef() + .translateToCatalogStyleForSchemaChange().isCommentSpecified()); + } + + @Test + public void testLegacyStringConstructorsKeepDottedTopLevelNames() { + DropColumnOp drop = new DropColumnOp("top.level", null, Collections.emptyMap()); + RenameColumnOp rename = new RenameColumnOp("top.level", "renamed"); + ModifyColumnCommentOp comment = new ModifyColumnCommentOp("top.level", "comment"); + + Assertions.assertFalse(drop.getColumnPath().isNested()); + Assertions.assertFalse(rename.getColumnPath().isNested()); + Assertions.assertFalse(comment.getColumnPath().isNested()); + Assertions.assertEquals("top.level", drop.getColumnPath().getFullPath()); + Assertions.assertEquals("top.level", rename.getColumnPath().getFullPath()); + Assertions.assertEquals("top.level", comment.getColumnPath().getFullPath()); + } + + @Test + public void testQuotedNestedIdentifiersAreNormalized() { + ModifyColumnOp dottedTopLevel = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN `a.b` BIGINT", + ModifyColumnOp.class, "a.b"); + Assertions.assertFalse(dottedTopLevel.getColumnPath().isNested()); + + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN info.`Metric` BIGINT", + ModifyColumnOp.class, "info.Metric"); + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN m_scalar.`key` BIGINT", + ModifyColumnOp.class, "m_scalar.key"); + assertSingleClausePath("ALTER TABLE t MODIFY COLUMN info.`Metric``Name` COMMENT 'quoted'", + ModifyColumnCommentOp.class, "info.Metric`Name"); + + AddColumnOp add = assertSingleClausePath( + "ALTER TABLE t ADD COLUMN info.`New``Field` INT NULL AFTER `Old``Field`", + AddColumnOp.class, "info.New`Field"); + Assertions.assertEquals("Old`Field", add.getColPos().getLastCol()); + AddColumnOp reparsedAdd = assertSingleClausePath( + "ALTER TABLE t " + add.toSql(), AddColumnOp.class, "info.New`Field"); + Assertions.assertEquals("Old`Field", reparsedAdd.getColPos().getLastCol()); + + RenameColumnOp rename = assertSingleClausePath( + "ALTER TABLE t RENAME COLUMN info.`Metric``Name` TO `New``Metric`", + RenameColumnOp.class, "info.Metric`Name"); + Assertions.assertEquals("New`Metric", rename.getNewColName()); + RenameColumnOp reparsedRename = assertSingleClausePath( + "ALTER TABLE t " + rename.toSql(), RenameColumnOp.class, "info.Metric`Name"); + Assertions.assertEquals("New`Metric", reparsedRename.getNewColName()); + } + + @Test + public void testStructMemberIdentifiersRoundTrip() { + AddColumnOp add = assertSingleClausePath( + "ALTER TABLE t ADD COLUMN info.payload " + + "STRUCT<`key`:INT,`Metric``Name`:STRING> NULL", + AddColumnOp.class, "info.payload"); + assertStructMemberNames((StructType) add.getColumnDef().getType()); + Assertions.assertTrue(add.toSql().contains("STRUCT<`key`:INT,`metric``name`:TEXT>")); + AddColumnOp reparsedAdd = assertSingleClausePath( + "ALTER TABLE t " + add.toSql(), AddColumnOp.class, "info.payload"); + assertStructMemberNames((StructType) reparsedAdd.getColumnDef().getType()); + + ModifyColumnOp modify = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.payload " + + "STRUCT<`key`:INT,`Metric``Name`:STRING>", + ModifyColumnOp.class, "info.payload"); + assertStructMemberNames((StructType) modify.getColumnDef().getType()); + ModifyColumnOp reparsedModify = assertSingleClausePath( + "ALTER TABLE t " + modify.toSql(), ModifyColumnOp.class, "info.payload"); + assertStructMemberNames((StructType) reparsedModify.getColumnDef().getType()); + + ModifyColumnOp ordinary = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.payload STRUCT", + ModifyColumnOp.class, "info.payload"); + Assertions.assertTrue(ordinary.toSql().contains("STRUCT")); + } + + @Test + public void testStructFieldSqlRoundTripIsLocaleIndependent() { + Locale originalLocale = Locale.getDefault(); + try { + Locale.setDefault(Locale.forLanguageTag("tr-TR")); + StructType structType = (StructType) parser.parseDataType("STRUCT<`in`:INT>"); + String renderedType = structType.toSql(); + + Assertions.assertTrue(renderedType.contains("`in`:INT")); + Assertions.assertDoesNotThrow(() -> parser.parseDataType(renderedType)); + } finally { + Locale.setDefault(originalLocale); + } + } + + @Test + public void testEmptyQuotedIdentifiersAreRejectedAsParseErrors() { + for (String sql : List.of( + "ALTER TABLE t ADD COLUMN `` INT NULL", + "ALTER TABLE t ADD COLUMN info.`` INT NULL", + "ALTER TABLE t MODIFY COLUMN info.`` BIGINT", + "ALTER TABLE t MODIFY COLUMN info.`` COMMENT 'comment'", + "ALTER TABLE t DROP COLUMN info.``", + "ALTER TABLE t RENAME COLUMN info.`` TO renamed", + "ALTER TABLE t ORDER BY (id, ``)")) { + ParseException exception = Assertions.assertThrows(ParseException.class, + () -> parser.parseSingle(sql), sql); + Assertions.assertTrue(exception.getMessage().contains("Quoted identifier cannot be empty"), sql); + } + } + + @Test + public void testEmptyQuotedIdentifiersOutsideColumnPathsKeepExistingParserSemantics() { + Assertions.assertDoesNotThrow(() -> parser.parseSingle("ALTER TABLE t CREATE BRANCH ``")); + Assertions.assertDoesNotThrow( + () -> parser.parseSingle("GRANT SELECT_PRIV ON `internal`.``.`` TO 'user1'")); + } + + @Test + public void testModifyColumnRoundTripPreservesNullabilityIntent() { + ModifyColumnOp omitted = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.metric BIGINT", + ModifyColumnOp.class, "info.metric"); + Assertions.assertFalse(omitted.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + Assertions.assertFalse(omitted.toSql().contains(" BIGINT NULL ")); + + ModifyColumnOp reparsedOmitted = assertSingleClausePath( + "ALTER TABLE t " + omitted.toSql(), ModifyColumnOp.class, "info.metric"); + Assertions.assertFalse(reparsedOmitted.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + + ModifyColumnOp nullable = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.metric BIGINT NULL", + ModifyColumnOp.class, "info.metric"); + ModifyColumnOp reparsedNullable = assertSingleClausePath( + "ALTER TABLE t " + nullable.toSql(), ModifyColumnOp.class, "info.metric"); + Assertions.assertTrue(reparsedNullable.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + } + + @Test + public void testModifyColumnRoundTripPreservesCommentIntent() { + ModifyColumnOp omitted = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN arr.element BIGINT", + ModifyColumnOp.class, "arr.element"); + Assertions.assertFalse(omitted.getColumnDef() + .translateToCatalogStyleForSchemaChange().isCommentSpecified()); + Assertions.assertFalse(omitted.toSql().contains(" COMMENT ")); + + ModifyColumnOp reparsedOmitted = assertSingleClausePath( + "ALTER TABLE t " + omitted.toSql(), ModifyColumnOp.class, "arr.element"); + Assertions.assertFalse(reparsedOmitted.getColumnDef() + .translateToCatalogStyleForSchemaChange().isCommentSpecified()); + + ModifyColumnOp empty = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN arr.element BIGINT COMMENT ''", + ModifyColumnOp.class, "arr.element"); + ModifyColumnOp reparsedEmpty = assertSingleClausePath( + "ALTER TABLE t " + empty.toSql(), ModifyColumnOp.class, "arr.element"); + Assertions.assertTrue(reparsedEmpty.getColumnDef() + .translateToCatalogStyleForSchemaChange().isCommentSpecified()); + Assertions.assertEquals("", reparsedEmpty.getColumnDef().getComment()); + } + + @Test + public void testStructMemberRoundTripPreservesCommentIntent() { + ModifyColumnOp modify = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.payload " + + "STRUCT", + ModifyColumnOp.class, "info.payload"); + StructType structType = (StructType) modify.getColumnDef().getType(); + Assertions.assertFalse(structType.getFields().get(0).isCommentSpecified()); + Assertions.assertTrue(structType.getFields().get(1).isCommentSpecified()); + + ModifyColumnOp reparsed = assertSingleClausePath( + "ALTER TABLE t " + modify.toSql(), ModifyColumnOp.class, "info.payload"); + StructType reparsedType = (StructType) reparsed.getColumnDef().getType(); + Assertions.assertFalse(reparsedType.getFields().get(0).isCommentSpecified()); + Assertions.assertTrue(reparsedType.getFields().get(1).isCommentSpecified()); + Assertions.assertEquals("", reparsedType.getFields().get(1).getComment()); + org.apache.doris.catalog.StructType catalogType = (org.apache.doris.catalog.StructType) reparsed + .getColumnDef().translateToCatalogStyleForSchemaChange().getType(); + Assertions.assertFalse(catalogType.getFields().get(0).isCommentSpecified()); + Assertions.assertTrue(catalogType.getFields().get(1).isCommentSpecified()); + } + + @Test + public void testModifyColumnCommentRoundTripEscapesQuotesAndBackslashes() { + assertCommentRoundTrip(false); + assertCommentRoundTrip(true); + } + + @Test + public void testColumnDefinitionCommentRoundTripEscapesQuotesAndBackslashes() { + assertColumnDefinitionCommentRoundTrip(false); + assertColumnDefinitionCommentRoundTrip(true); + } + + @Test + public void testRegularColumnDefinitionCommentRoundTripEscapesQuotesAndBackslashes() { + assertRegularColumnDefinitionCommentRoundTrip(false); + assertRegularColumnDefinitionCommentRoundTrip(true); + } + + @Test + public void testStructMemberCommentRoundTripEscapesQuotesAndBackslashes() { + assertStructMemberCommentRoundTrip(false); + assertStructMemberCommentRoundTrip(true); + } + + private void assertStructMemberCommentRoundTrip(boolean noBackslashEscapes) { + try (MockedStatic mockedSqlMode = Mockito.mockStatic(SqlModeHelper.class)) { + mockedSqlMode.when(SqlModeHelper::hasNoBackSlashEscapes).thenReturn(noBackslashEscapes); + + String sqlPath = noBackslashEscapes ? "C:\\tmp\\" : "C:\\\\tmp\\\\"; + String expectedComment = "owner's \"field\" C:\\tmp\\"; + ModifyColumnOp modify = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.payload " + + "STRUCT", + ModifyColumnOp.class, "info.payload"); + StructType structType = (StructType) modify.getColumnDef().getType(); + Assertions.assertEquals(expectedComment, structType.getFields().get(0).getComment()); + + ModifyColumnOp reparsedModify = assertSingleClausePath( + "ALTER TABLE t " + modify.toSql(), ModifyColumnOp.class, "info.payload"); + StructType reparsedType = (StructType) reparsedModify.getColumnDef().getType(); + Assertions.assertEquals(expectedComment, reparsedType.getFields().get(0).getComment()); + } + } + + private void assertStructMemberNames(StructType structType) { + Assertions.assertEquals("key", structType.getFields().get(0).getName()); + Assertions.assertEquals("metric`name", structType.getFields().get(1).getName()); + } + + private void assertRegularColumnDefinitionCommentRoundTrip(boolean noBackslashEscapes) { + try (MockedStatic mockedSqlMode = Mockito.mockStatic(SqlModeHelper.class)) { + mockedSqlMode.when(SqlModeHelper::hasNoBackSlashEscapes).thenReturn(noBackslashEscapes); + + String sqlPath = noBackslashEscapes ? "C:\\tmp\\" : "C:\\\\tmp\\\\"; + String expectedComment = "owner's \"field\" C:\\tmp\\"; + AddColumnsOp addColumns = assertSingleClause( + "ALTER TABLE t ADD COLUMN (owner STRING NULL COMMENT 'owner''s \"field\" " + + sqlPath + "', metric INT NULL)", AddColumnsOp.class); + Assertions.assertEquals(expectedComment, + addColumns.getColumnDefinitions().get(0).getComment()); + + AddColumnsOp reparsedAddColumns = assertSingleClause( + "ALTER TABLE t " + addColumns.toSql(), AddColumnsOp.class); + Assertions.assertEquals(expectedComment, + reparsedAddColumns.getColumnDefinitions().get(0).getComment()); + } + } + + private void assertColumnDefinitionCommentRoundTrip(boolean noBackslashEscapes) { + try (MockedStatic mockedSqlMode = Mockito.mockStatic(SqlModeHelper.class)) { + mockedSqlMode.when(SqlModeHelper::hasNoBackSlashEscapes).thenReturn(noBackslashEscapes); + + String sqlPath = noBackslashEscapes ? "C:\\tmp\\" : "C:\\\\tmp\\\\"; + String expectedComment = "owner's \"field\" C:\\tmp\\"; + AddColumnOp add = assertSingleClausePath( + "ALTER TABLE t ADD COLUMN info.owner STRING NULL COMMENT 'owner''s \"field\" " + + sqlPath + "'", + AddColumnOp.class, "info.owner"); + Assertions.assertEquals(expectedComment, add.getColumnDef().getComment()); + AddColumnOp reparsedAdd = assertSingleClausePath( + "ALTER TABLE t " + add.toSql(), AddColumnOp.class, "info.owner"); + Assertions.assertEquals(expectedComment, reparsedAdd.getColumnDef().getComment()); + + ModifyColumnOp modify = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.owner STRING COMMENT 'owner''s \"field\" " + + sqlPath + "'", + ModifyColumnOp.class, "info.owner"); + Assertions.assertEquals(expectedComment, modify.getColumnDef().getComment()); + ModifyColumnOp reparsedModify = assertSingleClausePath( + "ALTER TABLE t " + modify.toSql(), ModifyColumnOp.class, "info.owner"); + Assertions.assertEquals(expectedComment, reparsedModify.getColumnDef().getComment()); + } + } + + private void assertCommentRoundTrip(boolean noBackslashEscapes) { + try (MockedStatic mockedSqlMode = Mockito.mockStatic(SqlModeHelper.class)) { + mockedSqlMode.when(SqlModeHelper::hasNoBackSlashEscapes).thenReturn(noBackslashEscapes); + + String expectedComment = "owner's \"field\" C:\\tmp\\"; + ModifyColumnCommentOp comment = new ModifyColumnCommentOp( + ColumnPath.fromDotName("info.metric"), expectedComment); + String renderedSql = comment.toSql(); + Assertions.assertTrue(renderedSql.contains("\"\"field\"\"")); + Assertions.assertTrue(renderedSql.contains(noBackslashEscapes + ? "C:\\tmp\\" : "C:\\\\tmp\\\\")); + + ModifyColumnCommentOp reparsed = assertSingleClausePath( + "ALTER TABLE t " + renderedSql, ModifyColumnCommentOp.class, "info.metric"); + Assertions.assertEquals(expectedComment, reparsed.getComment()); + + ModifyColumnCommentOp doubledSingleQuote = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.metric COMMENT 'owner''s'", + ModifyColumnCommentOp.class, "info.metric"); + Assertions.assertEquals("owner's", doubledSingleQuote.getComment()); + ModifyColumnCommentOp doubledDoubleQuote = assertSingleClausePath( + "ALTER TABLE t MODIFY COLUMN info.metric COMMENT \"a\"\"b\"", + ModifyColumnCommentOp.class, "info.metric"); + Assertions.assertEquals("a\"b", doubledDoubleQuote.getComment()); + } + } + + private T assertSingleClausePath(String sql, Class clauseClass, + String expectedPath) { + T clause = assertSingleClause(sql, clauseClass); + if (clause instanceof AddColumnOp) { + Assertions.assertEquals(expectedPath, ((AddColumnOp) clause).getColumnPath().getFullPath()); + } else if (clause instanceof ModifyColumnOp) { + Assertions.assertEquals(expectedPath, ((ModifyColumnOp) clause).getColumnPath().getFullPath()); + } else if (clause instanceof ModifyColumnCommentOp) { + Assertions.assertEquals(expectedPath, ((ModifyColumnCommentOp) clause).getColumnPath().getFullPath()); + } else if (clause instanceof DropColumnOp) { + Assertions.assertEquals(expectedPath, ((DropColumnOp) clause).getColumnPath().getFullPath()); + } else if (clause instanceof RenameColumnOp) { + Assertions.assertEquals(expectedPath, ((RenameColumnOp) clause).getColumnPath().getFullPath()); + } + return clause; + } + + private T assertSingleClause(String sql, Class clauseClass) { + Plan plan = parser.parseSingle(sql); + Assertions.assertInstanceOf(AlterTableCommand.class, plan); + List clauses = ((AlterTableCommand) plan).getOps(); + Assertions.assertEquals(1, clauses.size()); + AlterTableOp clause = clauses.get(0); + Assertions.assertInstanceOf(clauseClass, clause); + return clauseClass.cast(clause); + } +} diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java index efbe72a75acecd..3c92606e4e3fcc 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/PruneNestedColumnTest.java @@ -1022,15 +1022,24 @@ public void testDataTypeAccessTree() { Assertions.assertEquals("struct>>>", columnType.toSql()); setAccessPathAndAssertType(slot, ImmutableList.of("s", "city"), "STRUCT"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "KEYS"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "VALUES"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "VALUES", "a"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "VALUES", "b"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "*"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "*", "a"), "STRUCT>>>"); - setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "*", "b"), "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "KEYS"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "VALUES"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "VALUES", "a"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "VALUES", "b"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "*"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "*", "a"), + "STRUCT>>>"); + setAccessPathAndAssertType(slot, ImmutableList.of("s", "data", "*", "*", "b"), + "STRUCT>>>"); setAccessPathsAndAssertType(slot, ImmutableList.of( diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommandTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommandTest.java index db64fe354d5cc3..f248e4382a0436 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommandTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommandTest.java @@ -17,22 +17,38 @@ package org.apache.doris.nereids.trees.plans.commands; +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.TableIf; import org.apache.doris.catalog.info.TableNameInfo; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.CatalogMgr; +import org.apache.doris.datasource.iceberg.IcebergExternalTable; +import org.apache.doris.nereids.parser.NereidsParser; +import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.commands.info.AddColumnsOp; import org.apache.doris.nereids.trees.plans.commands.info.AddPartitionFieldOp; import org.apache.doris.nereids.trees.plans.commands.info.AlterTableOp; import org.apache.doris.nereids.trees.plans.commands.info.DropPartitionFieldOp; import org.apache.doris.nereids.trees.plans.commands.info.EnableFeatureOp; +import org.apache.doris.nereids.trees.plans.commands.info.ModifyColumnOp; import org.apache.doris.nereids.trees.plans.commands.info.ReplacePartitionFieldOp; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; public class AlterTableCommandTest { + private final NereidsParser parser = new NereidsParser(); + @Test void testEnableFeatureOp() { List ops = new ArrayList<>(); @@ -155,4 +171,251 @@ void testReplacePartitionFieldOp() { "ALTER TABLE `internal`.`db`.`test` REPLACE PARTITION KEY bucket(16, id) WITH truncate(5, code) AS code_trunc", alterTableCommand.toSql()); } + + @Test + void testRejectNestedColumnPathForNonIcebergTable() { + TableIf table = Mockito.mock(TableIf.class); + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN s.c STRING NULL", + "ALTER TABLE t MODIFY COLUMN s.a BIGINT", + "ALTER TABLE t MODIFY COLUMN s.a COMMENT 'nested comment'", + "ALTER TABLE t DROP COLUMN s.c", + "ALTER TABLE t RENAME COLUMN s.c TO c2")) { + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter(sql).getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("Nested column path is only supported for Iceberg tables")); + } + } + + @Test + void testAllowNestedColumnPathForIcebergTable() throws AnalysisException { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + AlterTableCommand.checkColumnOperationsSupported(table, + parseAlter("ALTER TABLE t ADD COLUMN s.c STRING NULL").getOps()); + } + + @Test + void testRejectRequiredNestedColumnForIcebergTable() { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, + parseAlter("ALTER TABLE t ADD COLUMN s.required_field INT NOT NULL").getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("New nested field 's.required_field' must be nullable")); + } + + @Test + void testRejectRollupForIcebergColumnOperations() { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN c STRING NULL TO r1", + "ALTER TABLE t ADD COLUMN s.c STRING NULL TO r1", + "ALTER TABLE t ADD COLUMN (c1 STRING NULL, c2 INT NULL) IN r1", + "ALTER TABLE t DROP COLUMN c FROM r1", + "ALTER TABLE t DROP COLUMN s.c FROM r1", + "ALTER TABLE t MODIFY COLUMN c STRING FROM r1", + "ALTER TABLE t MODIFY COLUMN s.c STRING FROM r1", + "ALTER TABLE t ORDER BY (c1, c2) FROM r1")) { + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter(sql).getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("Rollup is not supported for Iceberg column operations")); + } + } + + @Test + void testRejectPropertiesForIcebergColumnOperations() { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN c STRING NULL PROPERTIES ('k' = 'v')", + "ALTER TABLE t ADD COLUMN s.c STRING NULL PROPERTIES ('k' = 'v')", + "ALTER TABLE t ADD COLUMN (c1 STRING NULL, c2 INT NULL) PROPERTIES ('k' = 'v')", + "ALTER TABLE t DROP COLUMN c PROPERTIES ('k' = 'v')", + "ALTER TABLE t DROP COLUMN s.c PROPERTIES ('k' = 'v')", + "ALTER TABLE t MODIFY COLUMN c STRING PROPERTIES ('k' = 'v')", + "ALTER TABLE t MODIFY COLUMN s.c STRING PROPERTIES ('k' = 'v')", + "ALTER TABLE t ORDER BY (c1, c2) PROPERTIES ('k' = 'v')")) { + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter(sql).getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("PROPERTIES are not supported for Iceberg column operations")); + } + } + + @Test + void testRejectKeyForIcebergAddAndModify() { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN c INT KEY NULL", + "ALTER TABLE t ADD COLUMN s.c INT KEY NULL", + "ALTER TABLE t ADD COLUMN (c1 INT KEY NULL, c2 INT NULL)", + "ALTER TABLE t MODIFY COLUMN c BIGINT KEY", + "ALTER TABLE t MODIFY COLUMN s.c BIGINT KEY")) { + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter(sql).getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("KEY is not supported for Iceberg ADD/MODIFY COLUMN")); + } + } + + @Test + void testRejectGeneratedColumnForIcebergAddAndModify() { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN c INT AS (id + 1) NULL", + "ALTER TABLE t ADD COLUMN s.c INT AS (id + 1) NULL", + "ALTER TABLE t ADD COLUMN (c1 INT AS (id + 1) NULL, c2 INT NULL)", + "ALTER TABLE t MODIFY COLUMN c BIGINT AS (id + 1)", + "ALTER TABLE t MODIFY COLUMN s.c BIGINT AS (id + 1)")) { + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter(sql).getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("Generated columns are not supported for Iceberg ADD/MODIFY COLUMN")); + } + } + + @Test + void testRejectUnsupportedDefaultChangesForIcebergTable() throws AnalysisException { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + for (String sql : Arrays.asList( + "ALTER TABLE t MODIFY COLUMN c BIGINT DEFAULT 7", + "ALTER TABLE t MODIFY COLUMN c BIGINT DEFAULT NULL", + "ALTER TABLE t MODIFY COLUMN ts DATETIME DEFAULT CURRENT_TIMESTAMP " + + "ON UPDATE CURRENT_TIMESTAMP")) { + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter(sql).getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("Modifying default values is not supported for Iceberg columns")); + } + + org.apache.doris.nereids.exceptions.AnalysisException complexDefaultException = Assertions.assertThrows( + org.apache.doris.nereids.exceptions.AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter( + "ALTER TABLE t MODIFY COLUMN st STRUCT DEFAULT 'x'").getOps())); + Assertions.assertTrue(complexDefaultException.getMessage() + .contains("Struct type column default value just support null")); + + AnalysisException onUpdateException = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter( + "ALTER TABLE t ADD COLUMN ts DATETIME NULL DEFAULT CURRENT_TIMESTAMP " + + "ON UPDATE CURRENT_TIMESTAMP").getOps())); + Assertions.assertTrue(onUpdateException.getMessage() + .contains("ON UPDATE is not supported for Iceberg ADD COLUMN")); + + AlterTableCommand.checkColumnOperationsSupported(table, + parseAlter("ALTER TABLE t ADD COLUMN c BIGINT NULL DEFAULT 7").getOps()); + } + + @Test + void testRejectCompoundIcebergColumnOperations() throws AnalysisException { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN s.good INT NULL, DROP COLUMN m.value.x", + "ALTER TABLE t MODIFY COLUMN c COMMENT 'new comment', ADD COLUMN d INT NULL", + "ALTER TABLE t ADD COLUMN c INT NULL, DROP COLUMN d", + "ALTER TABLE t RENAME COLUMN c TO c2, RENAME COLUMN d TO d2", + "ALTER TABLE t MODIFY COLUMN c COMMENT 'c', MODIFY COLUMN d COMMENT 'd'", + "ALTER TABLE t ORDER BY (c, d), ORDER BY (d, c)")) { + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> AlterTableCommand.checkColumnOperationsSupported(table, parseAlter(sql).getOps())); + Assertions.assertTrue(exception.getMessage() + .contains("Multiple Iceberg ALTER clauses are not supported")); + } + + AlterTableCommand.checkColumnOperationsSupported(table, + parseAlter("ALTER TABLE t ADD COLUMN (c1 INT NULL, c2 BIGINT NULL)").getOps()); + } + + @Test + void testPreserveEmptyAddColumnsValidationForIcebergTable() throws AnalysisException { + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + AddColumnsOp addColumnsOp = new AddColumnsOp(null, null, new HashMap<>()); + + AlterTableCommand.checkColumnOperationsSupported(table, Arrays.asList(addColumnsOp)); + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> addColumnsOp.validate(null)); + Assertions.assertTrue(exception.getMessage().contains("Columns is empty in add columns clause")); + } + + @Test + void testModifyColumnTracksExplicitNullability() { + ModifyColumnOp omitted = (ModifyColumnOp) parseAlter( + "ALTER TABLE t MODIFY COLUMN s.a BIGINT").getOps().get(0); + ModifyColumnOp nullable = (ModifyColumnOp) parseAlter( + "ALTER TABLE t MODIFY COLUMN s.a BIGINT NULL").getOps().get(0); + ModifyColumnOp notNullable = (ModifyColumnOp) parseAlter( + "ALTER TABLE t MODIFY COLUMN s.a BIGINT NOT NULL").getOps().get(0); + + Assertions.assertFalse(omitted.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + Assertions.assertTrue(nullable.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + Assertions.assertTrue(notNullable.getColumnDef() + .translateToCatalogStyleForSchemaChange().isNullableSpecified()); + } + + @Test + void testNestedIcebergColumnNamesBypassTopLevelSystemPrefixes() throws Exception { + Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + DatabaseIf database = Mockito.mock(DatabaseIf.class); + IcebergExternalTable table = Mockito.mock(IcebergExternalTable.class); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(catalogMgr.getCatalogOrDdlException("iceberg")).thenReturn(catalog); + Mockito.when(catalog.getDbOrDdlException("db")).thenReturn(database); + Mockito.when(database.getTableOrDdlException("t")).thenReturn(table); + + try (MockedStatic mockedEnv = Mockito.mockStatic(Env.class)) { + mockedEnv.when(Env::getCurrentEnv).thenReturn(env); + + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN s.__DORIS_metric INT NULL", + "ALTER TABLE t ADD COLUMN s.__doris_shadow_metric INT NULL", + "ALTER TABLE t MODIFY COLUMN s.__DORIS_metric BIGINT", + "ALTER TABLE t MODIFY COLUMN s.__doris_shadow_metric BIGINT", + "ALTER TABLE t RENAME COLUMN s.__DORIS_metric TO metric", + "ALTER TABLE t RENAME COLUMN s.metric TO __doris_shadow_metric", + "ALTER TABLE t ADD COLUMN s._row_id BIGINT NULL", + "ALTER TABLE t MODIFY COLUMN s._row_id BIGINT", + "ALTER TABLE t RENAME COLUMN s.metric TO _last_updated_sequence_number", + "ALTER TABLE t DROP COLUMN s.__DORIS_metric")) { + validateIcebergAlter(sql, table); + } + + for (String sql : Arrays.asList( + "ALTER TABLE t ADD COLUMN __DORIS_metric INT NULL", + "ALTER TABLE t ADD COLUMN __doris_shadow_metric INT NULL", + "ALTER TABLE t MODIFY COLUMN __DORIS_metric BIGINT")) { + org.apache.doris.nereids.exceptions.AnalysisException exception = Assertions.assertThrows( + org.apache.doris.nereids.exceptions.AnalysisException.class, + () -> validateIcebergAlter(sql, table)); + Assertions.assertTrue(exception.getMessage().contains("Incorrect column name")); + } + + AnalysisException renameException = Assertions.assertThrows(AnalysisException.class, + () -> validateIcebergAlter( + "ALTER TABLE t RENAME COLUMN metric TO __DORIS_metric", table)); + Assertions.assertTrue(renameException.getMessage().contains("Incorrect column name")); + AnalysisException dropException = Assertions.assertThrows(AnalysisException.class, + () -> validateIcebergAlter("ALTER TABLE t DROP COLUMN __DORIS_metric", table)); + Assertions.assertTrue(dropException.getMessage().contains("Do not support drop hidden column")); + } + } + + private void validateIcebergAlter(String sql, IcebergExternalTable table) throws Exception { + AlterTableCommand command = parseAlter(sql); + AlterTableCommand.checkColumnOperationsSupported(table, command.getOps()); + for (AlterTableOp op : command.getOps()) { + op.setTableName(new TableNameInfo("iceberg", "db", "t")); + op.validate(null); + } + } + + private AlterTableCommand parseAlter(String sql) { + Plan plan = parser.parseSingle(sql); + Assertions.assertInstanceOf(AlterTableCommand.class, plan); + return (AlterTableCommand) plan; + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinitionTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinitionTest.java index f20925011a4c44..6cbb4073df2d50 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinitionTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinitionTest.java @@ -17,6 +17,8 @@ package org.apache.doris.nereids.trees.plans.commands.info; +import org.apache.doris.nereids.types.StringType; + import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -33,4 +35,12 @@ public void testNameEquals() { boolean expected2 = false; Assertions.assertEquals(expected2, columnDefinition.nameEquals(otherColName2, false)); } + + @Test + public void testToSqlHandlesNullComment() { + ColumnDefinition columnDefinition = new ColumnDefinition("col1", StringType.INSTANCE, true, null); + + String sql = columnDefinition.toSql(); + Assertions.assertTrue(sql.endsWith("COMMENT \"\"")); + } } diff --git a/fe/fe-sql-parser/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 b/fe/fe-sql-parser/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 index cd2391b1775f1e..b300b6185c8730 100644 --- a/fe/fe-sql-parser/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 +++ b/fe/fe-sql-parser/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4 @@ -771,11 +771,11 @@ addRollupClause ; alterTableClause - : ADD COLUMN columnDef columnPosition? toRollup? properties=propertyClause? #addColumnClause + : ADD COLUMN columnDefWithPath columnPosition? toRollup? properties=propertyClause? #addColumnClause | ADD COLUMN LEFT_PAREN columnDefs RIGHT_PAREN toRollup? properties=propertyClause? #addColumnsClause - | DROP COLUMN name=identifier fromRollup? properties=propertyClause? #dropColumnClause - | MODIFY COLUMN columnDef columnPosition? fromRollup? + | DROP COLUMN name=qualifiedName fromRollup? properties=propertyClause? #dropColumnClause + | MODIFY COLUMN columnDefWithPath columnPosition? fromRollup? properties=propertyClause? #modifyColumnClause | ORDER BY identifierList fromRollup? properties=propertyClause? #reorderColumnsClause | ADD TEMPORARY? partitionDef @@ -794,14 +794,14 @@ alterTableClause | RENAME newName=identifier #renameClause | RENAME ROLLUP name=identifier newName=identifier #renameRollupClause | RENAME PARTITION name=identifier newName=identifier #renamePartitionClause - | RENAME COLUMN name=identifier newName=identifier #renameColumnClause + | RENAME COLUMN name=qualifiedName TO? newName=identifier #renameColumnClause | ADD indexDef #addIndexClause | DROP INDEX (IF EXISTS)? name=identifier partitionSpec? #dropIndexClause | ENABLE FEATURE name=STRING_LITERAL (WITH properties=propertyClause)? #enableFeatureClause | MODIFY DISTRIBUTION (DISTRIBUTED BY (HASH hashKeys=identifierList | RANDOM) (BUCKETS (INTEGER_VALUE | autoBucket=AUTO))?)? #modifyDistributionClause | MODIFY COMMENT comment=STRING_LITERAL #modifyTableCommentClause - | MODIFY COLUMN name=identifier COMMENT comment=STRING_LITERAL #modifyColumnCommentClause + | MODIFY COLUMN name=qualifiedName COMMENT comment=STRING_LITERAL #modifyColumnCommentClause | MODIFY ENGINE TO name=identifier properties=propertyClause? #modifyEngineClause | ADD TEMPORARY? PARTITIONS FROM from=partitionValueList TO to=partitionValueList @@ -1554,6 +1554,17 @@ columnDef (COMMENT comment=STRING_LITERAL)? ; +columnDefWithPath + : columnDef + | colNames+=identifier (DOT colNames+=identifier)+ type=dataType + KEY? + (aggType=aggTypeDef)? + ((GENERATED ALWAYS)? AS LEFT_PAREN generatedExpr=expression RIGHT_PAREN)? + ((NOT)? nullable=NULL)? + (AUTO_INCREMENT (LEFT_PAREN autoIncInitValue=number RIGHT_PAREN)?)? + (COMMENT comment=STRING_LITERAL)? + ; + indexDefs : indexes+=indexDef (COMMA indexes+=indexDef)* ; 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 ea8124c8aeb234..e9432c1efad1c5 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 @@ -40,13 +40,22 @@ public class StructField { @SerializedName(value = "containsNull") private final boolean containsNull; // Now always true (nullable field) + // Runtime-only schema change intent; do not persist it as part of the table schema. + private transient boolean commentSpecified; + public static final String DEFAULT_FIELD_NAME = "col"; public StructField(String name, Type type, String comment, boolean containsNull) { + this(name, type, comment, containsNull, !Strings.isNullOrEmpty(comment)); + } + + public StructField(String name, Type type, String comment, boolean containsNull, + boolean commentSpecified) { this.name = name.toLowerCase(); this.type = type; this.comment = comment; this.containsNull = containsNull; + this.commentSpecified = commentSpecified; } public StructField(String name, Type type) { @@ -65,6 +74,10 @@ public String getComment() { return comment; } + public boolean isCommentSpecified() { + return commentSpecified || !Strings.isNullOrEmpty(comment); + } + public String getName() { return name; } @@ -96,7 +109,7 @@ public String toSql(int depth) { if (type != null) { sb.append(":").append(typeSql); } - if (!Strings.isNullOrEmpty(comment)) { + if (isCommentSpecified()) { sb.append(String.format(" comment '%s'", comment)); } return sb.toString(); @@ -116,7 +129,7 @@ public String prettyPrint(int lpad) { typeStr = typeStr.substring(lpad); sb.append(":").append(typeStr); } - if (!Strings.isNullOrEmpty(comment)) { + if (isCommentSpecified()) { sb.append(String.format(" COMMENT '%s'", comment)); } return sb.toString(); @@ -153,7 +166,7 @@ public String toString() { if (type != null) { sb.append(":").append(type); } - if (!Strings.isNullOrEmpty(comment)) { + if (isCommentSpecified()) { sb.append(String.format(" COMMENT '%s'", comment)); } return sb.toString(); diff --git a/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out b/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out index dab31ada31b925..4565871aa8aa71 100644 --- a/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out +++ b/regression-test/data/external_table_p0/iceberg/iceberg_schema_change_ddl.out @@ -149,7 +149,7 @@ phone text Yes true \N User phone number grade double Yes true \N email text Yes true \N address struct Yes true \N -col1 double Yes true \N +col1 double Yes true \N Updated column1 type col2 text Yes true \N User defined column2 -- !after_no_comment -- @@ -329,4 +329,3 @@ department_id bigint Yes true \N 2 Bob 30 9223372036854775806 3 Charlie 22 100 3 Charlie 22 9223372036854775805 - diff --git a/regression-test/data/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_ddl.out b/regression-test/data/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_ddl.out new file mode 100644 index 00000000000000..93826ea80b7925 --- /dev/null +++ b/regression-test/data/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_ddl.out @@ -0,0 +1,12 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !desc -- +id int(11) +s struct +arr array> +m map> +arr_scalar array +m_scalar map + +-- !query_rows -- +1 \N 10 \N \N 100 \N \N 1000 \N \N 7 70 +2 first 20 after_a c2 200 202 201 2000 2002 2001 8 80 diff --git a/regression-test/data/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_spark_doris_interop.out b/regression-test/data/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_spark_doris_interop.out new file mode 100644 index 00000000000000..54a02e79582045 --- /dev/null +++ b/regression-test/data/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_spark_doris_interop.out @@ -0,0 +1,27 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !doris_driven_rows -- +1 \N 10 \N doris_before \N \N 100 \N \N \N 1000 \N \N +2 spark_first 20 spark_after_metric spark_after_doris spark_can_write_doris_field 202 200 203 201 2002 2000 2003 2001 + +-- !spark_driven_schema -- +id int(11) +info struct +events array> +attrs map> + +-- !spark_driven_rows_before_write -- +1 \N 10 \N spark_before \N \N 100 \N \N \N 1000 \N \N +2 spark_first_field 20 spark_after_metric_field spark_after spark_new_field 202 200 203 201 2002 2000 2003 2001 + +-- !spark_driven_rows_after_write -- +1 \N 10 \N spark_before \N \N 100 \N \N \N 1000 \N \N +2 spark_first_field 20 spark_after_metric_field spark_after spark_new_field 202 200 203 201 2002 2000 2003 2001 +3 doris_first_field 30 doris_after_metric_field doris_after_spark doris_can_write_spark_field 302 300 303 301 3002 3000 3003 3001 + +-- !required_nested_rows -- +1 10 old-label +2 20 \N + +-- !deep_nested_rows -- +1 last-old middle-old first-old old-note 1.5 12.34 +2 last-new middle-new first-new new-note 2.5 56.78 diff --git a/regression-test/suites/compaction/test_table_level_compaction_policy.groovy b/regression-test/suites/compaction/test_table_level_compaction_policy.groovy index 5edff4e38e0da5..0e1c70b02c9eea 100644 --- a/regression-test/suites/compaction/test_table_level_compaction_policy.groovy +++ b/regression-test/suites/compaction/test_table_level_compaction_policy.groovy @@ -219,22 +219,22 @@ suite("test_table_level_compaction_policy") { exception "only time series compaction policy support for time series config" } - test { - sql """ - CREATE TABLE ${tableName} ( - `c_custkey` int(11) NOT NULL COMMENT "", - `c_name` varchar(26) NOT NULL COMMENT "", - `c_address` varchar(41) NOT NULL COMMENT "", - `c_city` varchar(11) NOT NULL COMMENT "" - ) - DUPLICATE KEY (`c_custkey`) - DISTRIBUTED BY HASH(`c_custkey`) BUCKETS 1 - PROPERTIES ( - "replication_num" = "1" - ); - """ - sql """sync""" + sql """ + CREATE TABLE ${tableName} ( + `c_custkey` int(11) NOT NULL COMMENT "", + `c_name` varchar(26) NOT NULL COMMENT "", + `c_address` varchar(41) NOT NULL COMMENT "", + `c_city` varchar(11) NOT NULL COMMENT "" + ) + DUPLICATE KEY (`c_custkey`) + DISTRIBUTED BY HASH(`c_custkey`) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1" + ); + """ + sql """sync""" + test { sql """ alter table ${tableName} set ("compaction_policy" = "ok") """ diff --git a/regression-test/suites/doc/sql-manual/sql-function/test_array_function.groovy b/regression-test/suites/doc/sql-manual/sql-function/test_array_function.groovy index 6f9f1f7b0e0ca4..f20bfb60f38aa9 100644 --- a/regression-test/suites/doc/sql-manual/sql-function/test_array_function.groovy +++ b/regression-test/suites/doc/sql-manual/sql-function/test_array_function.groovy @@ -520,4 +520,4 @@ suite("test_array_function_doc", "p0") { qt_sql """ SELECT ARRAY_SORTBY(x -> x[1], [[1,2],[3,4]]); """ qt_sql """ SELECT ARRAY_SORT([[1,2],[3,4]]); """ qt_sql """ SELECT ARRAY_REVERSE_SORT([[1,2],[3,4]]); """ -} \ No newline at end of file +} diff --git a/regression-test/suites/external_table_p0/iceberg/iceberg_schema_change_ddl.groovy b/regression-test/suites/external_table_p0/iceberg/iceberg_schema_change_ddl.groovy index 93c8faf1ce522d..7d7672a4b3e6cc 100644 --- a/regression-test/suites/external_table_p0/iceberg/iceberg_schema_change_ddl.groovy +++ b/regression-test/suites/external_table_p0/iceberg/iceberg_schema_change_ddl.groovy @@ -193,7 +193,7 @@ suite("iceberg_schema_change_ddl", "p0,external") { sql """ ALTER TABLE ${table_name} MODIFY COLUMN non_col bigint""" exception "Column non_col does not exist" } - // not comment, the comment will be removed + // Omitted COMMENT preserves the existing comment. qt_before_no_comment "desc ${table_name}" sql """ ALTER TABLE ${table_name} MODIFY COLUMN col1 DOUBLE""" qt_after_no_comment "desc ${table_name}" @@ -305,7 +305,7 @@ suite("iceberg_schema_change_ddl", "p0,external") { // struct/complex type changes test { sql """ ALTER TABLE ${table_name} MODIFY COLUMN address STRING """ - exception "Cannot change column type" + exception "Modify column type from complex to primitive is not supported" } test { diff --git a/regression-test/suites/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_ddl.groovy b/regression-test/suites/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_ddl.groovy new file mode 100644 index 00000000000000..012bc88e3d9faf --- /dev/null +++ b/regression-test/suites/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_ddl.groovy @@ -0,0 +1,175 @@ +// 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. + +suite("test_iceberg_nested_schema_evolution_ddl", "p0,external,doris,external_docker,external_docker_doris") { + + String enabled = context.config.otherConfigs.get("enableIcebergTest") + if (enabled == null || !enabled.equalsIgnoreCase("true")) { + logger.info("disable iceberg test.") + return + } + + String restPort = context.config.otherConfigs.get("iceberg_rest_uri_port") + String minioPort = context.config.otherConfigs.get("iceberg_minio_port") + String externalEnvIp = context.config.otherConfigs.get("externalEnvIp") + String catalogName = "test_iceberg_nested_schema_evolution_ddl" + String dbName = "iceberg_nested_schema_evolution_db" + String tableName = "iceberg_nested_evolution" + + sql """drop catalog if exists ${catalogName}""" + sql """ + CREATE CATALOG ${catalogName} PROPERTIES ( + 'type'='iceberg', + 'iceberg.catalog.type'='rest', + 'uri' = 'http://${externalEnvIp}:${restPort}', + "s3.access_key" = "admin", + "s3.secret_key" = "password", + "s3.endpoint" = "http://${externalEnvIp}:${minioPort}", + "s3.region" = "us-east-1" + );""" + + sql """switch ${catalogName};""" + sql """drop database if exists ${dbName} force""" + sql """create database ${dbName}""" + sql """use ${dbName};""" + + sql """set enable_fallback_to_original_planner=false;""" + + sql """drop table if exists ${tableName}""" + sql """ + CREATE TABLE ${tableName} ( + id INT NOT NULL, + s STRUCT, + arr ARRAY>, + m MAP>, + arr_scalar ARRAY, + m_scalar MAP + ); + """ + + sql """ + INSERT INTO ${tableName} VALUES ( + 1, + STRUCT(10, 'old'), + ARRAY(STRUCT(100)), + MAP('k', STRUCT(1000)), + ARRAY(7), + MAP('k', 70) + ) + """ + + sql """ALTER TABLE ${tableName} ADD COLUMN s.c STRING NULL COMMENT 'new nested field'""" + sql """ALTER TABLE ${tableName} ADD COLUMN s.first_pos STRING NULL FIRST""" + sql """ALTER TABLE ${tableName} ADD COLUMN s.after_a STRING NULL AFTER a""" + sql """ALTER TABLE ${tableName} ADD COLUMN arr.element.y INT NULL""" + sql """ALTER TABLE ${tableName} ADD COLUMN arr.element.after_x INT NULL AFTER x""" + sql """ALTER TABLE ${tableName} ADD COLUMN m.value.y INT NULL""" + sql """ALTER TABLE ${tableName} ADD COLUMN m.value.after_v INT NULL AFTER v""" + sql """ALTER TABLE ${tableName} ADD COLUMN s.drop_me STRING NULL""" + sql """ALTER TABLE ${tableName} ADD COLUMN arr.element.drop_me INT NULL""" + sql """ALTER TABLE ${tableName} ADD COLUMN m.value.drop_me INT NULL""" + + test { + sql """ALTER TABLE ${tableName} ADD COLUMN s.required_field INT NOT NULL""" + exception "New nested field 's.required_field' must be nullable" + } + + sql """ALTER TABLE ${tableName} MODIFY COLUMN s.a BIGINT""" + sql """ALTER TABLE ${tableName} MODIFY COLUMN arr.element.x BIGINT""" + sql """ALTER TABLE ${tableName} MODIFY COLUMN m.value.v BIGINT""" + sql """ALTER TABLE ${tableName} MODIFY COLUMN arr_scalar.element BIGINT""" + sql """ALTER TABLE ${tableName} MODIFY COLUMN m_scalar.value BIGINT""" + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN m_scalar.`key` BIGINT""" + exception "Cannot modify MAP key nested column" + } + + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN arr_scalar.element COMMENT 'array element comment'""" + exception "Iceberg does not support comments on collection element or value fields" + } + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN m_scalar.value COMMENT 'map value comment'""" + exception "Iceberg does not support comments on collection element or value fields" + } + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN arr_scalar.element BIGINT COMMENT 'array element comment'""" + exception "Iceberg does not support comments on collection element or value fields" + } + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN m_scalar.value BIGINT COMMENT 'map value comment'""" + exception "Iceberg does not support comments on collection element or value fields" + } + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN arr_scalar.element BIGINT COMMENT ''""" + exception "Iceberg does not support comments on collection element or value fields" + } + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN m_scalar.value BIGINT COMMENT ''""" + exception "Iceberg does not support comments on collection element or value fields" + } + test { + sql """ALTER TABLE ${tableName} MODIFY COLUMN m_scalar.`key` COMMENT 'map key comment'""" + exception "Cannot modify comment MAP key nested column" + } + + sql """ALTER TABLE ${tableName} RENAME COLUMN s.c TO c2""" + sql """ALTER TABLE ${tableName} RENAME COLUMN arr.element.y TO y2""" + sql """ALTER TABLE ${tableName} RENAME COLUMN m.value.y TO y2""" + sql """ALTER TABLE ${tableName} MODIFY COLUMN s.`c2` COMMENT 'renamed struct field'""" + sql """ALTER TABLE ${tableName} MODIFY COLUMN arr.element.y2 COMMENT 'renamed array element field'""" + sql """ALTER TABLE ${tableName} MODIFY COLUMN m.value.y2 COMMENT 'renamed map value field'""" + sql """ALTER TABLE ${tableName} DROP COLUMN s.drop_me""" + sql """ALTER TABLE ${tableName} DROP COLUMN arr.element.drop_me""" + sql """ALTER TABLE ${tableName} DROP COLUMN m.value.drop_me""" + + sql """ + INSERT INTO ${tableName} VALUES ( + 2, + STRUCT('first', 20, 'after_a', 'new', 'c2'), + ARRAY(STRUCT(200, 202, 201)), + MAP('k', STRUCT(2000, 2002, 2001)), + ARRAY(8), + MAP('k', 80) + ) + """ + + qt_desc """ + SELECT COLUMN_NAME, COLUMN_TYPE + FROM ${catalogName}.information_schema.columns + WHERE TABLE_SCHEMA = '${dbName}' AND TABLE_NAME = '${tableName}' + ORDER BY ORDINAL_POSITION + """ + + order_qt_query_rows """ + SELECT id, + element_at(s, 'first_pos'), + element_at(s, 'a'), + element_at(s, 'after_a'), + element_at(s, 'c2'), + element_at(arr[1], 'x'), + element_at(arr[1], 'after_x'), + element_at(arr[1], 'y2'), + element_at(m['k'], 'v'), + element_at(m['k'], 'after_v'), + element_at(m['k'], 'y2'), + arr_scalar[1], + m_scalar['k'] + FROM ${tableName} + ORDER BY id + """ +} diff --git a/regression-test/suites/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_spark_doris_interop.groovy b/regression-test/suites/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_spark_doris_interop.groovy new file mode 100644 index 00000000000000..96725c2aedd884 --- /dev/null +++ b/regression-test/suites/external_table_p0/iceberg/test_iceberg_nested_schema_evolution_spark_doris_interop.groovy @@ -0,0 +1,553 @@ +// 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. + +import groovy.json.JsonSlurper + +suite("test_iceberg_nested_schema_evolution_spark_doris_interop", + "p0,external,iceberg,external_docker,external_docker_iceberg") { + String enabled = context.config.otherConfigs.get("enableIcebergTest") + if (enabled == null || !enabled.equalsIgnoreCase("true")) { + logger.info("disable iceberg test.") + return + } + + String catalogName = "test_iceberg_nested_schema_evolution_spark_doris_interop" + String dbName = "iceberg_nested_schema_evolution_interop_db" + String dorisTable = "doris_nested_evolution_to_spark" + String commentTable = "doris_nested_comment_semantics" + String sparkTable = "spark_nested_evolution_to_doris" + String mixedCaseTable = "spark_mixed_case_nested_collision" + String requiredTable = "spark_required_nested_evolution" + String advancedTable = "spark_deep_nested_evolution" + String restPort = context.config.otherConfigs.get("iceberg_rest_uri_port") + String minioPort = context.config.otherConfigs.get("iceberg_minio_port") + String externalEnvIp = context.config.otherConfigs.get("externalEnvIp") + + sql """drop catalog if exists ${catalogName}""" + sql """ + CREATE CATALOG ${catalogName} PROPERTIES ( + 'type'='iceberg', + 'iceberg.catalog.type'='rest', + 'uri' = 'http://${externalEnvIp}:${restPort}', + "s3.access_key" = "admin", + "s3.secret_key" = "password", + "s3.endpoint" = "http://${externalEnvIp}:${minioPort}", + "s3.region" = "us-east-1", + "meta.cache.iceberg.table.ttl-second" = "0" + ); + """ + + sql """switch ${catalogName}""" + sql """drop database if exists ${dbName} force""" + sql """create database ${dbName}""" + sql """use ${dbName}""" + sql """set enable_fallback_to_original_planner=false""" + + spark_iceberg """CREATE DATABASE IF NOT EXISTS demo.${dbName}""" + + sql """ + CREATE TABLE ${commentTable} ( + id INT, + info STRUCT< + metric:INT COMMENT 'metric doc', + note:STRING COMMENT 'old note', + clear_me:STRING COMMENT 'clear doc', + payload:STRUCT< + name:STRING COMMENT 'name doc', + count:INT COMMENT 'count doc' + > COMMENT 'payload doc' + > + ) + """ + sql """ALTER TABLE ${commentTable} MODIFY COLUMN info.note COMMENT 'new note'""" + sql """ALTER TABLE ${commentTable} MODIFY COLUMN info.metric BIGINT""" + sql """ALTER TABLE ${commentTable} MODIFY COLUMN info.clear_me STRING COMMENT ''""" + sql """ + ALTER TABLE ${commentTable} MODIFY COLUMN info.payload + STRUCT + """ + + String loadTableUrl = "http://${externalEnvIp}:${restPort}/v1/namespaces/${dbName}/tables/${commentTable}" + def loadTableResponse = new JsonSlurper().parseText(new URL(loadTableUrl).getText("UTF-8")) + def tableMetadata = loadTableResponse.metadata + def currentSchema = tableMetadata.schemas.find { + it["schema-id"] == tableMetadata["current-schema-id"] + } + assertNotNull(currentSchema, "current schema should exist in Iceberg metadata") + def infoColumn = currentSchema.fields.find { it.name == "info" } + assertNotNull(infoColumn, "info column should exist in Iceberg metadata") + def infoFields = infoColumn.type.fields.collectEntries { [(it.name): it] } + assertEquals("long", infoFields.metric.type) + assertEquals("metric doc", infoFields.metric.doc) + assertEquals("new note", infoFields.note.doc) + assertTrue(infoFields.clear_me.doc == null || infoFields.clear_me.doc == "") + assertEquals("payload doc", infoFields.payload.doc) + def payloadFields = infoFields.payload.type.fields.collectEntries { [(it.name): it] } + assertTrue(payloadFields.name.doc == null || payloadFields.name.doc == "") + assertEquals("long", payloadFields.count.type) + assertEquals("count doc", payloadFields.count.doc) + + sql """ + CREATE TABLE ${dorisTable} ( + id INT NOT NULL, + info STRUCT, + events ARRAY>, + attrs MAP> + ) + """ + sql """ + INSERT INTO ${dorisTable} VALUES ( + 1, + STRUCT(10, 'doris_before'), + ARRAY(STRUCT(100)), + MAP('k', STRUCT(1000)) + ) + """ + + sql """ALTER TABLE ${dorisTable} ADD COLUMN info.doris_added STRING NULL COMMENT 'added by doris'""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN info.doris_first STRING NULL FIRST""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN info.doris_after_metric STRING NULL AFTER metric""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN events.element.doris_score INT NULL""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN events.element.doris_first_score INT NULL FIRST""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN events.element.doris_after_score INT NULL AFTER score""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN attrs.value.doris_code INT NULL""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN attrs.value.doris_first_code INT NULL FIRST""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN attrs.value.doris_after_code INT NULL AFTER code""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN info.drop_me STRING NULL""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN events.element.drop_me INT NULL""" + sql """ALTER TABLE ${dorisTable} ADD COLUMN attrs.value.drop_me INT NULL""" + sql """ALTER TABLE ${dorisTable} MODIFY COLUMN info.metric BIGINT""" + sql """ALTER TABLE ${dorisTable} MODIFY COLUMN events.element.score BIGINT""" + sql """ALTER TABLE ${dorisTable} MODIFY COLUMN attrs.value.code BIGINT""" + sql """ALTER TABLE ${dorisTable} RENAME COLUMN info.doris_added TO doris_renamed""" + sql """ALTER TABLE ${dorisTable} RENAME COLUMN events.element.doris_score TO doris_score_renamed""" + sql """ALTER TABLE ${dorisTable} RENAME COLUMN attrs.value.doris_code TO doris_code_renamed""" + sql """ALTER TABLE ${dorisTable} MODIFY COLUMN info.doris_renamed COMMENT 'renamed by doris'""" + sql """ALTER TABLE ${dorisTable} MODIFY COLUMN events.element.doris_score_renamed COMMENT 'renamed by doris'""" + sql """ALTER TABLE ${dorisTable} MODIFY COLUMN attrs.value.doris_code_renamed COMMENT 'renamed by doris'""" + sql """ALTER TABLE ${dorisTable} DROP COLUMN info.drop_me""" + sql """ALTER TABLE ${dorisTable} DROP COLUMN events.element.drop_me""" + sql """ALTER TABLE ${dorisTable} DROP COLUMN attrs.value.drop_me""" + + spark_iceberg """REFRESH TABLE demo.${dbName}.${dorisTable}""" + spark_iceberg """ + INSERT INTO demo.${dbName}.${dorisTable} VALUES ( + 2, + NAMED_STRUCT('doris_first', 'spark_first', + 'metric', CAST(20 AS BIGINT), + 'doris_after_metric', 'spark_after_metric', + 'label', 'spark_after_doris', + 'doris_renamed', 'spark_can_write_doris_field'), + ARRAY(NAMED_STRUCT('doris_first_score', 202, + 'score', CAST(200 AS BIGINT), + 'doris_after_score', 203, + 'doris_score_renamed', 201)), + MAP('k', NAMED_STRUCT('doris_first_code', 2002, + 'code', CAST(2000 AS BIGINT), + 'doris_after_code', 2003, + 'doris_code_renamed', 2001)) + ) + """ + + sql """refresh table ${dbName}.${dorisTable}""" + def dorisDrivenSparkRows = spark_iceberg """ + SELECT id, + info.doris_first, + info.metric, + info.doris_after_metric, + info.label, + info.doris_renamed, + events[0].doris_first_score, + events[0].score, + events[0].doris_after_score, + events[0].doris_score_renamed, + attrs['k'].doris_first_code, + attrs['k'].code, + attrs['k'].doris_after_code, + attrs['k'].doris_code_renamed + FROM demo.${dbName}.${dorisTable} + ORDER BY id + """ + String dorisDrivenDorisQuery = """ + SELECT id, + element_at(info, 'doris_first'), + element_at(info, 'metric'), + element_at(info, 'doris_after_metric'), + element_at(info, 'label'), + element_at(info, 'doris_renamed'), + element_at(events[1], 'doris_first_score'), + element_at(events[1], 'score'), + element_at(events[1], 'doris_after_score'), + element_at(events[1], 'doris_score_renamed'), + element_at(attrs['k'], 'doris_first_code'), + element_at(attrs['k'], 'code'), + element_at(attrs['k'], 'doris_after_code'), + element_at(attrs['k'], 'doris_code_renamed') + FROM ${dorisTable} + ORDER BY id + """ + order_qt_doris_driven_rows dorisDrivenDorisQuery + def dorisDrivenDorisRows = sql dorisDrivenDorisQuery + assertSparkDorisResultEquals(dorisDrivenSparkRows, dorisDrivenDorisRows) + + spark_iceberg_multi """ + DROP TABLE IF EXISTS demo.${dbName}.${sparkTable}; + DROP TABLE IF EXISTS demo.${dbName}.${mixedCaseTable}; + CREATE TABLE demo.${dbName}.${mixedCaseTable} ( + Id INT, + Label STRING, + Info STRUCT + ) USING iceberg; + CREATE TABLE demo.${dbName}.${sparkTable} ( + id INT, + info STRUCT, + events ARRAY>, + attrs MAP> + ) USING iceberg; + INSERT INTO demo.${dbName}.${sparkTable} VALUES ( + 1, + NAMED_STRUCT('metric', 10, 'label', 'spark_before'), + ARRAY(NAMED_STRUCT('score', 100)), + MAP('k', NAMED_STRUCT('code', 1000)) + ); + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN info.spark_added STRING; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN info.spark_first STRING FIRST; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN info.spark_after_metric STRING AFTER metric; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN events.element.spark_score INT; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN events.element.spark_first_score INT FIRST; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN events.element.spark_after_score INT AFTER score; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN attrs.value.spark_code INT; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN attrs.value.spark_first_code INT FIRST; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN attrs.value.spark_after_code INT AFTER code; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN info.drop_me STRING; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN events.element.drop_me INT; + ALTER TABLE demo.${dbName}.${sparkTable} ADD COLUMN attrs.value.drop_me INT; + ALTER TABLE demo.${dbName}.${sparkTable} ALTER COLUMN info.metric TYPE BIGINT; + ALTER TABLE demo.${dbName}.${sparkTable} ALTER COLUMN events.element.score TYPE BIGINT; + ALTER TABLE demo.${dbName}.${sparkTable} ALTER COLUMN attrs.value.code TYPE BIGINT; + ALTER TABLE demo.${dbName}.${sparkTable} RENAME COLUMN info.spark_added TO spark_renamed; + ALTER TABLE demo.${dbName}.${sparkTable} RENAME COLUMN events.element.spark_score TO spark_score_renamed; + ALTER TABLE demo.${dbName}.${sparkTable} RENAME COLUMN attrs.value.spark_code TO spark_code_renamed; + ALTER TABLE demo.${dbName}.${sparkTable} ALTER COLUMN info.spark_renamed COMMENT 'renamed by spark'; + ALTER TABLE demo.${dbName}.${sparkTable} ALTER COLUMN events.element.spark_score_renamed COMMENT 'renamed by spark'; + ALTER TABLE demo.${dbName}.${sparkTable} ALTER COLUMN attrs.value.spark_code_renamed COMMENT 'renamed by spark'; + ALTER TABLE demo.${dbName}.${sparkTable} DROP COLUMN info.drop_me; + ALTER TABLE demo.${dbName}.${sparkTable} DROP COLUMN events.element.drop_me; + ALTER TABLE demo.${dbName}.${sparkTable} DROP COLUMN attrs.value.drop_me; + INSERT INTO demo.${dbName}.${sparkTable} VALUES ( + 2, + NAMED_STRUCT('spark_first', 'spark_first_field', + 'metric', CAST(20 AS BIGINT), + 'spark_after_metric', 'spark_after_metric_field', + 'label', 'spark_after', + 'spark_renamed', 'spark_new_field'), + ARRAY(NAMED_STRUCT('spark_first_score', 202, + 'score', CAST(200 AS BIGINT), + 'spark_after_score', 203, + 'spark_score_renamed', 201)), + MAP('k', NAMED_STRUCT('spark_first_code', 2002, + 'code', CAST(2000 AS BIGINT), + 'spark_after_code', 2003, + 'spark_code_renamed', 2001)) + ); + """ + + sql """refresh catalog ${catalogName}""" + sql """switch ${catalogName}""" + sql """use ${dbName}""" + + test { + sql """ALTER TABLE ${mixedCaseTable} ADD COLUMN id STRING NULL""" + exception "conflicts with existing Iceberg field 'Id' (case-insensitive)" + } + test { + sql """ALTER TABLE ${mixedCaseTable} ADD COLUMN (id STRING)""" + exception "conflicts with existing Iceberg field 'Id' (case-insensitive)" + } + test { + sql """ALTER TABLE ${mixedCaseTable} ADD COLUMN (new_field STRING, NEW_FIELD STRING)""" + exception "conflicts with another requested column (case-insensitive)" + } + test { + sql """ALTER TABLE ${mixedCaseTable} RENAME COLUMN label TO id""" + exception "conflicts with existing Iceberg field 'Id' (case-insensitive)" + } + sql """ALTER TABLE ${mixedCaseTable} RENAME COLUMN label TO label""" + + test { + sql """ALTER TABLE ${mixedCaseTable} ADD COLUMN info.metric STRING NULL""" + exception "conflicts with existing Iceberg field 'Info.Metric' (case-insensitive)" + } + test { + sql """ALTER TABLE ${mixedCaseTable} RENAME COLUMN info.label TO metric""" + exception "conflicts with existing Iceberg field 'Info.Metric' (case-insensitive)" + } + + qt_spark_driven_schema """ + SELECT COLUMN_NAME, COLUMN_TYPE + FROM ${catalogName}.information_schema.columns + WHERE TABLE_SCHEMA = '${dbName}' AND TABLE_NAME = '${sparkTable}' + ORDER BY ORDINAL_POSITION + """ + + String sparkDrivenDorisQuery = """ + SELECT id, + element_at(info, 'spark_first'), + element_at(info, 'metric'), + element_at(info, 'spark_after_metric'), + element_at(info, 'label'), + element_at(info, 'spark_renamed'), + element_at(events[1], 'spark_first_score'), + element_at(events[1], 'score'), + element_at(events[1], 'spark_after_score'), + element_at(events[1], 'spark_score_renamed'), + element_at(attrs['k'], 'spark_first_code'), + element_at(attrs['k'], 'code'), + element_at(attrs['k'], 'spark_after_code'), + element_at(attrs['k'], 'spark_code_renamed') + FROM ${sparkTable} + ORDER BY id + """ + order_qt_spark_driven_rows_before_write sparkDrivenDorisQuery + + sql """ + INSERT INTO ${sparkTable} VALUES ( + 3, + STRUCT('doris_first_field', 30, 'doris_after_metric_field', + 'doris_after_spark', 'doris_can_write_spark_field'), + ARRAY(STRUCT(302, 300, 303, 301)), + MAP('k', STRUCT(3002, 3000, 3003, 3001)) + ) + """ + + spark_iceberg """REFRESH TABLE demo.${dbName}.${sparkTable}""" + def sparkDrivenSparkRows = spark_iceberg """ + SELECT id, + info.spark_first, + info.metric, + info.spark_after_metric, + info.label, + info.spark_renamed, + events[0].spark_first_score, + events[0].score, + events[0].spark_after_score, + events[0].spark_score_renamed, + attrs['k'].spark_first_code, + attrs['k'].code, + attrs['k'].spark_after_code, + attrs['k'].spark_code_renamed + FROM demo.${dbName}.${sparkTable} + ORDER BY id + """ + order_qt_spark_driven_rows_after_write sparkDrivenDorisQuery + def sparkDrivenDorisRows = sql sparkDrivenDorisQuery + assertSparkDorisResultEquals(sparkDrivenSparkRows, sparkDrivenDorisRows) + + spark_iceberg_multi """ + DROP TABLE IF EXISTS demo.${dbName}.${requiredTable}; + CREATE TABLE demo.${dbName}.${requiredTable} ( + id INT, + info STRUCT + ) USING iceberg; + INSERT INTO demo.${dbName}.${requiredTable} VALUES ( + 1, + NAMED_STRUCT('required_metric', 10, 'required_label', 'old-label') + ); + """ + + sql """refresh catalog ${catalogName}""" + sql """switch ${catalogName}""" + sql """use ${dbName}""" + + sql """ALTER TABLE ${requiredTable} MODIFY COLUMN info.required_metric BIGINT""" + sql """ALTER TABLE ${requiredTable} MODIFY COLUMN info.required_label STRING NULL""" + test { + sql """ALTER TABLE ${requiredTable} MODIFY COLUMN info.required_label STRING NOT NULL""" + exception "Can not change nullable column info.required_label to not null" + } + + String requiredTableUrl = + "http://${externalEnvIp}:${restPort}/v1/namespaces/${dbName}/tables/${requiredTable}" + def requiredTableMetadata = new JsonSlurper().parseText( + new URL(requiredTableUrl).getText("UTF-8")).metadata + def requiredSchema = requiredTableMetadata.schemas.find { + it["schema-id"] == requiredTableMetadata["current-schema-id"] + } + def requiredInfo = requiredSchema.fields.find { it.name == "info" } + def requiredInfoFields = requiredInfo.type.fields.collectEntries { [(it.name): it] } + assertEquals("long", requiredInfoFields.required_metric.type) + assertTrue(requiredInfoFields.required_metric.required) + assertFalse(requiredInfoFields.required_label.required) + + sql """ + INSERT INTO ${requiredTable} VALUES ( + 2, + STRUCT(20, NULL) + ) + """ + + String requiredDorisQuery = """ + SELECT id, + element_at(info, 'required_metric'), + element_at(info, 'required_label') + FROM ${requiredTable} + ORDER BY id + """ + order_qt_required_nested_rows requiredDorisQuery + spark_iceberg """REFRESH TABLE demo.${dbName}.${requiredTable}""" + def requiredSparkRows = spark_iceberg """ + SELECT id, info.required_metric, info.required_label + FROM demo.${dbName}.${requiredTable} + ORDER BY id + """ + def requiredDorisRows = sql requiredDorisQuery + assertSparkDorisResultEquals(requiredSparkRows, requiredDorisRows) + + spark_iceberg_multi """ + DROP TABLE IF EXISTS demo.${dbName}.${advancedTable}; + CREATE TABLE demo.${dbName}.${advancedTable} ( + id INT, + root STRUCT< + first_field: STRING, + middle_field: STRING, + last_field: STRING, + items: ARRAY>>> + > + ) USING iceberg; + INSERT INTO demo.${dbName}.${advancedTable} VALUES ( + 1, + NAMED_STRUCT( + 'first_field', 'first-old', + 'middle_field', 'middle-old', + 'last_field', 'last-old', + 'items', ARRAY(NAMED_STRUCT( + 'attrs', MAP('k', NAMED_STRUCT( + 'score', CAST(1.5 AS FLOAT), + 'amount', CAST(12.34 AS DECIMAL(9, 2)), + 'note', 'old-note' + )) + )) + ) + ); + """ + + sql """refresh catalog ${catalogName}""" + sql """switch ${catalogName}""" + sql """use ${dbName}""" + + sql """ALTER TABLE ${advancedTable} MODIFY COLUMN root.last_field STRING FIRST""" + sql """ALTER TABLE ${advancedTable} MODIFY COLUMN root.first_field STRING AFTER middle_field""" + sql """ALTER TABLE ${advancedTable} MODIFY COLUMN root.items.element.attrs.value.score DOUBLE""" + sql """ALTER TABLE ${advancedTable} MODIFY COLUMN root.items.element.attrs.value.amount DECIMAL(18, 2)""" + test { + sql """ALTER TABLE ${advancedTable} + MODIFY COLUMN root.items.element.attrs.value.amount DECIMAL(18, 3)""" + exception "Cannot change column type" + } + test { + sql """ALTER TABLE ${advancedTable} + MODIFY COLUMN root.items.element.attrs.value.score FLOAT""" + exception "Cannot change column type" + } + sql """ALTER TABLE ${advancedTable} + ADD COLUMN root.items.element.attrs.value.added STRING NULL AFTER amount""" + sql """ALTER TABLE ${advancedTable} + RENAME COLUMN root.items.element.attrs.value.added TO renamed""" + sql """ALTER TABLE ${advancedTable} + MODIFY COLUMN root.items.element.attrs.value.renamed COMMENT 'deep field'""" + sql """ALTER TABLE ${advancedTable} + MODIFY COLUMN root.items.element.attrs.value.note STRING FIRST""" + + String advancedTableUrl = + "http://${externalEnvIp}:${restPort}/v1/namespaces/${dbName}/tables/${advancedTable}" + def advancedTableMetadata = new JsonSlurper().parseText( + new URL(advancedTableUrl).getText("UTF-8")).metadata + def advancedSchema = advancedTableMetadata.schemas.find { + it["schema-id"] == advancedTableMetadata["current-schema-id"] + } + def rootField = advancedSchema.fields.find { it.name == "root" } + assertEquals(["last_field", "middle_field", "first_field", "items"], + rootField.type.fields.collect { it.name }) + def itemsField = rootField.type.fields.find { it.name == "items" } + def attrsField = itemsField.type.element.fields.find { it.name == "attrs" } + def deepFields = attrsField.type.value.fields + assertEquals(["note", "score", "amount", "renamed"], deepFields.collect { it.name }) + assertEquals("double", deepFields.find { it.name == "score" }.type) + assertEquals("decimal(18, 2)", deepFields.find { it.name == "amount" }.type) + assertEquals("deep field", deepFields.find { it.name == "renamed" }.doc) + + sql """ALTER TABLE ${advancedTable} + DROP COLUMN root.items.element.attrs.value.renamed""" + + def droppedFieldMetadata = new JsonSlurper().parseText( + new URL(advancedTableUrl).getText("UTF-8")).metadata + def droppedFieldSchema = droppedFieldMetadata.schemas.find { + it["schema-id"] == droppedFieldMetadata["current-schema-id"] + } + def droppedRootField = droppedFieldSchema.fields.find { it.name == "root" } + def droppedItemsField = droppedRootField.type.fields.find { it.name == "items" } + def droppedAttrsField = droppedItemsField.type.element.fields.find { it.name == "attrs" } + assertEquals(["note", "score", "amount"], + droppedAttrsField.type.value.fields.collect { it.name }) + + sql """ + INSERT INTO ${advancedTable} VALUES ( + 2, + STRUCT( + 'last-new', + 'middle-new', + 'first-new', + ARRAY(STRUCT(MAP('k', STRUCT( + 'new-note', + CAST(2.5 AS DOUBLE), + CAST(56.78 AS DECIMAL(18, 2)) + )))) + ) + ) + """ + + String advancedDorisQuery = """ + SELECT id, + element_at(root, 'last_field'), + element_at(root, 'middle_field'), + element_at(root, 'first_field'), + element_at(element_at(element_at(root, 'items')[1], 'attrs')['k'], 'note'), + element_at(element_at(element_at(root, 'items')[1], 'attrs')['k'], 'score'), + element_at(element_at(element_at(root, 'items')[1], 'attrs')['k'], 'amount') + FROM ${advancedTable} + ORDER BY id + """ + order_qt_deep_nested_rows advancedDorisQuery + spark_iceberg """REFRESH TABLE demo.${dbName}.${advancedTable}""" + def advancedSparkRows = spark_iceberg """ + SELECT id, + root.last_field, + root.middle_field, + root.first_field, + root.items[0].attrs['k'].note, + root.items[0].attrs['k'].score, + root.items[0].attrs['k'].amount + FROM demo.${dbName}.${advancedTable} + ORDER BY id + """ + def advancedDorisRows = sql advancedDorisQuery + assertSparkDorisResultEquals(advancedSparkRows, advancedDorisRows) +}