szehon-ho commented on code in PR #17954:
URL: https://github.com/apache/iceberg/pull/17954#discussion_r4010485504
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -206,11 +210,45 @@ public Table createTable(
Identifier ident, StructType schema, Transform[] transforms, Map<String,
String> properties)
throws TableAlreadyExistsException {
Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ return createTable(ident, icebergSchema, transforms, properties,
SortOrder.unsorted());
+ }
+
+ @Override
+ public Table createTableLike(Identifier ident, TableInfo tableInfo, Table
sourceTable)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ // Spark intentionally excludes the source table's properties from
tableInfo and leaves it to
+ // the connector to decide which to clone via sourceTable. Clone the
source Iceberg table's
+ // properties and sort order, then let user-specified LIKE options (in
tableInfo) take
+ // precedence.
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo.schema());
Review Comment:
Please preserve the source Iceberg schema instead of rebuilding it from
`tableInfo.schema()`. This round trip maps Iceberg UUID to Spark string and
back to Iceberg string (and fixed to binary), and `SparkTypeToType` does not
restore write or initial defaults. For an Iceberg `SparkTable`, use the
original Iceberg schema and fall back to Spark conversion only for non-Iceberg
sources.
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -206,11 +210,45 @@ public Table createTable(
Identifier ident, StructType schema, Transform[] transforms, Map<String,
String> properties)
throws TableAlreadyExistsException {
Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ return createTable(ident, icebergSchema, transforms, properties,
SortOrder.unsorted());
+ }
+
+ @Override
+ public Table createTableLike(Identifier ident, TableInfo tableInfo, Table
sourceTable)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ // Spark intentionally excludes the source table's properties from
tableInfo and leaves it to
+ // the connector to decide which to clone via sourceTable. Clone the
source Iceberg table's
+ // properties and sort order, then let user-specified LIKE options (in
tableInfo) take
+ // precedence.
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo.schema());
+ Map<String, String> properties = Maps.newHashMap();
+ SortOrder sortOrder = SortOrder.unsorted();
+
+ if (sourceTable instanceof SparkTable) {
+ org.apache.iceberg.Table sourceIcebergTable = ((SparkTable)
sourceTable).table();
+ properties.putAll(sourceIcebergTable.properties());
Review Comment:
Please remove source location-bearing properties such as
`write.metadata.path`, `write.data.path`, and the legacy storage paths before
applying `tableInfo.properties()`. Otherwise the new table can write metadata
and data into the source table's directories despite having a different table
location, and orphan cleanup can delete the other table's files.
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -206,11 +210,45 @@ public Table createTable(
Identifier ident, StructType schema, Transform[] transforms, Map<String,
String> properties)
throws TableAlreadyExistsException {
Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ return createTable(ident, icebergSchema, transforms, properties,
SortOrder.unsorted());
+ }
+
+ @Override
+ public Table createTableLike(Identifier ident, TableInfo tableInfo, Table
sourceTable)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ // Spark intentionally excludes the source table's properties from
tableInfo and leaves it to
+ // the connector to decide which to clone via sourceTable. Clone the
source Iceberg table's
+ // properties and sort order, then let user-specified LIKE options (in
tableInfo) take
+ // precedence.
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo.schema());
+ Map<String, String> properties = Maps.newHashMap();
+ SortOrder sortOrder = SortOrder.unsorted();
+
+ if (sourceTable instanceof SparkTable) {
+ org.apache.iceberg.Table sourceIcebergTable = ((SparkTable)
sourceTable).table();
+ properties.putAll(sourceIcebergTable.properties());
Review Comment:
Please carry the source table's format version into the target properties
unless explicitly overridden. `format-version` is reserved and is not present
in `sourceIcebergTable.properties()`, so a v3 source defaults to v2; cloning a
schema containing variant, geometry, or geography then fails compatibility
validation.
--
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]
