[dossett]

11 years ago we needed this, but not anymore since we only support Hadoop 3.x. DirectCodecFactory is package private so these are not breaking changes.

[Fokko]

Looks like this changes a return type:

[ERROR] Failed to execute goal com.github.siom79.japicmp:japicmp-maven-plugin:0.26.5:cmp (default) on project parquet-hadoop: There is at least one incompatibility: org.apache.parquet.hadoop.DirectCodecFactory$DirectCodecPool$CodecPool.borrowDirectDecompressor():METHOD_RETURN_TYPE_CHANGED -> [Help 1]

[dossett]

Interesting! japicmp is being perhaps a little aggressive here since the method is a public class inside a package private class. But then again, that inner class should not really be public. Correcting that would itself be another japicmp warning, so I just fixed the changed return type.

[Fokko]

Thanks for the cleanup, this looks good to me. The reflection only ever guarded against Hadoop 1.x, and we already link against DirectDecompressionCodec directly in Lz4RawCodec, so this was dead weight. I ran TestDirectCodecFactory locally as well, all green.

One small behavior change worth being aware of: an IOException thrown by a native direct decompressor now surfaces as an IOException instead of being wrapped in a ParquetCompressionCodecException via InvocationTargetException. That is an improvement since it matches the heap path and the BytesDecompressor contract, just noting it.

[Fokko]

Pre-existing, but since you are here anyway: the argument on the next line should be "direct decompressor" instead of "compressor", this is the direct decompressor branch.

[Fokko]

Optional: since DirectCodecFactory is package-private, another way to deal with the japicmp complaint is to add excludes for org.apache.parquet.hadoop.DirectCodecFactory$DirectCodecPool$CodecPool#borrowDirectDecompressor() and org.apache.parquet.hadoop.DirectCodecFactory$DirectCodecPool#returnDirectDecompressor(java.lang.Object) to the japicmp config in the root pom. Then this can return DirectDecompressor and the cast in the FullDirectDecompressor constructor goes away. Keeping Object is fine with me too.

[dossett]

Thanks @Fokko, I went with your suggestions but did decide not to add to japicmp exception list and just stick with the prior return type.