wgtmac commented on code in PR #1300:
URL: https://github.com/apache/parquet-mr/pull/1300#discussion_r1525684443
##########
parquet-avro/src/main/java/org/apache/parquet/avro/AvroRecordConverter.java:
##########
@@ -930,6 +930,7 @@ public void end() {
static boolean isElementType(Type repeatedType, Schema elementSchema) {
if (repeatedType.isPrimitive()
|| repeatedType.asGroupType().getFieldCount() > 1
+ || repeatedType.getName().equals("array")
Review Comment:
```
1. If the repeated field is not a group, then its type is the element type
and elements are required.
2. If the repeated field is a group with multiple fields, then its type is
the element type and elements are required.
3. If the repeated field is a group with one field and is named either array
or uses the LIST-annotated group's name with _tuple appended then the repeated
type is the element type and elements are required.
4. Otherwise, the repeated field's type is the element type with the
repeated field's repetition.
```
`repeatedType.getName().equals("array")` is partially for rule 3, and
`repeatedType.asGroupType().getType(0).isRepetition(REPEATED))` is for rule 4.
But rule 3 also states the _tuple style naming scheme should come with `the
repeated field is a group with one field`, isn't it? If the repeated field is a
group with multiple fields with `_tuple` appended, is it still a valid element
type?
##########
parquet-avro/src/test/java/org/apache/parquet/avro/TestArrayCompatibility.java:
##########
@@ -603,7 +604,7 @@ public void testAvroCompatOptionalGroupInList() throws
Exception {
public void testAvroCompatOptionalGroupInListWithSchema() throws Exception {
Path test = writeDirect(
"message AvroCompatOptionalGroupInListWithSchema {" + " optional
group locations (LIST) {"
- + " repeated group array {"
+ + " repeated group my_list {"
Review Comment:
I agree with you. Is it better to add a test case with the incorrect schema
and check it throws?
--
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]
