github-actions[bot] commented on code in PR #65329:
URL: https://github.com/apache/doris/pull/65329#discussion_r3609822505
##########
be/src/format_v2/table/iceberg_reader.cpp:
##########
@@ -92,13 +92,14 @@ static Status build_missing_equality_delete_key_expr(const
format::ColumnDefinit
}
Field initial_default;
+ // A VARBINARY Field keeps a non-owning StringView until VLiteral
materializes it below.
+ std::string decoded_default;
Review Comment:
[P1] Carry metadata for dropped equality-delete fields
Iceberg requires an equality field that is dropped later to keep
participating in existing deletes. For an old data file followed by `ADD COLUMN
k INT DEFAULT 7`, an equality delete with `k = 7`, and then `DROP COLUMN k`, FE
serializes only the selected schema, so the fallback reaches this helper
without `k`'s immutable `initial-default`. V2 synthesizes NULL and can return
the deleted row; V1 instead fails with `Missing Iceberg schema metadata`. This
is distinct from the existing nested-ID thread because recursively searching
the current schema still cannot find an intentionally dropped field. Please
transport/resolve historical definitions for every equality ID and cover
add-default -> equality-delete -> drop -> scan in both readers.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -947,18 +1092,137 @@ 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, column, position, updateTime,
false);
+ return;
+ }
+
+ Table icebergTable = IcebergUtils.getIcebergTable(dorisTable);
+ ResolvedColumnPath resolvedPath =
resolveColumnPath(icebergTable.schema(), columnPath, "modify");
+ NestedField currentCol = resolvedPath.getField();
+ 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();
+ if (column.getType().isComplexType()) {
+ applyComplexTypeChange(updateSchema, resolvedPath.getFullPath(),
currentCol.type(),
+ column.getType(), false);
+ if (!Objects.equals(currentCol.doc(), column.getComment())) {
+ updateSchema.updateColumnDoc(resolvedPath.getFullPath(),
column.getComment());
+ }
+ } else {
+ applyPrimitiveColumnChange(updateSchema,
resolvedPath.getFullPath(), currentCol,
+ targetType.asPrimitiveType(), column.getComment());
+ }
+ applyExplicitNullableChange(updateSchema, resolvedPath.getFullPath(),
column, false);
+
+ 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");
+
+ UpdateSchema updateSchema = icebergTable.updateSchema();
+ updateSchema.updateColumnDoc(resolvedPath.getFullPath(),
StringUtils.defaultString(comment));
Review Comment:
[P1] Reject comments that Iceberg cannot persist on collection pseudo-fields
For direct `arr_scalar.element` and `m_scalar.value` targets, this call
records a pending doc update, but Iceberg 1.10.1's
`SchemaUpdate.ApplyChanges.list/map` consumes only type and optionality: a
doc-only change returns the original collection, and a combined change
reconstructs the pseudo-field without a doc. These newly accepted ALTERs
therefore commit successfully while discarding the requested comment. The
Mockito test only verifies this API call and the regression never reads the
persisted schema. Please reject direct `element`/`value` comments (including
COMMENT on their type MODIFY) unless the dependency can persist them, and
verify the committed schema rather than only the mock invocation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
