amogh-jahagirdar commented on code in PR #10755:
URL: https://github.com/apache/iceberg/pull/10755#discussion_r1866315984
##########
api/src/main/java/org/apache/iceberg/ExpireSnapshots.java:
##########
@@ -118,4 +118,16 @@ public interface ExpireSnapshots extends
PendingUpdate<List<Snapshot>> {
* @return this for method chaining
*/
ExpireSnapshots cleanExpiredFiles(boolean clean);
+
+ /**
+ * Allows expiration of unreachable metadata, such as partition spec as part
of the operation.
+ *
+ * @param clean setting this to true will remove metadata(such as partition
spec) that is no
+ * longer reachable by any snapshot
+ * @return this for method chaining
Review Comment:
Basically don't think we need to mention the "such as partition spec" twice,
I think once at the top level and clarifying that we're talking about layout
metadata like the spec, and in the future the schema should be sufficient imo.
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1108,6 +1108,25 @@ public Builder setDefaultPartitionSpec(int specId) {
return this;
}
+ Builder removeSpecIds(Iterable<Integer> specIds) {
Review Comment:
I still feel like this should be called `removeSpecs`. If we look at the
other APIs on TableMetadata builder like `setDefaultPartitionSpec(int specId)`
it aligns with that naming, I don't think we need the "Ids" suffix.
##########
api/src/main/java/org/apache/iceberg/ExpireSnapshots.java:
##########
@@ -118,4 +118,16 @@ public interface ExpireSnapshots extends
PendingUpdate<List<Snapshot>> {
* @return this for method chaining
*/
ExpireSnapshots cleanExpiredFiles(boolean clean);
+
+ /**
+ * Allows expiration of unreachable metadata, such as partition spec as part
of the operation.
+ *
+ * @param clean setting this to true will remove metadata(such as partition
spec) that is no
+ * longer reachable by any snapshot
+ * @return this for method chaining
Review Comment:
```
/**
* Allows expiration of unreachable table layout metadata, such as
partition specs as part of the operation.
*
* @param setting this to true will remove table layout metadata that is no
* longer reachable by any snapshot
* @return this for method chaining
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
