Skip to content

Commit

Permalink
feat(java): Standardize core exceptions to AdbcException
Browse files Browse the repository at this point in the history
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 #2237
  • Loading branch information
laurentgo committed Oct 14, 2024
1 parent 368f772 commit 841708e
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.arrow.adbc.core;

import java.util.Arrays;

/**
* An ADBC resource which can be closed.
*/
public interface AdbcCloseable extends AutoCloseable {
@Override
void close() throws AdbcException;

/**
* Closes all autoCloseables if not null and suppresses subsequent exceptions if more than one.
* @param autoCloseables the closeables to close
*/
public static void close(AdbcCloseable... adbcCloseables) throws AdbcException {
close(Arrays.asList(adbcCloseables));
}

/**
* Closes all {@link AdbcCloseable} instances if not null and suppresses subsequent exceptions if more than one.
* @param ac the closeables to close
*/
public static void close(Iterable<? extends AdbcCloseable> ac) throws AdbcException {
// this method can be called on a single object if it implements Iterable<AutoCloseable>
// like for example VectorContainer make sure we handle that properly
if (ac == null) {
return;
} else if (ac instanceof AdbcCloseable) {
((AdbcCloseable) ac).close();
return;
}

AdbcException topLevelException = null;
for (AdbcCloseable closeable : ac) {
try {
if (closeable != null) {
closeable.close();
}
} catch (AdbcException e) {
if (topLevelException == null) {
topLevelException = e;
} else if (e != topLevelException) {
topLevelException.addSuppressed(e);
}
}
}
if (topLevelException != null) {
throw topLevelException;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* <p>Connections are not required to be thread-safe, but they can be used from multiple threads so
* long as clients take care to serialize accesses to a connection.
*/
public interface AdbcConnection extends AutoCloseable, AdbcOptions {
public interface AdbcConnection extends AdbcCloseable, AdbcOptions {
/**
* Cancel execution of a query.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
* remote/networked databases, for in-memory databases, this object provides an explicit point of
* ownership.
*/
public interface AdbcDatabase extends AutoCloseable, AdbcOptions {
public interface AdbcDatabase extends AdbcCloseable, AdbcOptions {
/** Create a new connection to the database. */
AdbcConnection connect() throws AdbcException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* <p>Statements are not required to be thread-safe, but they can be used from multiple threads so
* long as clients take care to serialize accesses to a statement.
*/
public interface AdbcStatement extends AutoCloseable, AdbcOptions {
public interface AdbcStatement extends AdbcCloseable, AdbcOptions {
/**
* Cancel execution of a query.
*
Expand Down Expand Up @@ -181,7 +181,7 @@ default Schema getParameterSchema() throws AdbcException {
void prepare() throws AdbcException;

/** The result of executing a query with a result set. */
class QueryResult implements AutoCloseable {
class QueryResult implements AdbcCloseable {
private final long affectedRows;

private final ArrowReader reader;
Expand All @@ -206,8 +206,12 @@ public String toString() {

/** Close the contained reader. */
@Override
public void close() throws IOException {
public void close() throws AdbcException {
try {
reader.close();
} catch (IOException e) {
throw AdbcException.io(e.getMessage()).withCause(e);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,15 @@ public void setAutoCommit(boolean enableAutoCommit) throws AdbcException {
}

@Override
public void close() throws Exception {
public void close() throws AdbcException {
clientCache.invalidateAll();
AutoCloseables.close(client, allocator);
try {
AutoCloseables.close(client, allocator);
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
throw AdbcException.io(e.getMessage()).withCause(e);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public AdbcConnection connect() throws AdbcException {
}

@Override
public void close() throws Exception {}
public void close() throws AdbcException {}

@Override
public String toString() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,10 @@ public void prepare() throws AdbcException {
}

@Override
public void close() throws Exception {
public void close() throws AdbcException {
// TODO(https://github.com/apache/arrow/issues/39814): this is annotated wrongly upstream
if (preparedStatement != null) {
AutoCloseables.close(preparedStatement);
preparedStatement.close();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,8 +508,14 @@ public void setIsolationLevel(IsolationLevel level) throws AdbcException {
}

@Override
public void close() throws Exception {
AutoCloseables.close(connection, allocator);
public void close() throws AdbcException {
try {
AutoCloseables.close(connection, allocator);
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
throw AdbcException.io(e.getMessage()).withCause(e);
}
}

private void checkAutoCommit() throws AdbcException, SQLException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,13 @@ public AdbcConnection connect() throws AdbcException {
}

@Override
public void close() throws Exception {
public void close() throws AdbcException {
if (connection != null) {
connection.close();
try {
connection.close();
} catch (SQLException e) {
throw AdbcException.io(e.getMessage()).withCause(e);
}
}
connection = null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,14 @@ public void prepare() throws AdbcException {
}

@Override
public void close() throws Exception {
AutoCloseables.close(reader, resultSet, statement);
public void close() throws AdbcException {
try {
AutoCloseables.close(reader, resultSet, statement);
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
throw AdbcException.io(e.getMessage()).withCause(e);
}
}

private static final class BulkState {
Expand Down

0 comments on commit 841708e

Please sign in to comment.