clairemcginty commented on code in PR #3753:
URL: https://github.com/apache/parquet-java/pull/3753#discussion_r3926695264
##########
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:
Yeah - I was a bit worried about that. Mutating the caller conf is simplest
because we want to have these properties set in both `init()` (when computing
the projection) and `prepareForRead()` (when computing `avroSchema` if it's not
already set in file footer/read conf). What do you think about modifying
`metadata` instead of `conf`? like:
```diff
public ReadContext init(
ParquetConfiguration configuration, Map<String, String>
keyValueMetaData, MessageType fileSchema) {
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);
-
configuration.setBoolean(AvroSchemaConverter.ADD_LIST_ELEMENT_RECORDS, false);
+ metadata.put("inferred." +
AvroWriteSupport.WRITE_OLD_LIST_STRUCTURE, "false");
+ metadata.put("inferred." +
AvroSchemaConverter.ADD_LIST_ELEMENT_RECORDS, "false");
}
}
+ // ...read those properties in prepareForRead() and apply to copied
Configuration
```
Or - we could just re-compute `writesNewListStructure(fileSchema)` in both
`init()` and `prepareForRead()`. that might be more straightforward overall
🤷♀️
--
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]
