mchades commented on code in PR #5127:
URL: https://github.com/apache/gravitino/pull/5127#discussion_r1802613136
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -492,4 +526,153 @@ private Table internalCreateTable(
getHiddenPropertyNames(
catalogIdent, HasPropertyMetadata::tablePropertiesMetadata,
table.properties()));
}
+
+ private List<ColumnEntity> toColumnEntities(Column[] columns, AuditInfo
audit) {
+ return columns == null
+ ? Collections.emptyList()
+ : Arrays.stream(columns)
+ .map(c -> ColumnEntity.toColumnEntity(c, idGenerator.nextId(),
audit))
+ .collect(Collectors.toList());
+ }
+
+ private boolean isSameColumn(Column left, ColumnEntity right) {
+ return Objects.equal(left.name(), right.name())
+ && Objects.equal(left.dataType(), right.dataType())
+ && Objects.equal(left.comment(), right.comment())
+ && left.nullable() == right.nullable()
+ && left.autoIncrement() == right.autoIncrement()
+ && Objects.equal(left.defaultValue(), right.defaultValue());
+ }
+
+ private Pair<Boolean, List<ColumnEntity>> updateColumnsIfNecessary(
+ Table tableFromCatalog, TableEntity tableFromGravitino) {
+ if (tableFromCatalog == null || tableFromGravitino == null) {
+ LOG.warn(
+ "Cannot update columns for table when altering because table or
table entity is "
+ + "null");
+ return Pair.of(false, Collections.emptyList());
+ }
+
+ Map<String, Column> columnsFromCatalogTable =
+ tableFromCatalog.columns() == null
+ ? Collections.emptyMap()
+ : Arrays.stream(tableFromCatalog.columns())
+ .collect(Collectors.toMap(Column::name, Function.identity()));
+ Map<String, ColumnEntity> columnsFromTableEntity =
+ tableFromGravitino.columns() == null
+ ? Collections.emptyMap()
+ : tableFromGravitino.columns().stream()
+ .collect(Collectors.toMap(ColumnEntity::name,
Function.identity()));
+
+ // Check if columns need to be updated in Gravitino store
Review Comment:
The order of the columns also needs to be considered.
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -348,34 +364,45 @@ private void importTable(NameIdentifier identifier) {
// of external system to correct it.
LOG.warn(
"The Table uid {} existed but still need to be imported, this could
be happened "
- + "when Table is renamed by external systems not controlled by
Gravitino. In this case, "
- + "we need to overwrite the stored entity to keep the
consistency.",
+ + "when Table is renamed by external systems not controlled by
Gravitino. In this "
+ + "case, we need to overwrite the stored entity to keep the
consistency.",
stringId);
uid = stringId.id();
} else {
// If entity doesn't exist, we import the entity from the external
system.
uid = idGenerator.nextId();
}
+ AuditInfo audit =
+ AuditInfo.builder()
+ .withCreator(table.auditInfo().creator())
+ .withCreateTime(table.auditInfo().createTime())
+ .withLastModifier(table.auditInfo().lastModifier())
+ .withLastModifiedTime(table.auditInfo().lastModifiedTime())
+ .build();
+ List<ColumnEntity> columnEntityList =
+ toColumnEntities(table.tableFromCatalog().columns(), audit);
Review Comment:
The final modifier of the table could potentially be the column creator,
however, it appears that we may not have the capability to address this
scenario, right?
--
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]
