Skip to content

Commit 7592f65

Browse files
gopalldbclaude
andauthored
Fix ambiguous errors when statements are cancelled or connection closed (#1291)
## Summary - When a running query is cancelled (via `Statement.cancel()` or `Connection.close()`), the driver returned ambiguous errors like `"error: [null]"` or generic `"Statement execution failed"` indistinguishable from real connection errors - **Thrift**: Server returns `CANCELED_STATE` with `OK_STATUS` and null `errorMessage`. The driver didn't detect `CANCELED_STATE` as a terminal state, causing downstream failures with null data - **SEA**: `CANCELED` state fell through to `handleFailedExecution` which threw `EXECUTE_STATEMENT_FAILED` — same error code as real failures - `Connection.close()` called `statement.close()` without cancelling in-flight queries first, causing running statements to fail with socket errors ## Changes 1. **Thrift path** (`DatabricksThriftAccessor`): Detect `CANCELED_STATE` in polling and throw `DatabricksSQLException` with message `"Statement [id] was cancelled"`, SQL state `HY008`, error code `EXECUTE_STATEMENT_CANCELLED` 2. **SEA path** (`DatabricksSdkClient`): Detect `CANCELED` state in `handleFailedExecution` and throw with same clear signal 3. **Connection close** (`DatabricksConnection`): Best-effort `cancel()` on in-flight statements before closing 4. **Null error message** (`ExecutionStatus`): Default to `"Statement was cancelled"` when state is ABORTED but error message is null 5. **SQL state constant**: Add `OPERATION_CANCELLED_SQLSTATE = "HY008"` (standard SQL state for operation cancelled) ## Customer impact Customers can now programmatically distinguish cancellations from actual errors by checking: - SQL state: `HY008` = cancellation - Error code: `EXECUTE_STATEMENT_CANCELLED` - Message: `"Statement [id] was cancelled"` ## Test plan - [x] Full jdbc-core test suite: 3162 tests pass, 0 failures - [ ] Manual verification: cancel a long-running query and verify the error message/SQL state This pull request was AI-assisted by Isaac. --------- Signed-off-by: Gopal Lal <gopal.lal@databricks.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2a76760 commit 7592f65

8 files changed

Lines changed: 143 additions & 1 deletion

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
- Added support for using SQL SHOW commands for Thrift-mode metadata operations (`getTables`, `getColumns`, `getSchemas`, `getFunctions`, `getPrimaryKeys`, `getImportedKeys`, `getCrossReference`). Enable by setting `UseQueryForMetadata=1`. This aligns Thrift metadata behavior with Statement Execution API (SEA) mode.
1111

1212
### Fixed
13+
- Improved error messages for cancelled statements: operations cancelled via `Statement.cancel()` or closed connections now return SQL state `HY008` (operation cancelled) instead of generic error codes, making it easier for applications to detect and handle cancellations.
1314
- Fixed race condition between chunk download error handling and result set close that could cause invalid state transition warnings (`CHUNK_RELEASED -> DOWNLOAD_FAILED`) during Arrow Cloud Fetch operations in resource-constrained environments.
1415
- Fixed `EnableBatchedInserts` silently falling back to individual execution when table or schema names contain special characters (e.g., hyphens) inside backtick-quoted identifiers. Added a warn log when the fallback occurs.
1516
- Fixed `IntervalConverter` crash (`IllegalArgumentException: Invalid interval metadata`) when INTERVAL columns are returned via CloudFetch. Arrow metadata from CloudFetch uses underscored format (`INTERVAL_YEAR_MONTH`, `INTERVAL_DAY_TIME`) which the driver's regex did not accept.

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ class ExecutionStatus implements IExecutionStatus {
1818

1919
public ExecutionStatus(StatementStatus status) {
2020
this.state = getStateFromSdkState(status.getState());
21-
this.errorMessage = status.getError() != null ? status.getError().getMessage() : null;
21+
this.errorMessage =
22+
status.getError() != null && status.getError().getMessage() != null
23+
? status.getError().getMessage()
24+
: (this.state == ExecutionState.ABORTED ? "Statement was cancelled" : null);
2225
this.sqlState = status.getSqlState();
2326
this.sdkStatus = status;
2427
}

src/main/java/com/databricks/jdbc/common/DatabricksJdbcConstants.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ public final class DatabricksJdbcConstants {
109109
public static final String GCP_GOOGLE_ID_AUTH_TYPE = "google-id";
110110
public static final String DEFAULT_HTTP_EXCEPTION_SQLSTATE = "08000";
111111
public static final String QUERY_EXECUTION_TIMEOUT_SQLSTATE = "57KD0";
112+
113+
/** Standard SQL state for operation cancelled (SQLSTATE HY008). */
114+
public static final String OPERATION_CANCELLED_SQLSTATE = "HY008";
115+
112116
public static final int TEMPORARY_REDIRECT_STATUS_CODE = 307;
113117
public static final String REDACTED_TOKEN = "****";
114118
public static final String QUERY_TAGS = "query_tags";

src/main/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClient.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.databricks.jdbc.dbclient.impl.sqlexec;
22

33
import static com.databricks.jdbc.common.DatabricksJdbcConstants.JSON_HTTP_HEADERS;
4+
import static com.databricks.jdbc.common.DatabricksJdbcConstants.OPERATION_CANCELLED_SQLSTATE;
45
import static com.databricks.jdbc.common.DatabricksJdbcConstants.QUERY_EXECUTION_TIMEOUT_SQLSTATE;
56
import static com.databricks.jdbc.common.DatabricksJdbcConstants.TEMPORARY_REDIRECT_STATUS_CODE;
67
import static com.databricks.jdbc.common.EnvironmentVariables.DEFAULT_RESULT_ROW_LIMIT;
@@ -709,6 +710,17 @@ void handleFailedExecution(
709710
ExecuteStatementResponse response, String statementId, String statement) throws SQLException {
710711
StatementState statementState = response.getStatus().getState();
711712
ServiceError error = response.getStatus().getError();
713+
714+
// Distinguish cancellation from failure
715+
if (statementState == StatementState.CANCELED) {
716+
String cancelMessage = String.format("Statement [%s] was cancelled", statementId);
717+
LOGGER.info(cancelMessage);
718+
throw new DatabricksSQLException(
719+
cancelMessage,
720+
OPERATION_CANCELLED_SQLSTATE,
721+
DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED);
722+
}
723+
712724
String errorMessage =
713725
String.format(
714726
"Statement execution failed %s -> %s\n%s.", statementId, statement, statementState);

src/main/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftAccessor.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.databricks.jdbc.dbclient.impl.thrift;
22

3+
import static com.databricks.jdbc.common.DatabricksJdbcConstants.OPERATION_CANCELLED_SQLSTATE;
34
import static com.databricks.jdbc.common.DatabricksJdbcConstants.QUERY_EXECUTION_TIMEOUT_SQLSTATE;
45
import static com.databricks.jdbc.common.EnvironmentVariables.*;
56
import static com.databricks.jdbc.common.util.DatabricksThriftUtil.*;
@@ -422,6 +423,9 @@ DatabricksResultSet getStatementResult(
422423
try {
423424
response = getOperationStatus(request, statementId);
424425
TOperationState operationState = response.getOperationState();
426+
if (operationState == TOperationState.CANCELED_STATE) {
427+
throw cancelledStatementException(statementId.toSQLExecStatementId());
428+
}
425429
if (operationState == TOperationState.FINISHED_STATE) {
426430
verifySuccessStatus(
427431
response.getStatus(), "getStatementResult", statementId.toSQLExecStatementId());
@@ -828,6 +832,11 @@ private void checkOperationStatusForErrors(TGetOperationStatusResp statusResp, S
828832
errorMsg, statusResp.isSetSqlState() ? statusResp.getSqlState() : null);
829833
}
830834

835+
if (statusResp.isSetOperationState()
836+
&& statusResp.getOperationState() == TOperationState.CANCELED_STATE) {
837+
throw cancelledStatementException(statementId);
838+
}
839+
831840
if (statusResp.isSetOperationState() && isErrorOperationState(statusResp.getOperationState())) {
832841
String errorMsg =
833842
String.format(
@@ -863,6 +872,13 @@ private <T extends TBase<T, F>, F extends TFieldIdEnum> boolean hasResultDataInD
863872
return directResults.isSetResultSet() && directResults.isSetResultSetMetadata();
864873
}
865874

875+
private DatabricksSQLException cancelledStatementException(String statementId) {
876+
String msg = String.format("Statement [%s] was cancelled", statementId);
877+
LOGGER.info(msg);
878+
return new DatabricksSQLException(
879+
msg, OPERATION_CANCELLED_SQLSTATE, DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED);
880+
}
881+
866882
private boolean isErrorStatusCode(TStatus status) {
867883
if (status == null || !status.isSetStatusCode()) {
868884
LOGGER.error("Status code is not set, marking the response as failed");
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package com.databricks.jdbc.api.impl;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import com.databricks.jdbc.model.core.StatementStatus;
6+
import com.databricks.sdk.service.sql.StatementState;
7+
import org.junit.jupiter.api.Test;
8+
9+
class ExecutionStatusTest {
10+
11+
@Test
12+
void cancelledStateWithNullError_hasMeaningfulMessage() {
13+
StatementStatus cancelledStatus =
14+
new StatementStatus().setState(StatementState.CANCELED).setError(null);
15+
ExecutionStatus status = new ExecutionStatus(cancelledStatus);
16+
assertNotNull(status.getErrorMessage(), "Cancelled state should have non-null error message");
17+
assertTrue(status.getErrorMessage().contains("cancelled"));
18+
}
19+
20+
@Test
21+
void failedStateWithNullError_hasNullMessage() {
22+
StatementStatus failedStatus =
23+
new StatementStatus().setState(StatementState.FAILED).setError(null);
24+
ExecutionStatus status = new ExecutionStatus(failedStatus);
25+
assertNull(status.getErrorMessage(), "Failed state with null error should have null message");
26+
}
27+
28+
@Test
29+
void succeededStateWithNullError_hasNullMessage() {
30+
StatementStatus succeededStatus =
31+
new StatementStatus().setState(StatementState.SUCCEEDED).setError(null);
32+
ExecutionStatus status = new ExecutionStatus(succeededStatus);
33+
assertNull(status.getErrorMessage());
34+
}
35+
}

src/test/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClientTest.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,54 @@ public void testCancelStatement() throws DatabricksSQLException, IOException {
290290
eq(Void.class));
291291
}
292292

293+
@Test
294+
public void testHandleFailedExecution_CancelledState_ThrowsWithHY008() throws Exception {
295+
IDatabricksConnectionContext connectionContext =
296+
DatabricksConnectionContext.parse(JDBC_URL, new Properties());
297+
DatabricksSdkClient databricksSdkClient =
298+
new DatabricksSdkClient(connectionContext, statementExecutionService, apiClient);
299+
300+
StatementStatus cancelledStatus = new StatementStatus().setState(StatementState.CANCELED);
301+
ExecuteStatementResponse response =
302+
new ExecuteStatementResponse()
303+
.setStatementId(STATEMENT_ID.toSQLExecStatementId())
304+
.setStatus(cancelledStatus);
305+
306+
DatabricksSQLException exception =
307+
assertThrows(
308+
DatabricksSQLException.class,
309+
() ->
310+
databricksSdkClient.handleFailedExecution(
311+
response, STATEMENT_ID.toSQLExecStatementId(), STATEMENT));
312+
313+
assertEquals("HY008", exception.getSQLState());
314+
assertTrue(exception.getMessage().contains("was cancelled"));
315+
}
316+
317+
@Test
318+
public void testHandleFailedExecution_FailedState_ThrowsWithoutHY008() throws Exception {
319+
IDatabricksConnectionContext connectionContext =
320+
DatabricksConnectionContext.parse(JDBC_URL, new Properties());
321+
DatabricksSdkClient databricksSdkClient =
322+
new DatabricksSdkClient(connectionContext, statementExecutionService, apiClient);
323+
324+
StatementStatus failedStatus = new StatementStatus().setState(StatementState.FAILED);
325+
ExecuteStatementResponse response =
326+
new ExecuteStatementResponse()
327+
.setStatementId(STATEMENT_ID.toSQLExecStatementId())
328+
.setStatus(failedStatus);
329+
330+
DatabricksSQLException exception =
331+
assertThrows(
332+
DatabricksSQLException.class,
333+
() ->
334+
databricksSdkClient.handleFailedExecution(
335+
response, STATEMENT_ID.toSQLExecStatementId(), STATEMENT));
336+
337+
assertNotEquals("HY008", exception.getSQLState());
338+
assertTrue(exception.getMessage().contains("execution failed"));
339+
}
340+
293341
@Test
294342
public void testDisposition_arrowAndCloudFetchEnabled_usesExternalLinks() throws Exception {
295343
setupClientMocks(true, false);

src/test/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftAccessorTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,29 @@ void testGetStatementResult_pending() throws Exception {
449449
assertNull(resultSet.getMetaData());
450450
}
451451

452+
@Test
453+
void testGetStatementResult_cancelled_throwsWithHY008() throws Exception {
454+
when(connectionContext.getDirectResultMode()).thenReturn(false);
455+
accessor = spy(new DatabricksThriftAccessor(connectionContext));
456+
doReturn(thriftClient).when(accessor).getThriftClient();
457+
458+
// Server returns CANCELED_STATE with OK_STATUS and null errorMessage
459+
TGetOperationStatusResp cancelledResp =
460+
new TGetOperationStatusResp()
461+
.setStatus(new TStatus().setStatusCode(TStatusCode.SUCCESS_STATUS))
462+
.setOperationState(TOperationState.CANCELED_STATE);
463+
when(thriftClient.GetOperationStatus(any(TGetOperationStatusReq.class)))
464+
.thenReturn(cancelledResp);
465+
466+
DatabricksSQLException exception =
467+
assertThrows(
468+
DatabricksSQLException.class,
469+
() -> accessor.getStatementResult(tOperationHandle, null, session));
470+
471+
assertEquals("HY008", exception.getSQLState());
472+
assertTrue(exception.getMessage().contains("was cancelled"));
473+
}
474+
452475
@Test
453476
void testListPrimaryKeys() throws TException, SQLException, DatabricksValidationException {
454477
setup(false);

0 commit comments

Comments
 (0)