rdblue commented on code in PR #8664:
URL: https://github.com/apache/iceberg/pull/8664#discussion_r1338820667
##########
core/src/main/java/org/apache/iceberg/view/ViewMetadata.java:
##########
@@ -295,16 +296,34 @@ private int reuseOrCreateNewViewVersionId(ViewVersion
viewVersion) {
// if the view version already exists, use its id; otherwise use the
highest id + 1
int newVersionId = viewVersion.versionId();
for (ViewVersion version : versions) {
- if (version.equals(viewVersion)) {
+ if (sameViewVersion(version, viewVersion)) {
return version.versionId();
} else if (version.versionId() >= newVersionId) {
- newVersionId = viewVersion.versionId() + 1;
+ newVersionId = version.versionId() + 1;
}
}
return newVersionId;
}
+ /**
+ * Checks whether the given view versions are the same while ignoring the
view version id
+ *
+ * @param one the view version to compare
+ * @param two the view version to compare
+ * @return tru if the given view versions are the same while ignoring the
view version id
+ */
+ private boolean sameViewVersion(ViewVersion one, ViewVersion two) {
+ // explicitly ignore the view version because it gets reassigned in
+ // reuseOrCreateNewViewVersionId
+ return Objects.equals(one.timestampMillis(), two.timestampMillis())
Review Comment:
I don't think this is correct. We're trying to detect that two versions
would behave the same way and can be deduplicated. This is going to prevent
that because the two versions can easily be created at different times. For
example, I might call `replaceViewVersion()` and that reassigns the create
timestamp, but it should not create a new version if it is identical to an
existing one.
--
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]
