Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Java] unwrap() in NettyArrowBuf causes breaks when debug / error messages are logged #35553

Closed
martin-traverse opened this issue May 11, 2023 · 1 comment · Fixed by #35554
Closed

Comments

@martin-traverse
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

The implementation of NettyArrowBuf unwrap() is throwing UnsupportedOperationException, but the ByteBuf interface says unwrap() should return null when a buffer cannot be unwrapped. Netty's AbstractByteBuf base class calls unwrap() in toString() and prints extra info if the result is non-null. Since toString() is called every time a debug or error message is printed, real errors are getting masked and enabling debug logging is likely to cause unexpected errors.

Seems like a simple fix just to return null from unwrap() in NettyArrowBuf? I will submit a PR with the change if someone wants to take a look.

Component(s)

Java

@martin-traverse
Copy link
Contributor Author

A couple of references

@lidavidm lidavidm changed the title unwrap() in NettyArrowBuf causes breaks when debug / error messages are logged [Java] unwrap() in NettyArrowBuf causes breaks when debug / error messages are logged May 11, 2023
lidavidm pushed a commit that referenced this issue May 11, 2023
Suggested fix for #35553

No API changes, this just brings the implementation in line with the API as specified by Netty.

CI for Java run successfully here (not sure if this is publically visible):

https://github.com/martin-traverse/arrow/actions/runs/4949700024

* Closes: #35553

Authored-by: Martin Traverse <martin.traverse@accenture.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
@lidavidm lidavidm added this to the 13.0.0 milestone May 11, 2023
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this issue May 15, 2023
Suggested fix for apache#35553

No API changes, this just brings the implementation in line with the API as specified by Netty.

CI for Java run successfully here (not sure if this is publically visible):

https://github.com/martin-traverse/arrow/actions/runs/4949700024

* Closes: apache#35553

Authored-by: Martin Traverse <martin.traverse@accenture.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this issue May 16, 2023
Suggested fix for apache#35553

No API changes, this just brings the implementation in line with the API as specified by Netty.

CI for Java run successfully here (not sure if this is publically visible):

https://github.com/martin-traverse/arrow/actions/runs/4949700024

* Closes: apache#35553

Authored-by: Martin Traverse <martin.traverse@accenture.com>
Signed-off-by: David Li <li.davidm96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants