fix(bigquery-jdbc): fix htapi fallback due to permission logic - #14418
Conversation
…ipt query The listJobs API can return Job wrappers without fully-populated statistics, particularly when dealing with SET statements inside Scripts (such as variable assignments). When evaluating sub-jobs from a multi-statement BigQuery Script, the code assumed that `getStatistics()` and `.getScriptStatistics()` were not null and blindly dereferenced them (for example, to get evaluation kind), leading to NullPointerExceptions. This fix makes the statistics extraction gracefully handle `null` stats or non-QueryStatistics classes by providing null-safety fallback values. TAG=agy CONV=1f50bc2b-6723-4b07-b755-6f7b18bb91be
There was a problem hiding this comment.
Code Review
This pull request introduces support for a new service account environment variable (SA_EMAIL_NO_READAPI) across Cloud Build configurations, the Makefile, and integration tests, adding a test to verify high-throughput API fallback when the Read API is unavailable. It also simplifies exception handling in BigQueryStatement.java. Feedback suggests retaining the SQLException check in BigQueryStatement.java to avoid losing specific error properties (like SQLState) when wrapping exceptions, and utilizing try-with-resources in the new integration test to ensure proper resource management of connections and statements.
I am having trouble creating individual review comments. Click here to see my feedback.
java-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryStatement.java (925)
By completely removing the SQLException check, any pre-existing SQLException (including BigQueryJdbcException) will now be wrapped in a new BigQueryJdbcException. This causes a loss of the original SQLException's specific properties, such as the SQLState and vendor-specific error codes, which are crucial for JDBC clients to perform proper error handling.
Consider retaining the SQLException check so that existing SQLExceptions are thrown directly or preserved, while wrapping other exceptions (like RuntimeException) in BigQueryJdbcException.
if (ex instanceof SQLException) {
throw (ex instanceof BigQueryJdbcException)
? (BigQueryJdbcException) ex
: new BigQueryJdbcException(ex);
}
throw new BigQueryJdbcException(ex.getMessage(), ex);java-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/it/ITStatementTest.java (507-513)
Using try-with-resources ensures that the Connection and Statement are always closed properly, even if the test fails or throws an exception.
try (Connection connection = DriverManager.getConnection(connection_uri)) {
assertNotNull(connection);
assertFalse(connection.isClosed());
try (Statement statement = connection.createStatement()) {
validateStatement(statement, 1000);
}
}References
- Use try-with-resources to manage closeable resources such as Connection, PreparedStatement, and ResultSet in tests to ensure they are safely closed and prevent resource leaks.
…cTests TAG=agy CONV=f626ba62-4bee-4750-870f-c4d2558c7ad9
b459ab2 to
0d9b2fe
Compare
0d9b2fe to
eb5cfac
Compare
Fix HTAPI permission denied fallback logic. Due to added custom handling for Runtime exception,
processArrowResultwas no longer throwing SQLException for PermissionDenied error.Adding new SA that has no ReadAPI permissions & adding integration test.
b/561810298