gszadovszky commented on a change in pull request #561:
URL: https://github.com/apache/parquet-mr/pull/561#discussion_r467864366
##########
File path:
parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoMessageConverter.java
##########
@@ -222,11 +263,37 @@ public ProtoEnumConverter(ParentValueContainer parent,
Descriptors.FieldDescript
Descriptors.EnumValueDescriptor protoValue = enumLookup.get(binaryValue);
if (protoValue == null) {
- Set<Binary> knownValues = enumLookup.keySet();
- String msg = "Illegal enum value \"" + binaryValue + "\""
- + " in protocol buffer \"" + fieldType.getFullName() + "\""
- + " legal values are: \"" + knownValues + "\"";
- throw new InvalidRecordException(msg);
+ // in case of unknown enum value, protobuf is creating new
EnumValueDescriptor with the unknown number
+ // and name as following "UNKNOWN_ENUM_VALUE_" + parent.getName() +
"_" + number
+ // so the idea is to parse the name for data created by parquet-proto
before this patch
+ String unknownLabel = new String(binaryValue.getBytes());
Review comment:
I would suggest using `Binary.toStringUsingUTF8()` instead.
##########
File path:
parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoMessageConverter.java
##########
@@ -190,25 +211,45 @@ private Converter newScalarConverter(ParentValueContainer
pvc, Message.Builder p
private final Map<Binary, Descriptors.EnumValueDescriptor> enumLookup;
private Descriptors.EnumValueDescriptor[] dict;
private final ParentValueContainer parent;
+ private final Descriptors.EnumDescriptor enumType;
+ private final String unknownEnumPrefix;
+ private final boolean acceptUnknownEnum;
public ProtoEnumConverter(ParentValueContainer parent,
Descriptors.FieldDescriptor fieldType) {
this.parent = parent;
this.fieldType = fieldType;
- this.enumLookup = makeLookupStructure(fieldType);
+ this.enumType = fieldType.getEnumType();
+ this.enumLookup = makeLookupStructure(enumType);
+ unknownEnumPrefix = "UNKNOWN_ENUM_VALUE_" + enumType.getName() + "_";
+ acceptUnknownEnum = conf.getBoolean(CONFIG_ACCEPT_UNKNOWN_ENUM, false);
}
/**
* Fills lookup structure for translating between parquet enum values and
Protocol buffer enum values.
* */
- private Map<Binary, Descriptors.EnumValueDescriptor>
makeLookupStructure(Descriptors.FieldDescriptor enumFieldType) {
- Descriptors.EnumDescriptor enumType = enumFieldType.getEnumType();
+ private Map<Binary, Descriptors.EnumValueDescriptor>
makeLookupStructure(Descriptors.EnumDescriptor enumType) {
Map<Binary, Descriptors.EnumValueDescriptor> lookupStructure = new
HashMap<Binary, Descriptors.EnumValueDescriptor>();
- List<Descriptors.EnumValueDescriptor> enumValues = enumType.getValues();
+ if (extraMetadata.containsKey(METADATA_ENUM_PREFIX +
enumType.getFullName())) {
+ String enumNameNumberPairs = extraMetadata.get(METADATA_ENUM_PREFIX +
enumType.getFullName());
+ if (StringUtils.isBlank(enumNameNumberPairs)) {
+ LOG.info("No enum is written for " + enumType.getFullName());
Review comment:
As far as I understand this one will be logged quite often. We've
already had some about logging too much. I would suggest having this on debug
level instead.
##########
File path:
parquet-protobuf/src/main/java/org/apache/parquet/proto/ProtoMessageConverter.java
##########
@@ -22,7 +22,10 @@
import com.google.protobuf.Descriptors;
import com.google.protobuf.Message;
import com.twitter.elephantbird.util.Protobufs;
+import org.apache.commons.lang.StringUtils;
Review comment:
There is no direct dependency to `commons-lang` in `parquet-protobuf` so
I would not start using it for such a simple functionality.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
