GH-731: Avro adapter, output dictionary-encoded fields as enums

[martin-traverse]

What's Changed

Updated ArrowToAvro to output dictionary-encoded string vectors as Avro enums, where possible. Apologies for the delay - busy as usual!

To output dict encoded vectors as enums, a dictionary provider must be supplied to the top level methods with all the required dictionaries. All dictionary values must be present when the schema is written, i.e. before the data blocks are produced. If data is being written as a schema followed by multiple blocks, values added to a dictionary in between blocks will not be included in the schema resulting in an invalid Avro file (in general supply an invalid dictionary mapping will result in invalid output).

Dictionary encoded fields are checked to ensure they are valid Avro enums. If the dictionary encoded field is not a string field, or the string values are not valid Avro enums, the field is decoded and output as literal values. This is done by calling DictionaryEncoder.decode(vector, dictionary), which will consume memory for the vector. An alternative approach would be to decode values one-by-one, however this would require a significant change to the producer pattern since the current producers expect concrete vectors of the output type. Another option would be to throw an error if there are dictionary-encoded vectors that are not string types, i.e. push the responsibility onto client code. I'm not sure which approach is best - happy to take any guidance and I will update the code accordingly.

To read enums back the current approach for decoding is unchanged (the AvroToArrow config has to be set up with a MapDictionaryProvider which is populated when data is read). The last part of the Avro work is to add the capability for reading / writing whole files block-by-block, so there is an opportunity to do something with the top level APIs there, for now the current API works and I've used it in the round trip tests.

Please let me know any feedback, happy to update as needed!

Closes #731.

[martin-traverse]

On reflection, simply decoding a dict-encoded vector is not an option because it will leak. I have pushed an update that will reject dict-encoded fields that are not valid Avro enums (check is during schema production).

I do think automatically decoding dict-encoded fields would be a nicer behaviour. Should we perhaps think about updating the producer pattern to allow for this? E.g. every producer could take an optional index vector, of type BaseIntVector. If that is present, current index is mapped through the index vector which resolves the dictionary index. Would this be ok?

[lidavidm]

On reflection, simply decoding a dict-encoded vector is not an option because it will leak. I have pushed an update that will reject dict-encoded fields that are not valid Avro enums (check is during schema production).

I do think automatically decoding dict-encoded fields would be a nicer behaviour. Should we perhaps think about updating the producer pattern to allow for this? E.g. every producer could take an optional index vector, of type BaseIntVector. If that is present, current index is mapped through the index vector which resolves the dictionary index. Would this be ok?

So essentially, the "actual" vector would be the dictionary values, and the index vector would contain the "original" encoded vector? I think that sounds OK to me.

[lidavidm]

(You could also perhaps create a non-owning vector implementation that does this unwrapping? Though likely that's more work.)

[martin-traverse]

So essentially, the "actual" vector would be the dictionary values, and the index vector would contain the "original" encoded vector? I think that sounds OK to me.

Yes - essentially I want to avoid replicating the type-specific logic. I tried it out and looked ok, just not sure what happens with resetValueVector() though. Another option.... could be a DictionaryDecodingProducer, then the vector type for that producer would be the index type, with a concrete producer as a child. Dictionaries can't be updated in Avro anyway.... I think it works :) Will update the PR later for review.

[martin-traverse]

Hi @lidavidm - I have added the dictionary decoding producer, which turned out to be very simple. Now any dictionary encoded fields that are not valid Avro enums will be automatically decoded and output as their concrete type. This does require running a regex over the dictionary entries, but that only has to happen once when the producers are set up.

I do think we will need to change the enum read, so dictionaries are populated during the schema phase rather than the data phase in order to read whole files with multiple blocks. I'd like to keep that change back and do it as part of the next PR, which will be read / write for whole files.

Assuming you are happy with both these points then I think this PR is ready for review :-)

[lidavidm]

Thanks - I will try to get to reviewing this

[lidavidm]

LGTM (just one nit), thanks! Sorry for the long wait

[lidavidm]

Wording nit, but this sounds like the encoded values are ints; instead the encoded values are arbitrary and the encoding is ints, right?