szehon-ho commented on code in PR #16028:
URL: https://github.com/apache/iceberg/pull/16028#discussion_r3359626186
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/BaseSparkTable.java:
##########
@@ -88,6 +91,10 @@ public String name() {
return table.toString();
}
+ /**
+ * @deprecated since 1.12.0, use columns() instead
Review Comment:
nit: for consistency with the catalog methods in this PR (which use `{@link
#createTable(Identifier, TableInfo)}`), consider making this a clickable
javadoc link:
```suggestion
* @deprecated since 1.12.0, use {@link #columns()} instead
```
(Same applies to `SparkChangelogTable` and `RollbackStagedTable`.)
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/RollbackStagedTable.java:
##########
@@ -85,11 +86,20 @@ public String name() {
return table.name();
}
+ /**
+ * @deprecated since 1.12.0, use columns() instead
Review Comment:
nit: same as `BaseSparkTable` -- use a `{@link}` for consistency.
```suggestion
* @deprecated since 1.12.0, use {@link #columns()} instead
```
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/SparkChangelogTable.java:
##########
@@ -57,6 +60,10 @@ public String name() {
return table.name() + "." + TABLE_NAME;
}
+ /**
+ * @deprecated since 1.12.0, use columns() instead
Review Comment:
nit: same as `BaseSparkTable` -- use a `{@link}` for consistency.
```suggestion
* @deprecated since 1.12.0, use {@link #columns()} instead
```
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkRewriteTableCatalog.java:
##########
@@ -73,10 +74,20 @@ public void invalidateTable(Identifier ident) {
throw new UnsupportedOperationException(CLASS_NAME + " does not support
table invalidation");
}
+ /**
+ * @deprecated since 1.12.0, use {@link #createTable(Identifier, TableInfo)}
instead.
+ */
+ @Deprecated
@Override
public SparkTable createTable(
Identifier ident, StructType schema, Transform[] partitions, Map<String,
String> properties)
throws TableAlreadyExistsException {
+ return createTable(ident, Spark3Util.tableInfo(schema, partitions,
properties));
+ }
Review Comment:
nit (optional): since this catalog only throws
`UnsupportedOperationException`, the deprecated `StructType` override is now
redundant -- Spark's default `createTable(Identifier, TableInfo)` chain would
route here anyway, and the only effect of keeping it is an extra
`Spark3Util.tableInfo(...)` conversion before throwing. You could drop this
deprecated override entirely and keep just the `TableInfo` override below.
--
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]
