diff --git a/java/src/main/java/org/lance/fragment/DataFile.java b/java/src/main/java/org/lance/fragment/DataFile.java index 1120a5286c1..9755c271d67 100644 --- a/java/src/main/java/org/lance/fragment/DataFile.java +++ b/java/src/main/java/org/lance/fragment/DataFile.java @@ -85,7 +85,16 @@ public boolean equals(Object o) { && Objects.equals(path, that.path) && Arrays.equals(fields, that.fields) && Arrays.equals(columnIndices, that.columnIndices) - && Objects.equals(fileSizeBytes, that.fileSizeBytes); + && Objects.equals(fileSizeBytes, that.fileSizeBytes) + && Objects.equals(baseId, that.baseId); + } + + @Override + public int hashCode() { + int result = Objects.hash(path, fileMajorVersion, fileMinorVersion, fileSizeBytes, baseId); + result = 31 * result + Arrays.hashCode(fields); + result = 31 * result + Arrays.hashCode(columnIndices); + return result; } @Override diff --git a/java/src/main/java/org/lance/fragment/DeletionFile.java b/java/src/main/java/org/lance/fragment/DeletionFile.java index 6fa17a37c3f..2eed2fafb0f 100644 --- a/java/src/main/java/org/lance/fragment/DeletionFile.java +++ b/java/src/main/java/org/lance/fragment/DeletionFile.java @@ -95,7 +95,13 @@ public boolean equals(Object o) { return id == that.id && readVersion == that.readVersion && fileType == that.fileType - && Objects.equals(numDeletedRows, that.numDeletedRows); + && Objects.equals(numDeletedRows, that.numDeletedRows) + && Objects.equals(baseId, that.baseId); + } + + @Override + public int hashCode() { + return Objects.hash(id, readVersion, numDeletedRows, fileType, baseId); } @Override diff --git a/java/src/main/java/org/lance/operation/DataOverlay.java b/java/src/main/java/org/lance/operation/DataOverlay.java index 99abf84b797..57f5ed83f55 100644 --- a/java/src/main/java/org/lance/operation/DataOverlay.java +++ b/java/src/main/java/org/lance/operation/DataOverlay.java @@ -161,15 +161,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - int dataFileHash = - Objects.hash( - dataFile.getPath(), - dataFile.getFileMajorVersion(), - dataFile.getFileMinorVersion(), - dataFile.getFileSizeBytes()); - dataFileHash = 31 * dataFileHash + Arrays.hashCode(dataFile.getFields()); - dataFileHash = 31 * dataFileHash + Arrays.hashCode(dataFile.getColumnIndices()); - return Objects.hash(dataFileHash, coverage, committedVersion); + return Objects.hash(dataFile, coverage, committedVersion); } } diff --git a/java/src/test/java/org/lance/fragment/DataFileEqualityTest.java b/java/src/test/java/org/lance/fragment/DataFileEqualityTest.java new file mode 100644 index 00000000000..1726cda1727 --- /dev/null +++ b/java/src/test/java/org/lance/fragment/DataFileEqualityTest.java @@ -0,0 +1,73 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.lance.fragment; + +import org.lance.FragmentMetadata; + +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Equality contract of fragment metadata value classes. */ +public class DataFileEqualityTest { + + private static DataFile dataFile(Integer baseId) { + return new DataFile("data/a.lance", new int[] {0, 1}, new int[] {0, 1}, 2, 1, 1024L, baseId); + } + + private static DeletionFile deletionFile(Integer baseId) { + return new DeletionFile(7L, 3L, 5L, DeletionFileType.BITMAP, baseId); + } + + @Test + void testDataFileEqualsAndHashCodeIncludeBaseId() { + for (Integer baseId : new Integer[] {null, 1}) { + assertEquals(dataFile(baseId), dataFile(baseId)); + assertEquals(dataFile(baseId).hashCode(), dataFile(baseId).hashCode()); + } + // Same relative path under different bases refers to different physical files. + assertNotEquals(dataFile(1), dataFile(2)); + assertNotEquals(dataFile(null), dataFile(1)); + } + + @Test + void testDeletionFileEqualsAndHashCodeIncludeBaseId() { + for (Integer baseId : new Integer[] {null, 1}) { + assertEquals(deletionFile(baseId), deletionFile(baseId)); + assertEquals(deletionFile(baseId).hashCode(), deletionFile(baseId).hashCode()); + } + assertNotEquals(deletionFile(1), deletionFile(2)); + assertNotEquals(deletionFile(null), deletionFile(1)); + } + + @Test + void testEqualFragmentMetadataWorksInHashSet() { + FragmentMetadata a = + new FragmentMetadata( + 0, Collections.singletonList(dataFile(null)), 100L, deletionFile(null), null); + FragmentMetadata b = + new FragmentMetadata( + 0, Collections.singletonList(dataFile(null)), 100L, deletionFile(null), null); + assertEquals(a, b); + assertEquals(a.hashCode(), b.hashCode()); + Set set = new HashSet<>(Collections.singletonList(a)); + assertTrue(set.contains(b)); + } +}