Skip to content

PARQUET-1879 MapKeyValue is not a valid Logical Type #798

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
gszadovszky merged 2 commits into from
Jul 6, 2020
Merged

PARQUET-1879 MapKeyValue is not a valid Logical Type #798

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
cc9cc5f
PARQUET-1879 MapKeyValue is not a valid Logical Type
Jun 23, 2020
7eddaec
PARQUET-1879 MapKeyValue is not a valid Logical Type
Jun 25, 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
10 changes: 5 additions & 5 deletions parquet-avro/src/test/java/org/apache/parquet/avro/TestAvroSchemaConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,13 @@ public void testAllTypes() throws Exception {
" }\n" +
" }\n" +
" required group mymap (MAP) {\n" +
" repeated group map (MAP_KEY_VALUE) {\n" +
" repeated group key_value (MAP_KEY_VALUE) {\n" +
" required binary key (UTF8);\n" +
" required int32 value;\n" +
" }\n" +
" }\n" +
" required group myemptymap (MAP) {\n" +
" repeated group map (MAP_KEY_VALUE) {\n" +
" repeated group key_value (MAP_KEY_VALUE) {\n" +
" required binary key (UTF8);\n" +
" required int32 value;\n" +
" }\n" +
Expand Down Expand Up @@ -249,13 +249,13 @@ public void testAllTypesOldListBehavior() throws Exception {
" repeated int32 array;\n" +
" }\n" +
" required group mymap (MAP) {\n" +
" repeated group map (MAP_KEY_VALUE) {\n" +
" repeated group key_value (MAP_KEY_VALUE) {\n" +
" required binary key (UTF8);\n" +
" required int32 value;\n" +
" }\n" +
" }\n" +
" required group myemptymap (MAP) {\n" +
" repeated group map (MAP_KEY_VALUE) {\n" +
" repeated group key_value (MAP_KEY_VALUE) {\n" +
" required binary key (UTF8);\n" +
" required int32 value;\n" +
" }\n" +
Expand Down Expand Up @@ -320,7 +320,7 @@ public void testOptionalMapValue() throws Exception {
schema,
"message record1 {\n" +
" required group myintmap (MAP) {\n" +
" repeated group map (MAP_KEY_VALUE) {\n" +
" repeated group key_value (MAP_KEY_VALUE) {\n" +
" required binary key (UTF8);\n" +
" optional int32 value;\n" +
" }\n" +
Expand Down
4 changes: 2 additions & 2 deletions parquet-avro/src/test/java/org/apache/parquet/avro/TestReadWrite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ public void write(Map<String, Object> record) {

recordConsumer.startField("mymap", index);
recordConsumer.startGroup();
recordConsumer.startField("map", 0);
recordConsumer.startField("key_value", 0);
recordConsumer.startGroup();
Map<String, Integer> mymap = (Map<String, Integer>) record.get("mymap");
recordConsumer.startField("key", 0);
Expand All @@ -553,7 +553,7 @@ public void write(Map<String, Object> record) {
}
recordConsumer.endField("value", 1);
recordConsumer.endGroup();
recordConsumer.endField("map", 0);
recordConsumer.endField("key_value", 0);
recordConsumer.endGroup();
recordConsumer.endField("mymap", index++);

Expand Down
5 changes: 3 additions & 2 deletions parquet-column/src/main/java/org/apache/parquet/schema/ConversionPatterns.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
*/
public abstract class ConversionPatterns {

static final String MAP_REPEATED_NAME = "key_value";
private static final String ELEMENT_NAME = "element";

/**
Expand All @@ -49,15 +50,15 @@ private static GroupType listWrapper(Repetition repetition, String alias, Logica
}

public static GroupType mapType(Repetition repetition, String alias, Type keyType, Type valueType) {
return mapType(repetition, alias, "map", keyType, valueType);
return mapType(repetition, alias, MAP_REPEATED_NAME, keyType, valueType);
}

public static GroupType stringKeyMapType(Repetition repetition, String alias, String mapAlias, Type valueType) {
return mapType(repetition, alias, mapAlias, new PrimitiveType(Repetition.REQUIRED, PrimitiveTypeName.BINARY, "key", stringType()), valueType);
}

public static GroupType stringKeyMapType(Repetition repetition, String alias, Type valueType) {
return stringKeyMapType(repetition, alias, "map", valueType);
return stringKeyMapType(repetition, alias, MAP_REPEATED_NAME, valueType);
}

public static GroupType mapType(Repetition repetition, String alias, String mapAlias, Type keyType, Type valueType) {
Expand Down
8 changes: 5 additions & 3 deletions parquet-column/src/main/java/org/apache/parquet/schema/Types.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.apache.parquet.schema.LogicalTypeAnnotation.mapType;

/**
* This class provides fluent builders that produce Parquet schema Types.
* <p>
Expand Down Expand Up @@ -1179,18 +1181,18 @@ protected Type build(String name) {
keyType = STRING_KEY;
}

GroupBuilder<GroupType> builder = buildGroup(repetition).as(OriginalType.MAP);
GroupBuilder<GroupType> builder = buildGroup(repetition).as(mapType());
if (id != null) {
builder.id(id.intValue());
}

if (valueType != null) {
return builder
.repeatedGroup().addFields(keyType, valueType).named("map")
.repeatedGroup().addFields(keyType, valueType).named(ConversionPatterns.MAP_REPEATED_NAME)
.named(name);
} else {
return builder
.repeatedGroup().addFields(keyType).named("map")
.repeatedGroup().addFields(keyType).named(ConversionPatterns.MAP_REPEATED_NAME)
.named(name);
}
}
Expand Down
56 changes: 28 additions & 28 deletions parquet-column/src/test/java/org/apache/parquet/schema/TestTypeBuilders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ public void testRequiredMap() {
typeList.add(new PrimitiveType(REQUIRED, INT64, "key"));
typeList.add(new PrimitiveType(REQUIRED, INT64, "value"));
GroupType expected = new GroupType(REQUIRED, "myMap", OriginalType.MAP, new GroupType(REPEATED,
"map",
"key_value",
typeList));
GroupType actual = Types.requiredMap()
.key(INT64)
Expand All @@ -501,7 +501,7 @@ public void testOptionalMap() {
typeList.add(new PrimitiveType(REQUIRED, INT64, "key"));
typeList.add(new PrimitiveType(REQUIRED, INT64, "value"));
GroupType expected = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED,
"map",
"key_value",
typeList));
GroupType actual = Types.optionalMap()
.key(INT64)
Expand All @@ -515,7 +515,7 @@ public void testMapWithRequiredValue() {
List<Type> typeList = new ArrayList<>();
typeList.add(new PrimitiveType(REQUIRED, INT64, "key"));
typeList.add(new PrimitiveType(REQUIRED, INT64, "value"));
GroupType map = new GroupType(REQUIRED, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(REQUIRED, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));
MessageType expected = new MessageType("mapParent", map);
GroupType actual = Types.buildMessage().requiredMap()
Expand All @@ -530,7 +530,7 @@ public void testMapWithOptionalValue() {
List<Type> typeList = new ArrayList<>();
typeList.add(new PrimitiveType(REQUIRED, INT64, "key"));
typeList.add(new PrimitiveType(OPTIONAL, INT64, "value"));
GroupType map = new GroupType(REQUIRED, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(REQUIRED, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));
MessageType expected = new MessageType("mapParent", map);
GroupType actual = Types.buildMessage().requiredMap()
Expand All @@ -554,7 +554,7 @@ public void testMapWithGroupKeyAndOptionalGroupValue() {
valueFields.add(new PrimitiveType(OPTIONAL, INT32, "two"));
typeList.add(new GroupType(OPTIONAL, "value", valueFields));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

GroupType actual = Types.optionalMap()
Expand Down Expand Up @@ -582,7 +582,7 @@ public void testMapWithGroupKeyAndRequiredGroupValue() {
valueFields.add(new PrimitiveType(OPTIONAL, INT32, "two"));
typeList.add(new GroupType(REQUIRED, "value", valueFields));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -611,7 +611,7 @@ public void testMapWithGroupKeyAndOptionalValue() {

typeList.add(new PrimitiveType(OPTIONAL, DOUBLE, "value"));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -637,7 +637,7 @@ public void testMapWithGroupKeyAndRequiredValue() {

typeList.add(new PrimitiveType(REQUIRED, DOUBLE, "value"));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -666,7 +666,7 @@ public void testMapWithOptionalGroupValue() {
valueFields.add(new PrimitiveType(OPTIONAL, INT32, "two"));
typeList.add(new GroupType(OPTIONAL, "value", valueFields));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -693,7 +693,7 @@ public void testMapWithRequiredGroupValue() {
valueFields.add(new PrimitiveType(OPTIONAL, INT32, "two"));
typeList.add(new GroupType(REQUIRED, "value", valueFields));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -727,7 +727,7 @@ public void testMapWithNestedGroupKeyAndNestedGroupValue() {
valueFields.add(new PrimitiveType(OPTIONAL, INT32, "two"));
typeList.add(new GroupType(OPTIONAL, "value", valueFields));

GroupType map = new GroupType(REQUIRED, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(REQUIRED, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -761,7 +761,7 @@ public void testMapWithRequiredListValue() {
"list",
new PrimitiveType(OPTIONAL, INT64, "element"))));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -786,7 +786,7 @@ public void testMapWithOptionalListValue() {
"list",
new PrimitiveType(OPTIONAL, INT64, "element"))));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -811,9 +811,9 @@ public void testMapWithRequiredMapValue() {

typeList.add(new PrimitiveType(REQUIRED, INT64, "key"));
typeList.add(new GroupType(REQUIRED, "value", OriginalType.MAP,
new GroupType(REPEATED, "map", innerMapTypeList)));
new GroupType(REPEATED, "key_value", innerMapTypeList)));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -839,9 +839,9 @@ public void testMapWithOptionalMapValue() {

typeList.add(new PrimitiveType(REQUIRED, INT64, "key"));
typeList.add(new GroupType(OPTIONAL, "value", OriginalType.MAP,
new GroupType(REPEATED, "map", innerMapTypeList)));
new GroupType(REPEATED, "key_value", innerMapTypeList)));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -869,7 +869,7 @@ public void testMapWithGroupKeyAndRequiredListValue() {
"list",
new PrimitiveType(OPTIONAL, INT64, "element"))));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -898,7 +898,7 @@ public void testMapWithGroupKeyAndOptionalListValue() {
"list",
new PrimitiveType(OPTIONAL, INT64, "element"))));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -928,9 +928,9 @@ public void testMapWithGroupKeyAndRequiredMapValue() {
"first"
)));
typeList.add(new GroupType(REQUIRED, "value", OriginalType.MAP,
new GroupType(REPEATED, "map", innerMapTypeList)));
new GroupType(REPEATED, "key_value", innerMapTypeList)));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand Down Expand Up @@ -961,9 +961,9 @@ public void testMapWithGroupKeyAndOptionalMapValue() {
"first"
)));
typeList.add(new GroupType(OPTIONAL, "value", OriginalType.MAP,
new GroupType(REPEATED, "map", innerMapTypeList)));
new GroupType(REPEATED, "key_value", innerMapTypeList)));

GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -986,7 +986,7 @@ public void testMapWithNullValue() {
List<Type> typeList = new ArrayList<>();

typeList.add(new PrimitiveType(REQUIRED, INT64, "key"));
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -1004,7 +1004,7 @@ public void testMapWithDefaultKeyAndNullValue() {
List<Type> typeList = new ArrayList<>();

typeList.add(new PrimitiveType(REQUIRED, BINARY, "key", OriginalType.UTF8));
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "map",
GroupType map = new GroupType(OPTIONAL, "myMap", OriginalType.MAP, new GroupType(REPEATED, "key_value",
typeList));

MessageType expected = new MessageType("mapParent", map);
Expand All @@ -1022,7 +1022,7 @@ public void testMapWithPreBuiltKeyAndValueTypes() {
Type valueType = Types.required(BOOLEAN).named("value");

GroupType map = new GroupType(REQUIRED, "myMap", OriginalType.MAP,
new GroupType(REPEATED, "map", new Type[] {
new GroupType(REPEATED, "key_value", new Type[] {
keyType,
valueType
}));
Expand Down Expand Up @@ -1204,7 +1204,7 @@ public void testRequiredMapWithinList() {
List<Type> fields = new ArrayList<>();
fields.add(new GroupType(REQUIRED, "element", OriginalType.MAP,
new GroupType(REPEATED,
"map",
"key_value",
innerFields)));
GroupType expected = new GroupType(OPTIONAL, "myList", OriginalType.LIST,
new GroupType(REPEATED,
Expand All @@ -1230,7 +1230,7 @@ public void testOptionalMapWithinList() {
List<Type> fields = new ArrayList<>();
fields.add(new GroupType(OPTIONAL, "element", OriginalType.MAP,
new GroupType(REPEATED,
"map",
"key_value",
innerFields)));
GroupType expected = new GroupType(OPTIONAL, "myList", OriginalType.LIST,
new GroupType(REPEATED,
Expand Down
7 changes: 1 addition & 6 deletions ...et-hadoop/src/main/java/org/apache/parquet/format/converter/ParquetMetadataConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ private void visitChildren(final List<SchemaElement> result,
}

LogicalType convertToLogicalType(LogicalTypeAnnotation logicalTypeAnnotation) {
return logicalTypeAnnotation.accept(LOGICAL_TYPE_ANNOTATION_VISITOR).get();
return logicalTypeAnnotation.accept(LOGICAL_TYPE_ANNOTATION_VISITOR).orElse(null);
}

ConvertedType convertToConvertedType(LogicalTypeAnnotation logicalTypeAnnotation) {
Expand Down Expand Up @@ -464,11 +464,6 @@ public Optional<LogicalType> visit(UUIDLogicalTypeAnnotation uuidLogicalType) {
public Optional<LogicalType> visit(LogicalTypeAnnotation.IntervalLogicalTypeAnnotation intervalLogicalType) {
return of(LogicalType.UNKNOWN(new NullType()));
}

@Override
public Optional<LogicalType> visit(LogicalTypeAnnotation.MapKeyValueTypeAnnotation mapKeyValueLogicalType) {
return of(LogicalType.UNKNOWN(new NullType()));
}
}

private void addRowGroup(ParquetMetadata parquetMetadata, List<RowGroup> rowGroups, BlockMetaData block) {
Expand Down
Loading