amogh-jahagirdar commented on code in PR #10755:
URL: https://github.com/apache/iceberg/pull/10755#discussion_r1697317660
##########
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:
> The RemoveUnusedSpecs will succeed without actually removing unused specs
for REST catalog.
@advancedxy This indicates there's a problem with the current implementation
then imo. If there was a way to avoid the REST spec change I think it would've
been OK as long as we can guarantee failure on the client side until the
support was added for the metadata update type. But I think that's unavoidable,
and I agree with @rdblue that we should probably just add the metadata update
type.
I also am reasonably confident that a REST catalog can safely handle this
RemovePartitionSpec update. The default spec ID needs to be the same and there
must have been no writes to any branches. If any of those are not true, the
server should fail the update.
--
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
