Skip to content

Commit

Permalink
add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
770120041 committed Dec 5, 2023
1 parent 7f685e6 commit f4b36e3
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 12 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/org/apache/iceberg/SchemaParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ private static String defaultValueToJsonString(byte[] bytes) {

private static String defaultValueToJsonString(Object value) {
try {
return JsonUtil.mapper().writeValueAsString(value);
return JsonUtil.mapper().writeValueAsString(AvroSchemaUtil.convertToJavaDefaultValue(value));
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}
Expand Down
16 changes: 10 additions & 6 deletions core/src/main/java/org/apache/iceberg/avro/TypeToSchema.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public Schema struct(Types.StructType struct, List<Schema> fieldSchemas) {
Object defaultValue = structField.hasDefaultValue() ? structField.getDefaultValue() :
(structField.isOptional() ? JsonProperties.NULL_VALUE : null);
Schema.Field field = new Schema.Field(fieldName, fieldSchemas.get(i), structField.doc(),
convertToJsonNull(defaultValue));
convertComplexNullToJsonNull(defaultValue));
if (!isValidFieldName) {
field.addProp(AvroSchemaUtil.ICEBERG_FIELD_NAME_PROP, origFieldName);
}
Expand Down Expand Up @@ -238,13 +238,15 @@ public Schema primitive(Type.PrimitiveType primitive) {

// This function ensures that all nested null are converted to JsonProperties.NULL_VALUE
// to make sure JacksonUtils.toJsonNode() converts them properly.
private Object convertToJsonNull(Object defaultValue) {
private Object convertComplexNullToJsonNull(Object defaultValue) {
if (defaultValue instanceof Map) {
for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) defaultValue).entrySet()) {
if (entry.getValue() instanceof Map || entry.getValue() instanceof Collection) {
entry.setValue(convertToJsonNull(entry.getValue()));
entry.setValue(convertComplexNullToJsonNull(entry.getValue()));
} else {
entry.setValue(JsonProperties.NULL_VALUE);
if (entry.getValue() == null) {
entry.setValue(JsonProperties.NULL_VALUE);
}
}
}
return defaultValue;
Expand All @@ -254,9 +256,11 @@ private Object convertToJsonNull(Object defaultValue) {

for (Object element : originalList) {
if (element instanceof Map || element instanceof Collection) {
copiedList.add(convertToJsonNull(element));
} else {
copiedList.add(convertComplexNullToJsonNull(element));
} else if (element == null) {
copiedList.add(JsonProperties.NULL_VALUE);
} else {
copiedList.add(element);
}
}
return copiedList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,4 +584,37 @@ public void testConversionOfRecordWithNestedSubElement() {
String jSchema = SchemaParser.toJson(iSchema);
org.apache.iceberg.Schema roundTripiSchema = SchemaParser.fromJson(jSchema);
}
@Test
public void testConversionOfRecordWithNestedSubElementWithNotNullDefaultValue() {
String schemaString = "{\n" +
" \"type\": \"record\",\n" +
" \"name\": \"OuterRecord\",\n" +
" \"fields\": [\n" +
" {\n" +
" \"name\": \"nestedRecord\",\n" +
" \"type\": {\n" +
" \"type\": \"record\",\n" +
" \"name\": \"InnerRecord\",\n" +
" \"fields\": [\n" +
" {\n" +
" \"name\": \"myArray\",\n" +
" \"type\": {\n" +
" \"type\": \"array\",\n" +
" \"items\": \"int\"\n" +
" },\n" +
" \"default\": [1, 2, 3]\n" +
" }\n" +
" ],\n" +
" \"default\": {\"myArray\": [1, 2, 3]}\n" +
" },\n" +
" \"default\": {\"myArray\": [1, 2, 3]}\n" +
" }\n" +
" ],\n" +
" \"default\": {\"nestedRecord\": {\"myArray\": [1, 2, 3]}}\n" +
"}";
Schema schema = new Schema.Parser().parse(schemaString);
org.apache.iceberg.Schema iSchema = AvroSchemaUtil.toIceberg(schema);
String jSchema = SchemaParser.toJson(iSchema);
org.apache.iceberg.Schema roundTripiSchema = SchemaParser.fromJson(jSchema);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public void testAvroDefaultValues() throws IOException {
* Test nested array with default null on complex types
* if the table contains non-primitive Avro types (InnerElement in the test below)
* as the first field and arrays of InnerElement as the second field,
* it previously leads to a NullPointerException when operating on the table.
* it leads to a NullPointerException when operating on the table.
*/
@Test
public void testNestedArrayWithDefaultNullOnComplexTypes() throws IOException {
Expand Down Expand Up @@ -270,18 +270,25 @@ public void testNestedArrayWithDefaultNullOnComplexTypes() throws IOException {
"}";
org.apache.avro.Schema writeSchema = new org.apache.avro.Schema.Parser().parse(writeSchemaString);
org.apache.iceberg.Schema icebergWriteSchema = AvroSchemaUtil.toIceberg(writeSchema);

List<GenericData.Record> expected = RandomData.generateList(icebergWriteSchema, 2, 0L);


Assert.assertTrue("Delete should succeed", testFile.delete());

// write records with initial writeSchema
try (FileAppender<GenericData.Record> writer = Avro.write(Files.localOutput(testFile))
.schema(icebergWriteSchema)
.named("test")
.build()) {
for (GenericData.Record rec : expected) {
writer.add(rec);
}
}
}


/*
* Test nested array with default null on complex types.
* This test differs from testNestedArrayWithDefaultNullOnComplexTypes on the type
* of InnerField1Param, it is a primitive type in this test.
* of InnerField1Param, when it is a primitive type, no NPE is thrown when operating on the table.
*/
@Test
public void testNestedArrayWithDefaultNullOnPrimitiveTypes() throws IOException {
Expand Down Expand Up @@ -347,6 +354,63 @@ public void testNestedArrayWithDefaultNullOnPrimitiveTypes() throws IOException


Assert.assertTrue("Delete should succeed", testFile.delete());

// write records with initial writeSchema
try (FileAppender<GenericData.Record> writer = Avro.write(Files.localOutput(testFile))
.schema(icebergWriteSchema)
.named("test")
.build()) {
for (GenericData.Record rec : expected) {
writer.add(rec);
}
}
}

@Test
public void testNestedArrayWithDefaultNullOnArrayTypes() throws IOException {
String writeSchemaString = "{\n" +
" \"type\": \"record\",\n" +
" \"name\": \"OuterRecord\",\n" +
" \"fields\": [\n" +
" {\n" +
" \"name\": \"nestedRecord\",\n" +
" \"type\": {\n" +
" \"type\": \"record\",\n" +
" \"name\": \"InnerRecord\",\n" +
" \"fields\": [\n" +
" {\n" +
" \"name\": \"myArray\",\n" +
" \"type\": {\n" +
" \"type\": \"array\",\n" +
" \"items\": \"int\"\n" +
" },\n" +
" \"default\": [1, 2, 3]\n" +
" }\n" +
" ],\n" +
" \"default\": {\"myArray\": [1, 2, 3]}\n" +
" },\n" +
" \"default\": {\"myArray\": [1, 2, 3]}\n" +
" }\n" +
" ],\n" +
" \"default\": {\"nestedRecord\": {\"myArray\": [1, 2, 3]}}\n" +
"}";
org.apache.avro.Schema writeSchema = new org.apache.avro.Schema.Parser().parse(writeSchemaString);
org.apache.iceberg.Schema icebergWriteSchema = AvroSchemaUtil.toIceberg(writeSchema);

List<GenericData.Record> expected = RandomData.generateList(icebergWriteSchema, 2, 0L);

File testFile = temp.newFile();
Assert.assertTrue("Delete should succeed", testFile.delete());

// write records with initial writeSchema
try (FileAppender<GenericData.Record> writer = Avro.write(Files.localOutput(testFile))
.schema(icebergWriteSchema)
.named("test")
.build()) {
for (GenericData.Record rec : expected) {
writer.add(rec);
}
}
}
}

0 comments on commit f4b36e3

Please sign in to comment.