Skip to content

Commit b9f4dc1

Browse files
authored
[ES-1717770] Fix timeout exception during fetch results (databricks#1199)
## Description - This fix was made in the [PR databricks#1135](databricks#1135), but it did not consider the fetch result timeout. This PR fixes that. ## Testing - unit tests ## Additional Notes to the Reviewer - Most of the code changes are propogating errors as `SQLException`, but the main code change is a small one and is present in `src/main/java/com/databricks/jdbc/common/util/DatabricksThriftUtil.java` file OVERRIDE_FREEZE=true
1 parent a8e339e commit b9f4dc1

48 files changed

Lines changed: 320 additions & 257 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
- **Enhanced `enableMultipleCatalogSupport` behavior**: When this parameter is disabled (`enableMultipleCatalogSupport=0`), metadata operations (such as `getSchemas()`, `getTables()`, `getColumns()`, etc.) now return results only when the catalog parameter is either `null` or matches the current catalog. For any other catalog name, an empty result set is returned. This ensures metadata queries are restricted to the current catalog context. When enabled (`enableMultipleCatalogSupport=1`), metadata operations continue to work across all accessible catalogs.
1919

2020
### Fixed
21+
- Fixed timeout exception handling to throw `SQLTimeoutException` instead of `DatabricksHttpException` when queries timeout during result fetching phase. This completes the timeout exception fix to handle both query execution polling and result fetching phases.
2122
- Fixed `getTypeInfo()` and `getClientInfoProperties()` to return fresh ResultSet instances on each call instead of shared static instances. This resolves issues where calling these methods multiple times would fail due to exhausted cursor state (Issue #1178).
2223
- Fixed complex data type metadata support when retrieving 0 rows in Arrow format
2324
- Normalized TIMESTAMP_NTZ to TIMESTAMP in Thrift path for consistency with SEA behavior

src/main/java/com/databricks/client/jdbc/DataSource.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import com.databricks.jdbc.common.DatabricksJdbcConstants;
66
import com.databricks.jdbc.common.DatabricksJdbcUrlParams;
7-
import com.databricks.jdbc.exception.DatabricksSQLException;
87
import com.databricks.jdbc.log.JdbcLogger;
98
import com.databricks.jdbc.log.JdbcLoggerFactory;
109
import com.databricks.jdbc.pooling.DatabricksPooledConnection;
@@ -37,13 +36,13 @@ public DataSource(Driver driver) {
3736
}
3837

3938
@Override
40-
public Connection getConnection() throws DatabricksSQLException {
39+
public Connection getConnection() throws SQLException {
4140
LOGGER.debug("public Connection getConnection()");
4241
return getConnection(this.getUsername(), this.getPassword());
4342
}
4443

4544
@Override
46-
public Connection getConnection(String username, String password) throws DatabricksSQLException {
45+
public Connection getConnection(String username, String password) throws SQLException {
4746
LOGGER.debug("public Connection getConnection(String, String)");
4847
if (username != null) {
4948
setUsername(username);
@@ -55,14 +54,13 @@ public Connection getConnection(String username, String password) throws Databri
5554
}
5655

5756
@Override
58-
public PooledConnection getPooledConnection() throws DatabricksSQLException {
57+
public PooledConnection getPooledConnection() throws SQLException {
5958
LOGGER.debug("public PooledConnection getPooledConnection()");
6059
return new DatabricksPooledConnection(getConnection());
6160
}
6261

6362
@Override
64-
public PooledConnection getPooledConnection(String user, String password)
65-
throws DatabricksSQLException {
63+
public PooledConnection getPooledConnection(String user, String password) throws SQLException {
6664
LOGGER.debug("public PooledConnection getPooledConnection(String, String)");
6765
return new DatabricksPooledConnection(getConnection(user, password));
6866
}

src/main/java/com/databricks/client/jdbc/Driver.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public boolean acceptsURL(String url) {
4747
}
4848

4949
@Override
50-
public Connection connect(String url, Properties info) throws DatabricksSQLException {
50+
public Connection connect(String url, Properties info) throws SQLException {
5151
if (!acceptsURL(url)) {
5252
// Return null connection if URL is not accepted - as per JDBC standard.
5353
return null;

src/main/java/com/databricks/jdbc/api/impl/DatabricksConnection.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public DatabricksConnection(
6363
}
6464

6565
@Override
66-
public void open() throws DatabricksSQLException {
66+
public void open() throws SQLException {
6767
this.session.open();
6868
}
6969

@@ -401,7 +401,7 @@ public void rollback() throws SQLException {
401401
}
402402

403403
@Override
404-
public void close() throws DatabricksSQLException {
404+
public void close() throws SQLException {
405405
LOGGER.debug("public void close()");
406406
for (IDatabricksStatementInternal statement : statementSet) {
407407
statement.close(false);

src/main/java/com/databricks/jdbc/api/impl/DatabricksResultSet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public DatabricksResultSet(
8484
StatementType statementType,
8585
IDatabricksSession session,
8686
IDatabricksStatementInternal parentStatement)
87-
throws DatabricksSQLException {
87+
throws SQLException {
8888
this.executionStatus = new ExecutionStatus(statementStatus);
8989
this.statementId = statementId;
9090
if (resultData != null) {

src/main/java/com/databricks/jdbc/api/impl/DatabricksSession.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.databricks.jdbc.telemetry.latency.DatabricksMetricsTimedProcessor;
2929
import com.databricks.sdk.support.ToStringer;
3030
import com.google.common.annotations.VisibleForTesting;
31+
import java.sql.SQLException;
3132
import java.util.HashMap;
3233
import java.util.Map;
3334
import javax.annotation.Nullable;
@@ -131,7 +132,7 @@ public boolean isOpen() {
131132
}
132133

133134
@Override
134-
public void open() throws DatabricksSQLException {
135+
public void open() throws SQLException {
135136
LOGGER.debug("public void open()");
136137

137138
// Skip for tests, it would be already set
@@ -205,7 +206,7 @@ public void open() throws DatabricksSQLException {
205206
}
206207

207208
@Override
208-
public void close() throws DatabricksSQLException {
209+
public void close() throws SQLException {
209210
LOGGER.debug("public void close()");
210211
synchronized (this) {
211212
if (isSessionOpen) {
@@ -361,7 +362,7 @@ public void setEmptyMetadataClient() {
361362
public void forceClose() {
362363
try {
363364
this.close();
364-
} catch (DatabricksSQLException e) {
365+
} catch (SQLException e) {
365366
LOGGER.error("Error closing session resources, but marking the session as closed.");
366367
} finally {
367368
this.isSessionOpen = false;

src/main/java/com/databricks/jdbc/api/impl/ExecutionResultFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ static IExecutionResult getResultSet(
3232
StatementId statementId,
3333
IDatabricksSession session,
3434
IDatabricksStatementInternal statement)
35-
throws DatabricksSQLException {
35+
throws SQLException {
3636
IExecutionResult resultHandler = getResultHandler(data, manifest, statementId, session);
3737
if (manifest.getIsVolumeOperation() != null && manifest.getIsVolumeOperation()) {
3838
return new VolumeOperationResult(

src/main/java/com/databricks/jdbc/api/impl/IExecutionResult.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.databricks.jdbc.api.impl;
22

33
import com.databricks.jdbc.exception.DatabricksSQLException;
4+
import java.sql.SQLException;
45

56
/** Interface to provide methods over an underlying statement result */
67
public interface IExecutionResult {
@@ -26,7 +27,7 @@ public interface IExecutionResult {
2627
*
2728
* @return true if cursor is moved at next row
2829
*/
29-
boolean next() throws DatabricksSQLException;
30+
boolean next() throws SQLException;
3031

3132
/** Returns if there is next row in the result set */
3233
boolean hasNext();

src/main/java/com/databricks/jdbc/api/impl/LazyThriftResult.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.databricks.jdbc.log.JdbcLoggerFactory;
1111
import com.databricks.jdbc.model.client.thrift.generated.TFetchResultsResp;
1212
import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode;
13+
import java.sql.SQLException;
1314

1415
public class LazyThriftResult implements IExecutionResult {
1516
private static final JdbcLogger LOGGER = JdbcLoggerFactory.getLogger(LazyThriftResult.class);
@@ -99,10 +100,10 @@ public long getCurrentRow() {
99100
* Moves the cursor to the next row. Fetches additional data from server if needed.
100101
*
101102
* @return true if there is a next row, false if at the end
102-
* @throws DatabricksSQLException if an error occurs while fetching data
103+
* @throws SQLException if an error occurs while fetching data
103104
*/
104105
@Override
105-
public boolean next() throws DatabricksSQLException {
106+
public boolean next() throws SQLException {
106107
if (isClosed || hasReachedEnd) {
107108
return false;
108109
}
@@ -221,9 +222,9 @@ private void loadCurrentBatch() throws DatabricksSQLException {
221222
/**
222223
* Fetches the next batch of data from the server and creates columnar views.
223224
*
224-
* @throws DatabricksSQLException if the fetch operation fails
225+
* @throws SQLException if the fetch operation fails
225226
*/
226-
private void fetchNextBatch() throws DatabricksSQLException {
227+
private void fetchNextBatch() throws SQLException {
227228
try {
228229
LOGGER.debug("Fetching next batch, current total rows fetched: {}", totalRowsFetched);
229230
currentResponse = session.getDatabricksClient().getMoreResults(statement);

src/main/java/com/databricks/jdbc/api/impl/arrow/AbstractRemoteChunkProvider.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode;
2020
import com.databricks.jdbc.telemetry.TelemetryHelper;
2121
import com.databricks.sdk.service.sql.BaseChunkInfo;
22+
import java.sql.SQLException;
2223
import java.util.concurrent.ConcurrentHashMap;
2324
import java.util.concurrent.ConcurrentMap;
2425
import java.util.concurrent.ExecutionException;
@@ -94,7 +95,7 @@ protected AbstractRemoteChunkProvider(
9495
IDatabricksHttpClient httpClient,
9596
int maxParallelChunkDownloadsPerQuery,
9697
CompressionCodec compressionCodec)
97-
throws DatabricksSQLException {
98+
throws SQLException {
9899
this.chunkReadyTimeoutSeconds = session.getConnectionContext().getChunkReadyTimeoutSeconds();
99100
this.maxParallelChunkDownloadsPerQuery = maxParallelChunkDownloadsPerQuery;
100101
this.session = session;
@@ -264,7 +265,7 @@ private ConcurrentMap<Long, T> initializeChunksMap(
264265
TFetchResultsResp resultsResp,
265266
IDatabricksStatementInternal parentStatement,
266267
IDatabricksSession session)
267-
throws DatabricksSQLException {
268+
throws SQLException {
268269
ConcurrentMap<Long, T> chunkIndexMap = new ConcurrentHashMap<>();
269270
populateChunkIndexMap(resultsResp.getResults(), chunkIndexMap);
270271
while (resultsResp.hasMoreRows) {

0 commit comments

Comments
 (0)