rdblue commented on code in PR #10755:
URL: https://github.com/apache/iceberg/pull/10755#discussion_r1697279627
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1425,6 +1460,7 @@ private boolean hasChanges() {
|| (discardChanges && !changes.isEmpty())
|| metadataLocation != null
|| suppressHistoricalSnapshots
+ || hasRemovedSpecs
Review Comment:
> Yes, as discussed earlier, I don't think we should expose
removePartitionSpec directly to the REST API as there's no easy way to ensure
that the spec to remove is indeed not used.
We don't currently have operations that can't be supported by the REST
protocol, so this is an area where we should be careful. I agree that we want
to ensure that there are no new references to specs that are being removed, but
I'm skeptical that there is no way to do that. There are also problems with not
sending this because it would be a silent no-op when sending changes to REST
catalogs.
It's unlikely that a new snapshot would be written with a spec that is being
removed because this already validates that the default spec is not being
removed. A conflict here would require that a concurrent writer is using a spec
other than the default or has changed the default spec. There's already a
validation for the second case, `assert-default-spec-id`. For the first case,
this could require that no branch states have changed using
`assert-ref-snapshot-id`.
--
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
