ajantha-bhat commented on code in PR #8678:
URL: https://github.com/apache/iceberg/pull/8678#discussion_r1341480615
##########
core/src/main/java/org/apache/iceberg/view/ViewMetadata.java:
##########
@@ -321,14 +321,20 @@ private int reuseOrCreateNewViewVersionId(ViewVersion
viewVersion) {
/**
* Checks whether the given view versions would behave the same while
ignoring the view version
- * id, the creation timestamp, and the summary.
+ * id, the creation timestamp, and the operation.
*
* @param one the view version to compare
* @param two the view version to compare
* @return true if the given view versions would behave the same
*/
private boolean sameViewVersion(ViewVersion one, ViewVersion two) {
- return Objects.equals(one.representations(), two.representations())
+ // ignore the "operation" contained in the summary for the comparison
+ Map<String, String> summaryOne = Maps.newHashMap(one.summary());
+ Map<String, String> summaryTwo = Maps.newHashMap(two.summary());
+ summaryOne.remove("operation");
+ summaryTwo.remove("operation");
+ return Objects.equals(summaryOne, summaryTwo)
+ && Objects.equals(one.representations(), two.representations())
Review Comment:
I think we should capture somewhere in spec that two views are considered as
equal even if their operations are different?
I can understand we are handling some edge case of "dummy replace" operation
here. But looks weird to me.
(If the replace operation has new SQL, anyways it won't be considered as
equal)
--
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
