Skip to content

HDDS-15174. Add tests for Ozone Iceberg statistics file copy plan. #10189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
slfan1989 wants to merge 2 commits into
base: master
Choose a base branch
from
+52 −0
Open

HDDS-15174. Add tests for Ozone Iceberg statistics file copy plan. #10189

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b04aa1
HDDS-15174. Add tests for Ozone Iceberg statistics file copy plan.
slfan1989 May 5, 2026
6bafb5c
HDDS-15174. Add tests for Ozone Iceberg statistics file copy plan.
slfan1989 May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions ...ceberg/src/test/java/org/apache/hadoop/ozone/iceberg/TestRewriteTablePathOzoneAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.hadoop.ozone.iceberg;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.BufferedReader;
Expand All @@ -33,12 +34,14 @@
import org.apache.iceberg.DataFile;
import org.apache.iceberg.DataFiles;
import org.apache.iceberg.FileFormat;
import org.apache.iceberg.GenericStatisticsFile;
import org.apache.iceberg.HasTableOperations;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.RewriteTablePathUtil;
import org.apache.iceberg.Schema;
import org.apache.iceberg.Snapshot;
import org.apache.iceberg.StaticTableOperations;
import org.apache.iceberg.StatisticsFile;
import org.apache.iceberg.Table;
import org.apache.iceberg.TableMetadata;
import org.apache.iceberg.TableMetadata.MetadataLogEntry;
Expand Down Expand Up @@ -188,6 +191,51 @@ void tablePathRewriteForStartAndEndVersionProvided() throws Exception {
assertAllInternalPathsRewritten(csvPairs, targetPrefix);
}

@Test
void statsFileCopyPlanReturnsEmptySetForEmptyStats() {
Set<Pair<String, String>> copyPlan =
RewriteTablePathOzoneUtils.statsFileCopyPlan(List.of(), List.of());

assertTrue(copyPlan.isEmpty());
}

@Test
void statsFileCopyPlanRejectsMismatchedStatsCount() {
IllegalArgumentException exception = assertThrows(IllegalArgumentException.class,
() -> RewriteTablePathOzoneUtils.statsFileCopyPlan(
List.of(statisticsFile("before-1.stats", 100)),
List.of()));

assertEquals("Before and after path rewrite, statistic files count should be same",
exception.getMessage());
}
Comment on lines +202 to +211
Copy link
Copy Markdown
Contributor

@sreejasahithi sreejasahithi May 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statistics files always have the .stats extension in real usage and .puffin is the extension used for Deletion Vector files (*-deletes.puffin). While the test works with any string since statsFileCopyPlan never validates the extension, using .puffin is misleading. Should be .stats

Copy link
Copy Markdown
Contributor Author

@slfan1989 slfan1989 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! Fixed. The test data now uses .stats for statistics files, which better matches the real file naming and avoids implying deletion vector files.


@Test
void statsFileCopyPlanRejectsMismatchedStatsFileSize() {
IllegalArgumentException exception = assertThrows(IllegalArgumentException.class,
() -> RewriteTablePathOzoneUtils.statsFileCopyPlan(
List.of(statisticsFile("before-1.stats", 100)),
List.of(statisticsFile("after-1.stats", 200))));

assertEquals("Before and after path rewrite, statistic files size should be same",
exception.getMessage());
}

@Test
void statsFileCopyPlanReturnsBeforeToAfterPathPairs() {
Set<Pair<String, String>> copyPlan = RewriteTablePathOzoneUtils.statsFileCopyPlan(
List.of(
statisticsFile("before-1.stats", 100),
statisticsFile("before-2.stats", 200)),
List.of(
statisticsFile("after-1.stats", 100),
statisticsFile("after-2.stats", 200)));

assertEquals(Set.of(
Pair.of("before-1.stats", "after-1.stats"),
Pair.of("before-2.stats", "after-2.stats")), copyPlan);
}

/**
* For every staged metadata JSON file in the CSV, parses the file and asserts that:
* - The table location starts with target
Expand Down Expand Up @@ -276,4 +324,8 @@ private DataFile dummyDataFile(String dataPath) {
.withFormat(FileFormat.PARQUET)
.build();
}

private static StatisticsFile statisticsFile(String path, long fileSizeInBytes) {
return new GenericStatisticsFile(1L, path, fileSizeInBytes, 0L, List.of());
}
}