-
Notifications
You must be signed in to change notification settings - Fork 97
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
feat(java): Standardize core exceptions to AdbcException #2247
base: main
Are you sure you want to change the base?
Conversation
ADBC base classes/interfaces are not consistent in using `AdbcException` for all their methods (sometimes `Exception` or `IOException` are used). Introduces `AdbcCloseable` which is a subinterface of `AutoCloseable` throwing `AdbcException` instead of `Exception` and replace usage of `AutoCloseable` in core interfaces with `AdbcCloseable` Also replaces use of `IOException` in `QueryResult` with `AdbcException`. Fixes apache#2237
We wanted to treat @zeroshade @paleolimbot any opinions here? |
Sorry, didn't know the rules but I don't mind voting on it and/or start a discussion on arrow-devel. I guess while I was getting familiar with the code I had a couple of questions about some of the interfaces |
What questions? |
I am not familiar with the Java conventions around this kind of thing, although it seems like this is not any change in semantics around calling any particular method, just an improvement in error handling. This is a breaking change because somebody who implemented a driver in Java would have to update their |
Yes, and potentially users would have to update their |
Got it. It does seem like a good change to do and a good one to do sooner than later (e.g., before we wrap all the C drivers and make them accessible in Java). It does seem "spec level", although I am not sure that bumping a major version would communicate that change very well. If it's possible to make this change before we make all the C drivers available in Java I think it would be a good thing? |
Ok. I'd be OK with bending the 'rules' here a bit just so long as something makes it to the mailing list to notify people |
ADBC base classes/interfaces are not consistent in using
AdbcException
for all their methods (sometimesException
orIOException
are used).Introduces
AdbcCloseable
which is a subinterface ofAutoCloseable
throwingAdbcException
instead ofException
and replace usage ofAutoCloseable
in core interfaces withAdbcCloseable
Also replaces use of
IOException
inQueryResult
withAdbcException
.Fixes #2237