ARROW-10174: [Java] Fix reading/writing dict structs

[HedgehogCode]

When translating between the memory FieldType and message FieldType for
dictionary encoded vectors the children of the dictionary field were not
handled correctly.

• When going from memory format to message format the Field must have the
  children of the dictionary field.
• When going from message format to memory format the Field must have no
  children but the dictionary must have the mapped children

[github-actions]

https://issues.apache.org/jira/browse/ARROW-10174

[emkornfield]

@liyafan82 do you have time to review?

[liyafan82]

@liyafan82 do you have time to review?

@emkornfield Sure. I will take a look in one or two days.

[liyafan82]

Maybe we need to verify the encoded vector?

[HedgehogCode]

No. Encoding of struct vectors is tested in TestDictionaryVector#testEncodeStruct. The encoded vector should be fine.

[liyafan82]

should we validate the read dictionary vector?

[HedgehogCode]

Yes. I added a check.

[liyafan82]

We need to consider the null values in v?

[HedgehogCode]

Yes, why not. I added an if switch for them.

[liyafan82]

Maybe we also need to set the value count for the child vector

[HedgehogCode]

No. The struct vector sets the value count of its children in #setValueCount.

[HedgehogCode]

I implemented the changes and force pushed them.

Thank you for the advice with the RangeEqualsVisitor. Using it I discovered another bug in DictionaryUtility. The field for the Dictionary was always created with nullable=false but this is incorrect because dictionaries can be nullable. Therefore, I replaced this with the nullability read from the schema.

[liyafan82]

we should call setIndexDefined only if v.get(i) != null?

[HedgehogCode]

I am not sure.
The current implementation does not allow setting any struct element to null. If we only call #setIndexDefined if the child element is not null the implementation would not allow (non null) structs with all child values beeing null. I am okay with both but I think we should keep the API simple because this method is just there to make tests shorter.

[liyafan82]

For a StructVector, we can set an element to null. For its super class (NonNullableStructVector), we cannot set an element to null.

Here, I generally prefer setting an element to null if all sub-elements are null, because ValueVectorDataPopulator is a generally-purpose class, and we may use it to test scenarios with null elements.

[HedgehogCode]

I see. I changed it.

[liyafan82]

it would be beneficial to close them through a try-with-resource clause to avoid resource leak?

[HedgehogCode]

Yes, I changed it. I did not do it before because I did not want the deep nesting.

[liyafan82]

I see. Deep nesting is not good-looking.

[liyafan82]

LGTM, will merge soon if there are no more comments.

[liyafan82]

Merging. Thanks for your effort. @HedgehogCode