Skip to content

Core: MetadataUpdateParser should write updates/removals fields rather than updated/removed #6317

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 1 commit into from
Dec 1, 2022
Merged

Core: MetadataUpdateParser should write updates/removals fields rather than updated/removed #6317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 34 additions & 6 deletions core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,16 @@ private MetadataUpdateParser() {}
private static final String MAX_REF_AGE_MS = "max-ref-age-ms";

// SetProperties
// the REST API Spec defines "updates" but we initially used "updated",
// thus we need to support reading both indefinitely
private static final String UPDATED = "updated";
private static final String UPDATES = "updates";

// RemoveProperties
// the REST API Spec defines "removals" but we initially used "removed",
// thus we need to support reading both indefinitely
private static final String REMOVED = "removed";
private static final String REMOVALS = "removals";

// SetLocation
private static final String LOCATION = "location";
Expand Down Expand Up @@ -368,13 +374,13 @@ private static void writeRemoveSnapshotRef(

private static void writeSetProperties(MetadataUpdate.SetProperties update, JsonGenerator gen)
throws IOException {
gen.writeFieldName(UPDATED);
gen.writeFieldName(UPDATES);
gen.writeObject(update.updated());
}

private static void writeRemoveProperties(
MetadataUpdate.RemoveProperties update, JsonGenerator gen) throws IOException {
gen.writeFieldName(REMOVED);
gen.writeFieldName(REMOVALS);
gen.writeObject(update.removed());
}

Expand Down Expand Up @@ -471,13 +477,35 @@ private static MetadataUpdate readRemoveSnapshotRef(JsonNode node) {
}

private static MetadataUpdate readSetProperties(JsonNode node) {
Map<String, String> updated = JsonUtil.getStringMap(UPDATED, node);
return new MetadataUpdate.SetProperties(updated);
Map<String, String> updates;

boolean hasLegacyField = node.has(UPDATED);
boolean hasUpdatesField = node.has(UPDATES);
if (hasLegacyField && hasUpdatesField) {
updates = JsonUtil.getStringMap(UPDATES, node);
} else if (hasLegacyField) {
updates = JsonUtil.getStringMap(UPDATED, node);
} else {
updates = JsonUtil.getStringMap(UPDATES, node);
}

return new MetadataUpdate.SetProperties(updates);
}

private static MetadataUpdate readRemoveProperties(JsonNode node) {
Set<String> removed = JsonUtil.getStringSet(REMOVED, node);
return new MetadataUpdate.RemoveProperties(removed);
Set<String> removals;

boolean hasLegacyField = node.has(REMOVED);
boolean hasRemovalsField = node.has(REMOVALS);
if (hasLegacyField && hasRemovalsField) {
removals = JsonUtil.getStringSet(REMOVALS, node);
} else if (hasLegacyField) {
removals = JsonUtil.getStringSet(REMOVED, node);
} else {
removals = JsonUtil.getStringSet(REMOVALS, node);
}

return new MetadataUpdate.RemoveProperties(removals);
}

private static MetadataUpdate readSetLocation(JsonNode node) {
Expand Down
31 changes: 29 additions & 2 deletions core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,9 +664,23 @@ public void testSetPropertiesFromJson() {
"prop1", "val1",
"prop2", "val2");
String propsMap = "{\"prop1\":\"val1\",\"prop2\":\"val2\"}";

// make sure reading "updated" & "updates" both work
String json = String.format("{\"action\":\"%s\",\"updated\":%s}", action, propsMap);
MetadataUpdate expected = new MetadataUpdate.SetProperties(props);
assertEquals(action, expected, MetadataUpdateParser.fromJson(json));

json = String.format("{\"action\":\"%s\",\"updates\":%s}", action, propsMap);
expected = new MetadataUpdate.SetProperties(props);
assertEquals(action, expected, MetadataUpdateParser.fromJson(json));

// if "updated" & "updates" are defined, then "updates" takes precedence
json =
String.format(
"{\"action\":\"%s\",\"updates\":%s,\"updated\":{\"propX\":\"valX\"}}",
action, propsMap);
expected = new MetadataUpdate.SetProperties(props);
assertEquals(action, expected, MetadataUpdateParser.fromJson(json));
}

@Test
Expand All @@ -692,7 +706,7 @@ public void testSetPropertiesToJson() {
"prop1", "val1",
"prop2", "val2");
String propsMap = "{\"prop1\":\"val1\",\"prop2\":\"val2\"}";
String expected = String.format("{\"action\":\"%s\",\"updated\":%s}", action, propsMap);
String expected = String.format("{\"action\":\"%s\",\"updates\":%s}", action, propsMap);
MetadataUpdate update = new MetadataUpdate.SetProperties(props);
String actual = MetadataUpdateParser.toJson(update);
Assert.assertEquals(
Expand All @@ -705,17 +719,30 @@ public void testRemovePropertiesFromJson() {
String action = MetadataUpdateParser.REMOVE_PROPERTIES;
Set<String> toRemove = ImmutableSet.of("prop1", "prop2");
String toRemoveAsJSON = "[\"prop1\",\"prop2\"]";

// make sure reading "removed" & "removals" both work
String json = String.format("{\"action\":\"%s\",\"removed\":%s}", action, toRemoveAsJSON);
MetadataUpdate expected = new MetadataUpdate.RemoveProperties(toRemove);
assertEquals(action, expected, MetadataUpdateParser.fromJson(json));

json = String.format("{\"action\":\"%s\",\"removals\":%s}", action, toRemoveAsJSON);
expected = new MetadataUpdate.RemoveProperties(toRemove);
assertEquals(action, expected, MetadataUpdateParser.fromJson(json));

// if "removed" & "removals" are defined, then "removals" takes precedence
json =
String.format(
"{\"action\":\"%s\",\"removals\":%s,\"removed\": [\"propX\"]}", action, toRemoveAsJSON);
expected = new MetadataUpdate.RemoveProperties(toRemove);
assertEquals(action, expected, MetadataUpdateParser.fromJson(json));
}

@Test
public void testRemovePropertiesToJson() {
String action = MetadataUpdateParser.REMOVE_PROPERTIES;
Set<String> toRemove = ImmutableSet.of("prop1", "prop2");
String toRemoveAsJSON = "[\"prop1\",\"prop2\"]";
String expected = String.format("{\"action\":\"%s\",\"removed\":%s}", action, toRemoveAsJSON);
String expected = String.format("{\"action\":\"%s\",\"removals\":%s}", action, toRemoveAsJSON);
MetadataUpdate update = new MetadataUpdate.RemoveProperties(toRemove);
String actual = MetadataUpdateParser.toJson(update);
Assert.assertEquals(
Expand Down