Skip to content

Add assertions for sequence numbers to TestFastAppend #1038

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

Merged
danielcweeks merged 2 commits into from
May 21, 2020
Merged

Add assertions for sequence numbers to TestFastAppend #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8527010
Add assertions for sequence numbers to TestFastAppend.
rdblue May 13, 2020
af51438
Fix checkstyle.
rdblue May 13, 2020
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
69 changes: 66 additions & 3 deletions core/src/test/java/org/apache/iceberg/TableTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.stream.LongStream;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.OutputFile;
import org.apache.iceberg.types.Types;
Expand Down Expand Up @@ -87,9 +88,15 @@ public class TableTestBase {
public TestTables.TestTable table = null;

protected final int formatVersion;
@SuppressWarnings("checkstyle:MemberName")
protected final Assertions V1Assert;
@SuppressWarnings("checkstyle:MemberName")
protected final Assertions V2Assert;

public TableTestBase(int formatVersion) {
this.formatVersion = formatVersion;
this.V1Assert = new Assertions(1, formatVersion);
this.V2Assert = new Assertions(2, formatVersion);
}

@Before
Expand Down Expand Up @@ -197,6 +204,14 @@ ManifestEntry manifestEntry(ManifestEntry.Status status, Long snapshotId, DataFi
}

void validateSnapshot(Snapshot old, Snapshot snap, DataFile... newFiles) {
validateSnapshot(old, snap, null, newFiles);
}

void validateSnapshot(Snapshot old, Snapshot snap, long sequenceNumber, DataFile... newFiles) {
validateSnapshot(old, snap, (Long) sequenceNumber, newFiles);
}

void validateSnapshot(Snapshot old, Snapshot snap, Long sequenceNumber, DataFile... newFiles) {
List<ManifestFile> oldManifests = old != null ? old.manifests() : ImmutableList.of();

// copy the manifests to a modifiable list and remove the existing manifests
Expand All @@ -215,6 +230,10 @@ void validateSnapshot(Snapshot old, Snapshot snap, DataFile... newFiles) {

for (ManifestEntry entry : ManifestFiles.read(manifest, FILE_IO).entries()) {
DataFile file = entry.file();
if (sequenceNumber != null) {
V1Assert.assertEquals("Sequence number should default to 0", 0, entry.sequenceNumber().longValue());
V2Assert.assertEquals("Sequence number should match expected", sequenceNumber, entry.sequenceNumber());
}
Assert.assertEquals("Path should match expected", newPaths.next(), file.path().toString());
Assert.assertEquals("File's snapshot ID should match", id, (long) entry.snapshotId());
}
Expand Down Expand Up @@ -242,12 +261,23 @@ List<String> paths(DataFile... dataFiles) {
return paths;
}

static void validateManifest(ManifestFile manifest,
Iterator<Long> ids,
Iterator<DataFile> expectedFiles) {
void validateManifest(ManifestFile manifest,
Iterator<Long> ids,
Iterator<DataFile> expectedFiles) {
validateManifest(manifest, null, ids, expectedFiles);
}

void validateManifest(ManifestFile manifest,
Iterator<Long> seqs,
Iterator<Long> ids,
Iterator<DataFile> expectedFiles) {
for (ManifestEntry entry : ManifestFiles.read(manifest, FILE_IO).entries()) {
DataFile file = entry.file();
DataFile expected = expectedFiles.next();
if (seqs != null) {
V1Assert.assertEquals("Sequence number should default to 0", 0, entry.sequenceNumber().longValue());
V2Assert.assertEquals("Sequence number should match expected", seqs.next(), entry.sequenceNumber());
}
Assert.assertEquals("Path should match expected",
expected.path().toString(), file.path().toString());
Assert.assertEquals("Snapshot ID should match expected ID",
Expand Down Expand Up @@ -280,6 +310,10 @@ static Iterator<ManifestEntry.Status> statuses(ManifestEntry.Status... statuses)
return Iterators.forArray(statuses);
}

static Iterator<Long> seqs(long... seqs) {
return LongStream.of(seqs).iterator();
}

static Iterator<Long> ids(Long... ids) {
return Iterators.forArray(ids);
}
Expand All @@ -291,4 +325,33 @@ static Iterator<DataFile> files(DataFile... files) {
static Iterator<DataFile> files(ManifestFile manifest) {
return ManifestFiles.read(manifest, FILE_IO).iterator();
}

/**
* Used for assertions that only apply if the table version is v2.
*/
protected static class Assertions {
private final boolean enabled;

private Assertions(int validForVersion, int formatVersion) {
this.enabled = validForVersion == formatVersion;
}

void assertEquals(String context, int expected, int actual) {
if (enabled) {
Assert.assertEquals(context, expected, actual);
}
}

void assertEquals(String context, long expected, long actual) {
if (enabled) {
Assert.assertEquals(context, expected, actual);
}
}

void assertEquals(String context, Object expected, Object actual) {
if (enabled) {
Assert.assertEquals(context, expected, actual);
}
}
}
}
54 changes: 40 additions & 14 deletions core/src/test/java/org/apache/iceberg/TestFastAppend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,21 @@ public void testEmptyTableAppend() {

TableMetadata base = readMetadata();
Assert.assertNull("Should not have a current snapshot", base.currentSnapshot());
Assert.assertEquals("Table should start with last-sequence-number 0", 0, base.lastSequenceNumber());

Snapshot pending = table.newFastAppend()
table.newFastAppend()
.appendFile(FILE_A)
.appendFile(FILE_B)
.apply();
.commit();

Snapshot snap = table.currentSnapshot();

validateSnapshot(base.currentSnapshot(), snap, 1, FILE_A, FILE_B);

validateSnapshot(base.currentSnapshot(), pending, FILE_A, FILE_B);
V2Assert.assertEquals("Snapshot sequence number should be 1", 1, snap.sequenceNumber());
V2Assert.assertEquals("Last sequence number should be 1", 1, readMetadata().lastSequenceNumber());

V1Assert.assertEquals("Table should end with last-sequence-number 0", 0, base.lastSequenceNumber());
}

@Test
Expand All @@ -67,17 +75,25 @@ public void testEmptyTableAppendManifest() throws IOException {

TableMetadata base = readMetadata();
Assert.assertNull("Should not have a current snapshot", base.currentSnapshot());
Assert.assertEquals("Table should start with last-sequence-number 0", 0, base.lastSequenceNumber());

ManifestFile manifest = writeManifest(FILE_A, FILE_B);
Snapshot pending = table.newFastAppend()
table.newFastAppend()
.appendManifest(manifest)
.apply();
.commit();

validateSnapshot(base.currentSnapshot(), pending, FILE_A, FILE_B);
Snapshot snap = table.currentSnapshot();

validateSnapshot(base.currentSnapshot(), snap, 1, FILE_A, FILE_B);

// validate that the metadata summary is correct when using appendManifest
Assert.assertEquals("Summary metadata should include 2 added files",
"2", pending.summary().get("added-data-files"));
"2", snap.summary().get("added-data-files"));

V2Assert.assertEquals("Snapshot sequence number should be 1", 1, snap.sequenceNumber());
V2Assert.assertEquals("Last sequence number should be 1", 1, readMetadata().lastSequenceNumber());

V1Assert.assertEquals("Table should end with last-sequence-number 0", 0, base.lastSequenceNumber());
}

@Test
Expand All @@ -86,22 +102,32 @@ public void testEmptyTableAppendFilesAndManifest() throws IOException {

TableMetadata base = readMetadata();
Assert.assertNull("Should not have a current snapshot", base.currentSnapshot());
Assert.assertEquals("Table should start with last-sequence-number 0", 0, base.lastSequenceNumber());

ManifestFile manifest = writeManifest(FILE_A, FILE_B);
Snapshot pending = table.newFastAppend()
table.newFastAppend()
.appendFile(FILE_C)
.appendFile(FILE_D)
.appendManifest(manifest)
.apply();
.commit();

Snapshot snap = table.currentSnapshot();

long pendingId = pending.snapshotId();
long commitId = snap.snapshotId();

validateManifest(pending.manifests().get(0),
ids(pendingId, pendingId),
validateManifest(snap.manifests().get(0),
seqs(1, 1),
ids(commitId, commitId),
files(FILE_C, FILE_D));
validateManifest(pending.manifests().get(1),
ids(pendingId, pendingId),
validateManifest(snap.manifests().get(1),
seqs(1, 1),
ids(commitId, commitId),
files(FILE_A, FILE_B));

V2Assert.assertEquals("Snapshot sequence number should be 1", 1, snap.sequenceNumber());
V2Assert.assertEquals("Last sequence number should be 1", 1, readMetadata().lastSequenceNumber());

V1Assert.assertEquals("Table should end with last-sequence-number 0", 0, base.lastSequenceNumber());
}

@Test
Expand Down