steveloughran commented on PR #3589: URL: https://github.com/apache/parquet-java/pull/3589#issuecomment-4556525394
NPE cause is [THRIFT-5916](https://issues.apache.org/jira/browse/THRIFT-5916) Bring Java recursion limit support at parity with C++; https://github.com/apache/thrift/pull/3287 commit #66dae3f is the "least change to existing code" patch, not necessarily the most elegant. It provides enough of a stub class to stop thrift blowing up. The alternative is for ParquetProtocol and BufferedProtocolReadToWrite.NullProtocol to * Implement incrementRecursionDepth()/decrementRecursionDepth() as no-ops * Override all the implementations of checkReadBytesAvailable() to be no-ops. Good: less stubbing of thrift internals Bad: - duplicate implementation of no-op methods. They're only no-op methods though... - the transport is still null, so risk of NPEs surfacing if new checks are added. Either way there's a risk of regressions if they add more checks in future releases. There's also the open issue "should there be a depth limit?"; we're adding one when hardening variants after all. But there we can be fairly confident there are no large datasets using the type. If thrift cpp enforces a depth of 64, then so will parquet-cpp, won't it? In which case modifying this PR to use the default depth of 64 would be consistent. -- 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]
