Skip to content

Commit 4b5cd1d

Browse files
Review with Devin contd.
1 parent 4b291f9 commit 4b5cd1d

File tree

3 files changed

+14
-23
lines changed

3 files changed

+14
-23
lines changed

extensions/iceberg/src/main/java/io/deephaven/iceberg/location/IcebergTableParquetLocation.java

+3-9
Original file line numberDiff line numberDiff line change
@@ -4,32 +4,26 @@
44
package io.deephaven.iceberg.location;
55

66
import io.deephaven.api.SortColumn;
7-
import io.deephaven.base.verify.Require;
87
import io.deephaven.engine.table.impl.locations.TableKey;
98
import io.deephaven.engine.table.impl.locations.TableLocation;
109
import io.deephaven.parquet.table.ParquetInstructions;
1110
import io.deephaven.parquet.table.location.ParquetTableLocation;
1211
import org.jetbrains.annotations.NotNull;
13-
import org.jetbrains.annotations.Nullable;
1412

1513
import java.util.List;
1614

17-
public class IcebergTableParquetLocation extends ParquetTableLocation implements TableLocation {
15+
class IcebergTableParquetLocation extends ParquetTableLocation implements TableLocation {
1816

19-
@Nullable
20-
private final List<SortColumn> sortedColumns;
21-
22-
public IcebergTableParquetLocation(
17+
IcebergTableParquetLocation(
2318
@NotNull final TableKey tableKey,
2419
@NotNull final IcebergTableParquetLocationKey tableLocationKey,
2520
@NotNull final ParquetInstructions readInstructions) {
2621
super(tableKey, tableLocationKey, readInstructions);
27-
sortedColumns = Require.neqNull(tableLocationKey.sortedColumns(), "tableLocationKey.sortedColumns()");
2822
}
2923

3024
@Override
3125
@NotNull
3226
public List<SortColumn> getSortedColumns() {
33-
return sortedColumns;
27+
return ((IcebergTableParquetLocationKey) getKey()).sortedColumns();
3428
}
3529
}

extensions/iceberg/src/main/java/io/deephaven/iceberg/location/IcebergTableParquetLocationKey.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import io.deephaven.engine.table.impl.locations.TableLocationKey;
99
import io.deephaven.parquet.table.ParquetInstructions;
1010
import io.deephaven.parquet.table.location.ParquetTableLocationKey;
11+
import io.deephaven.util.annotations.InternalUseOnly;
1112
import org.apache.iceberg.DataFile;
1213
import org.apache.iceberg.ManifestFile;
1314
import io.deephaven.util.channel.SeekableChannelsProvider;
@@ -25,6 +26,7 @@
2526
/**
2627
* {@link TableLocationKey} implementation for use with data stored in Iceberg tables in the parquet format.
2728
*/
29+
@InternalUseOnly
2830
public class IcebergTableParquetLocationKey extends ParquetTableLocationKey implements IcebergTableLocationKey {
2931

3032
private static final String IMPLEMENTATION_NAME = IcebergTableParquetLocationKey.class.getSimpleName();
@@ -119,7 +121,7 @@ public IcebergTableParquetLocationKey(
119121
manifestSequenceNumber = manifestFile.sequenceNumber();
120122

121123
this.readInstructions = readInstructions;
122-
this.sortedColumns = sortedColumns;
124+
this.sortedColumns = Require.neqNull(sortedColumns, "sortedColumns");
123125
}
124126

125127
@Override

extensions/iceberg/src/test/java/io/deephaven/iceberg/junit5/SqliteCatalogBase.java

+8-13
Original file line numberDiff line numberDiff line change
@@ -1051,28 +1051,23 @@ void testAutoRefreshingPartitionedAppend() throws InterruptedException {
10511051
/**
10521052
* Verify that the sort order for the data files in the table match the expected sort order.
10531053
*/
1054-
private void verifySortOrder(
1054+
private static void verifySortOrder(
10551055
final IcebergTableAdapter tableAdapter,
10561056
final List<List<SortColumn>> expectedSortOrders) {
10571057
verifySortOrder(tableAdapter, expectedSortOrders,
10581058
ParquetInstructions.EMPTY.withTableDefinition(tableAdapter.definition()));
10591059
}
10601060

1061-
private void verifySortOrder(
1061+
private static void verifySortOrder(
10621062
@NotNull final IcebergTableAdapter tableAdapter,
10631063
@NotNull final List<List<SortColumn>> expectedSortOrders,
10641064
@NotNull final ParquetInstructions readInstructions) {
10651065
final org.apache.iceberg.Table icebergTable = tableAdapter.icebergTable();
1066-
final Map<ManifestFile, List<DataFile>> manifestToDataFiles =
1067-
IcebergUtils.manifestToDataFiles(icebergTable, icebergTable.currentSnapshot());
10681066
final List<List<SortColumn>> actualSortOrders = new ArrayList<>();
1069-
for (final Map.Entry<ManifestFile, List<DataFile>> entry : manifestToDataFiles.entrySet()) {
1070-
final List<DataFile> dataFiles = entry.getValue();
1071-
for (final DataFile dataFile : dataFiles) {
1072-
actualSortOrders.add(computeSortedColumns(icebergTable, dataFile, readInstructions));
1073-
}
1074-
}
1075-
assertThat(actualSortOrders).containsExactlyInAnyOrderElementsOf(expectedSortOrders);
1067+
IcebergUtils.allDataFiles(icebergTable, icebergTable.currentSnapshot())
1068+
.forEach(dataFile -> actualSortOrders
1069+
.add(computeSortedColumns(icebergTable, dataFile, readInstructions)));
1070+
assertThat(actualSortOrders).isEqualTo(expectedSortOrders);
10761071
}
10771072

10781073
@Test
@@ -1110,8 +1105,8 @@ void testApplyDefaultSortOrder() {
11101105

11111106
// Verify that the new data file is sorted
11121107
verifySortOrder(tableAdapter, List.of(
1113-
List.of(),
1114-
List.of(SortColumn.asc(ColumnName.of("intCol")))));
1108+
List.of(SortColumn.asc(ColumnName.of("intCol"))),
1109+
List.of()));
11151110

11161111
// Append more unsorted data to the table without enforcing sort order
11171112
tableWriterWithoutSorting.append(IcebergWriteInstructions.builder()

0 commit comments

Comments
 (0)