feat: Handle complex/nested data types from a parquet file in Parquet Data Source: Struct, Repeated Struct, Maps and Timestamp of type SimpleGroup #148
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,94 @@ | ||
| package io.odpf.dagger.common.serde.parquet; | ||
|
|
||
| import org.apache.parquet.example.data.simple.SimpleGroup; | ||
| import org.apache.parquet.schema.GroupType; | ||
| import org.apache.parquet.schema.LogicalTypeAnnotation; | ||
| import org.apache.parquet.schema.Type; | ||
|
|
||
| import static org.apache.parquet.schema.Type.Repetition.OPTIONAL; | ||
| import static org.apache.parquet.schema.Type.Repetition.REPEATED; | ||
| import static org.apache.parquet.schema.Type.Repetition.REQUIRED; | ||
|
|
||
| public class SimpleGroupValidation { | ||
| public static boolean checkFieldExistsAndIsInitialized(SimpleGroup simpleGroup, String fieldName) { | ||
| return simpleGroup.getType().containsField(fieldName) && simpleGroup.getFieldRepetitionCount(fieldName) != 0; | ||
| } | ||
|
|
||
| /** | ||
| * This method checks if the map field inside the simple group is | ||
| * serialized using this legacy format: | ||
| * {@code | ||
| * <pre> | ||
| * repeated group <name> { | ||
| * <repetition-type> <data-type> key; | ||
| * <repetition-type> <data-type> value; | ||
| * } | ||
| * </pre> | ||
| * } | ||
| * The outer group is always repeated. key and value are constant field names. | ||
| * | ||
| * @param simpleGroup The SimpleGroup object inside which the map field is present | ||
| * @param fieldName The name of the map field | ||
| * @return true, if the map structure follows the spec and false otherwise. | ||
| */ | ||
| public static boolean checkIsLegacySimpleGroupMap(SimpleGroup simpleGroup, String fieldName) { | ||
| if (!(simpleGroup.getType().getType(fieldName) instanceof GroupType)) { | ||
| return false; | ||
| } | ||
| GroupType nestedMapGroupType = simpleGroup.getType().getType(fieldName).asGroupType(); | ||
| return nestedMapGroupType.isRepetition(Type.Repetition.REPEATED) | ||
| && nestedMapGroupType.getFieldCount() == 2 | ||
| && nestedMapGroupType.containsField("key") | ||
| && nestedMapGroupType.containsField("value"); | ||
| } | ||
|
|
||
| /** | ||
| * This method checks if the map field inside the simple group is | ||
| * serialized using this standard parquet map specification: | ||
| * {@code | ||
| * <pre> | ||
| * <repetition-type> group <name> (MAP) { | ||
| * repeated group key_value { | ||
| * required <data-type> key; | ||
| * <repetition-type> <data-type> value; | ||
| * } | ||
| * } | ||
| * </pre> | ||
| * } | ||
| * The validation checks below follow the <a href="https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#maps">Apache Parquet LogicalTypes Specification</a>for Maps. | ||
| * | ||
| * @param simpleGroup The SimpleGroup object inside which the map field is present | ||
| * @param fieldName The name of the map field | ||
| * @return true, if the map structure follows the spec and false otherwise. | ||
| */ | ||
| public static boolean checkIsStandardSimpleGroupMap(SimpleGroup simpleGroup, String fieldName) { | ||
|
Contributor
There was a problem hiding this comment. this looks more like a method in mapHandler rather than a static method here. Any particular rationale for not keeping it there?
Member
Author
There was a problem hiding this comment. As discussed, we can keep this for now |
||
| return applyMapFieldValidations(simpleGroup, fieldName) | ||
| && applyNestedKeyValueFieldValidations(simpleGroup, fieldName); | ||
| } | ||
|
|
||
| private static boolean applyMapFieldValidations(SimpleGroup simpleGroup, String fieldName) { | ||
| Type mapType = simpleGroup.getType().getType(fieldName); | ||
| if (mapType instanceof GroupType) { | ||
| GroupType mapGroupType = mapType.asGroupType(); | ||
| return (mapGroupType.getRepetition().equals(OPTIONAL) | ||
| || mapGroupType.isRepetition(REQUIRED)) | ||
| && mapGroupType.getLogicalTypeAnnotation().equals(LogicalTypeAnnotation.mapType()) | ||
| && mapGroupType.getFieldCount() == 1; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| private static boolean applyNestedKeyValueFieldValidations(SimpleGroup simpleGroup, String fieldName) { | ||
| GroupType mapGroupType = simpleGroup.getType().getType(fieldName).asGroupType(); | ||
| if (mapGroupType.containsField("key_value")) { | ||
| Type nestedKeyValueType = mapGroupType.getType("key_value"); | ||
| if (nestedKeyValueType instanceof GroupType) { | ||
| GroupType nestedKeyValueGroupType = nestedKeyValueType.asGroupType(); | ||
| return nestedKeyValueGroupType.isRepetition(REPEATED) | ||
| && nestedKeyValueGroupType.containsField("key") | ||
| && nestedKeyValueGroupType.getType("key").isRepetition(REQUIRED); | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks more like a method in mapHandler rather than a static method here. Any particular rationale for not keeping it there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are mainly 2 reasons:
ifbased checks and hence this needs to be tested for different corner cases. The unit tests are verbose. Here it is a public function and can be tested as it is, but when placed in MapHandler, this would ideally become a private function and hence the entire unit would need to be tested through the public interface oftransformFromParquet. This method already has a lot of big tests right now and moving this validator function to MapHandler would result in a lot of hard to understand tests. The MapHandler class is already quite big and I don't want to make it bigger.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, let's keep this for now