Review with Ryan contd.

deephaven · Nov 15, 2024 · 9116c63 · 9116c63
1 parent 968f769
commit 9116c63
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 19 deletions.
diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergBaseLayout.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/layout/IcebergBaseLayout.java
@@ -20,7 +20,6 @@
 import org.jetbrains.annotations.Nullable;
 
 import java.net.URI;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
@@ -42,11 +41,6 @@ public abstract class IcebergBaseLayout implements TableLocationKeyFinder<Iceber
      */
     final IcebergReadInstructions instructions;
 
-    /**
-     * A cache of {@link IcebergTableLocationKey IcebergTableLocationKeys} keyed by the URI of the file they represent.
-     */
-    final Map<URI, IcebergTableLocationKey> cache;
-
     /**
      * The {@link Snapshot} from which to discover data files.
      */
@@ -129,8 +123,6 @@ public IcebergBaseLayout(
         this.instructions = instructions;
         this.dataInstructionsProvider = dataInstructionsProvider;
         this.tableDef = tableAdapter.definition(instructions);
-
-        this.cache = new HashMap<>();
     }
 
     abstract IcebergTableLocationKey keyFromDataFile(ManifestFile manifestFile, DataFile dataFile, URI fileUri);

diff --git a/...s/iceberg/src/main/java/io/deephaven/iceberg/location/IcebergTableParquetLocationKey.java b/...s/iceberg/src/main/java/io/deephaven/iceberg/location/IcebergTableParquetLocationKey.java
@@ -89,9 +89,9 @@ public ParquetInstructions readInstructions() {
     }
 
     /**
-     * Precedence-wise this implementation compares {@code order}, then applies a {@link PartitionsComparator} to
-     * {@code partitions}, then {@code dataSequenceNumber}, then {@code fileSequenceNumber}, then
-     * {@code manifestSequenceNumber}, then {@code dataFilePos}, and finally compares {@code uri}.
+     * Precedence-wise this implementation compares {@code order}, then {@code dataSequenceNumber}, then
+     * {@code fileSequenceNumber}, then {@code manifestSequenceNumber}, then {@code dataFilePos}, and finally compares
+     * {@code uri}.
      *
      * @inheritDoc
      */
@@ -104,9 +104,6 @@ public int compareTo(@NotNull final TableLocationKey other) {
             if ((comparisonResult = Integer.compare(order, otherTyped.order)) != 0) {
                 return comparisonResult;
             }
-            if ((comparisonResult = PartitionsComparator.INSTANCE.compare(partitions, otherTyped.partitions)) != 0) {
-                return comparisonResult;
-            }
             if ((comparisonResult = Long.compare(dataSequenceNumber, otherTyped.dataSequenceNumber)) != 0) {
                 return comparisonResult;
             }
@@ -121,7 +118,8 @@ public int compareTo(@NotNull final TableLocationKey other) {
             }
             return uri.compareTo(otherTyped.uri);
         }
-        throw new ClassCastException("Cannot compare " + getClass() + " to " + other.getClass());
+        // When comparing with non-iceberg location key, we want to compare both partitions and URI
+        return super.compareTo(other);
     }
 
     @Override
@@ -140,7 +138,7 @@ public boolean equals(@Nullable final Object other) {
                 && fileSequenceNumber == otherTyped.fileSequenceNumber
                 && dataFilePos == otherTyped.dataFilePos
                 && manifestSequenceNumber == otherTyped.manifestSequenceNumber
-                && super.equals(otherTyped);
+                && uri.equals(otherTyped.uri);
     }
 
     @Override
@@ -152,7 +150,7 @@ public int hashCode() {
             result = prime * result + Long.hashCode(fileSequenceNumber);
             result = prime * result + Long.hashCode(dataFilePos);
             result = prime * result + Long.hashCode(manifestSequenceNumber);
-            result = prime * result + super.hashCode();
+            result = prime * result + uri.hashCode();
             // Don't use 0; that's used by StandaloneTableLocationKey, and also our sentinel for the need to compute
             if (result == 0) {
                 final int fallbackHashCode = IcebergTableParquetLocationKey.class.hashCode();

diff --git a/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergUpdateMode.java b/extensions/iceberg/src/main/java/io/deephaven/iceberg/util/IcebergUpdateMode.java
@@ -37,15 +37,16 @@ public static IcebergUpdateMode manualRefreshingMode() {
     }
 
     /**
-     * Set a automatically-refreshing update mode for this table using the default refresh interval of 60 seconds. The
+     * Set an automatically-refreshing update mode for this table using the default refresh interval of 60 seconds. The
      * ordering of the data in the table will depend on the order in which the data files are discovered on refresh.
      */
     public static IcebergUpdateMode autoRefreshingMode() {
         return AUTO_REFRESHING;
     }
 
     /**
-     * Set a automatically-refreshing update mode for this table using the provided refresh interval
+     * Set an automatically-refreshing update mode for this table using the provided refresh interval. The ordering of
+     * the data in the table will depend on the order in which the data files are discovered on refresh.
      *
      * @param refreshMs the refresh interval in milliseconds
      */