wgtmac commented on code in PR #3753:
URL: https://github.com/apache/parquet-java/pull/3753#discussion_r3921973251
##########
parquet-avro/src/main/java/org/apache/parquet/avro/AvroReadSupport.java:
##########
@@ -128,6 +136,18 @@ public ReadContext init(
MessageType projection = fileSchema;
Map<String, String> metadata = new LinkedHashMap<String, String>();
+ boolean autoDetectListStructure =
+ configuration.getBoolean(AUTO_DETECT_LIST_STRUCTURE,
AUTO_DETECT_LIST_STRUCTURE_DEFAULT);
+
+ if (autoDetectListStructure
+ && configuration.get(AvroWriteSupport.WRITE_OLD_LIST_STRUCTURE) == null
+ && configuration.get(AvroSchemaConverter.ADD_LIST_ELEMENT_RECORDS) ==
null) {
+ if (writesNewListStructure(fileSchema)) {
+ configuration.setBoolean(AvroWriteSupport.WRITE_OLD_LIST_STRUCTURE,
false);
Review Comment:
This writes the inferred mode back into the shared Configuration.
ParquetReader reuses that configuration across files, so after a 3-level file,
a later 2-level file (especially with a projection) is still converted as
3-level. Could this stay per-file/read-context instead of mutating the caller's
configuration?
##########
parquet-avro/src/main/java/org/apache/parquet/avro/AvroReadSupport.java:
##########
@@ -128,6 +136,18 @@ public ReadContext init(
MessageType projection = fileSchema;
Map<String, String> metadata = new LinkedHashMap<String, String>();
+ boolean autoDetectListStructure =
+ configuration.getBoolean(AUTO_DETECT_LIST_STRUCTURE,
AUTO_DETECT_LIST_STRUCTURE_DEFAULT);
+
+ if (autoDetectListStructure
+ && configuration.get(AvroWriteSupport.WRITE_OLD_LIST_STRUCTURE) == null
Review Comment:
Is it a good time to make a shift on the default value of
`AvroWriteSupport.WRITE_OLD_LIST_STRUCTURE`? It has caused a lot of troubles.
##########
parquet-avro/src/main/java/org/apache/parquet/avro/AvroReadSupport.java:
##########
@@ -230,4 +250,39 @@ private GenericData getDataModel(ParquetConfiguration
conf, Schema schema) {
return ReflectionUtils.newInstance(suppClass,
ConfigurationUtil.createHadoopConfiguration(conf))
.get();
}
+
+ private static boolean writesNewListStructure(MessageType schema) {
+ return Boolean.TRUE.equals(allListStructuresAreThreeLevel(schema));
+ }
+
+ // Given a Parquet schema, return true only if the schema:
+ // - contains one or more List fields
+ // - encodes every List field using 3-level list structure
+ private static Boolean allListStructuresAreThreeLevel(Type type) {
+ if (type.isPrimitive()) {
+ return null;
+ }
+ GroupType group = type.asGroupType();
+ if (group.getLogicalTypeAnnotation() instanceof
LogicalTypeAnnotation.ListLogicalTypeAnnotation) {
+ if (group.getFieldCount() != 1) {
+ return false;
+ }
+ Type repeated = group.getType(0);
+ return !repeated.isPrimitive()
Review Comment:
I may be misremembering the LIST compatibility rules, so I wanted to check
this edge case: could a legacy 2-level list also use the `list`/`element` names
and therefore look like this to `allListStructuresAreThreeLevel`? If so, would
auto-detect change its Avro shape unexpectedly, or is this case ruled out by
the spec or writer assumptions?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
