amogh-jahagirdar commented on code in PR #10755:
URL: https://github.com/apache/iceberg/pull/10755#discussion_r1853082951
##########
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 removal of unreachable partition specs as part of the expiration
operation
+ *
+ * @param removeUnusedSpecs setting this to true will remove partition specs
that are no longer
+ * reachable by any snapshot
+ * @return this for method chaining
+ */
+ default ExpireSnapshots removeUnusedSpecs(boolean removeUnusedSpecs) {
Review Comment:
Thanks @advancedxy ! I'm in favor of the client side API option for now,
just had a comment on the name of it.
I think there's an important discussion to be had on how REST servers can
indicate to clients the specific supported update types, this can either be
done through config endpoint or through the existing endpoints discovery
mechanism where each update type is attached as a query fragment to the
endpoint and clients just use that.
My opinion is that it's not worth blocking this on a REST protocol
discussion since we can always just come back and deprecate this client side
option and have it run as part of the default procedure implementation.
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1108,6 +1108,24 @@ public Builder setDefaultPartitionSpec(int specId) {
return this;
}
+ Builder removeUnusedSpecs(Iterable<Integer> specIds) {
Review Comment:
+1, in the context of a TableMetadata builder, I think the right name is
`removeSpecs` or `removeSpecIds`. Whether it's unused or not is irrelevant for
the purpose of the TableMetadata builder API
##########
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 specs as
part of the operation.
+ *
+ * @param clean setting this to true will remove metadata(such as partition
spec) that are no
+ * longer reachable by any snapshot
+ * @return this for method chaining
+ */
+ default ExpireSnapshots cleanExpiredMeta(boolean clean) {
Review Comment:
I'd prefer to use the full `metadata` word, especially in a public API, we
don't really get much from an abbreviation.
`cleanExpiredMetadata`
--
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]
