Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
aihuaxu committed Dec 21, 2024
1 parent 4bcb86b commit b276d3f
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 24 deletions.
8 changes: 8 additions & 0 deletions api/src/main/java/org/apache/iceberg/types/Types.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ private Types() {}
private static final Pattern DECIMAL =
Pattern.compile("decimal\\(\\s*(\\d+)\\s*,\\s*(\\d+)\\s*\\)");

public static Type typeFromTypeString(String typeString) {
if (VariantType.get().toString().equalsIgnoreCase(typeString)) {
return Types.VariantType.get();
}

return Types.fromPrimitiveString(typeString);
}

public static PrimitiveType fromPrimitiveString(String typeString) {
String lowerTypeString = typeString.toLowerCase(Locale.ROOT);
if (TYPES.containsKey(lowerTypeString)) {
Expand Down
8 changes: 4 additions & 4 deletions api/src/test/java/org/apache/iceberg/types/TestTypeUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -607,17 +607,17 @@ public void testReassignOrRefreshIds(Type testType) {
required(10, "a", Types.IntegerType.get()),
Types.NestedField.required("c")
.withId(11)
.ofType(testType)
.ofType(Types.IntegerType.get())
.withInitialDefault(23)
.withWriteDefault(34)
.build(),
required(12, "B", Types.IntegerType.get())),
required(12, "B", testType)),
Sets.newHashSet(10));
Schema sourceSchema =
new Schema(
Lists.newArrayList(
required(1, "a", Types.IntegerType.get()),
required(15, "B", Types.IntegerType.get())));
required(15, "B", testType)));

Schema actualSchema = TypeUtil.reassignOrRefreshIds(schema, sourceSchema);
Schema expectedSchema =
Expand All @@ -630,7 +630,7 @@ public void testReassignOrRefreshIds(Type testType) {
.withInitialDefault(23)
.withWriteDefault(34)
.build(),
required(15, "B", Types.IntegerType.get())));
required(15, "B", testType)));

assertThat(actualSchema.asStruct()).isEqualTo(expectedSchema.asStruct());
}
Expand Down
7 changes: 1 addition & 6 deletions core/src/main/java/org/apache/iceberg/SchemaParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ private SchemaParser() {}
private static final String STRUCT = "struct";
private static final String LIST = "list";
private static final String MAP = "map";
private static final String VARIANT = "variant";
private static final String FIELDS = "fields";
private static final String ELEMENT = "element";
private static final String KEY = "key";
Expand Down Expand Up @@ -182,11 +181,7 @@ public static String toJson(Schema schema, boolean pretty) {

private static Type typeFromJson(JsonNode json) {
if (json.isTextual()) {
if (VARIANT.equalsIgnoreCase(json.asText())) {
return Types.VariantType.get();
}

return Types.fromPrimitiveString(json.asText());
return Types.typeFromTypeString(json.asText());
} else if (json.isObject()) {
JsonNode typeObj = json.get(TYPE);
if (typeObj != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@
*/
package org.apache.iceberg.avro;

import static org.apache.iceberg.types.Types.NestedField.required;
import static org.assertj.core.api.Assertions.assertThat;

import java.util.Collections;
import org.apache.avro.SchemaBuilder;
import org.apache.iceberg.Schema;
import org.apache.iceberg.types.Types;
import org.junit.jupiter.api.Test;

public class TestAvroSchemaProjection {
Expand Down Expand Up @@ -152,16 +150,4 @@ public void projectWithMapSchemaChanged() {
.as("Result of buildAvroProjection is missing some IDs")
.isFalse();
}

@Test
public void testVariantConversion() {
Schema schema = new Schema(required(1, "variantCol", Types.VariantType.get()));
org.apache.avro.Schema avroSchema = AvroSchemaUtil.convert(schema.asStruct());

org.apache.avro.Schema variantSchema = avroSchema.getField("variantCol").schema();
assertThat(variantSchema.getType()).isEqualTo(org.apache.avro.Schema.Type.RECORD);
assertThat(variantSchema.getFields().size()).isEqualTo(2);
assertThat(variantSchema.getField("metadata")).isNotNull();
assertThat(variantSchema.getField("value")).isNotNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -368,4 +368,17 @@ public void testFieldDocsArePreserved() {
Lists.newArrayList(Iterables.transform(origSchema.columns(), Types.NestedField::doc));
assertThat(fieldDocs).isEqualTo(origFieldDocs);
}

@Test
public void testVariantConversion() {
org.apache.iceberg.Schema schema =
new org.apache.iceberg.Schema(required(1, "variantCol", Types.VariantType.get()));
org.apache.avro.Schema avroSchema = AvroSchemaUtil.convert(schema.asStruct());

org.apache.avro.Schema variantSchema = avroSchema.getField("variantCol").schema();
assertThat(variantSchema.getType()).isEqualTo(org.apache.avro.Schema.Type.RECORD);
assertThat(variantSchema.getFields().size()).isEqualTo(2);
assertThat(variantSchema.getField("metadata")).isNotNull();
assertThat(variantSchema.getField("value")).isNotNull();
}
}

0 comments on commit b276d3f

Please sign in to comment.