Skip to content
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: add methods for unwrapping Spanner client #1914

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

olavloite
Copy link
Collaborator

Adds methods for unwrapping the underlying DatabaseClient and Spanner instance that is used by a JDBC connection.

@olavloite olavloite requested a review from a team as a code owner February 14, 2025 15:06
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: spanner Issues related to the googleapis/java-spanner-jdbc API. labels Feb 14, 2025
@olavloite olavloite requested a review from rayudu3745 February 14, 2025 15:06
Adds methods for unwrapping the underlying DatabaseClient and Spanner
instance that is used by a JDBC connection.
@olavloite olavloite force-pushed the unwrap-spanner-client branch from 456fbd4 to f22db16 Compare February 14, 2025 15:13
@olavloite olavloite requested a review from a team as a code owner February 14, 2025 15:13
* Connection}, e.g. starting a read/write transaction on the {@link DatabaseClient} will not
* start a transaction on this connection.
*/
default DatabaseClient getDatabaseClient() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can customer do something like below?

CloudSpannerJdbcConnection spannerJdbcConnection =
          connection.unwrap(CloudSpannerJdbcConnection.class);
      List<String> ddl =
          spannerJdbcConnection
              .getSpanner()
              .getDatabaseClient()

Is there any difference between this and the exposed method? I mean do we need to expose a seperate method for getDatabaseClient?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, there is no difference, as the only key that is used to determine which DatabaseClient to get from a Spanner instance is the DatabaseId. But this might change in the future, for example when #1856 is resolved. With this, the JDBC driver might use additional properties to determine which DatabaseClient is used for a connection.

Comment on lines +74 to +81
public DatabaseId getDatabaseId() {
return this.options.getDatabaseId();
}

@Override
public DatabaseClient getDatabaseClient() {
return getSpannerConnection().getDatabaseClient();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interested to understand if there is a customer usecase where they would like to use databaseclient directly instead of using the connection.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is more of an 'escape hatch'. We try to support most features directly in the JDBC driver, but some Spanner features don't have a good equivalent in JDBC. In those cases, it can be easier to just use the database client directly. One such example could for example be BatchWrite. Although we can also create a method in the JDBC Connection that just delegates the call directly to the underlying database client, giving the option of just unwrapping the client ensures that all features are usable, even if there is no such delegate method.

@olavloite olavloite merged commit ee6082f into main Feb 15, 2025
22 checks passed
@olavloite olavloite deleted the unwrap-spanner-client branch February 15, 2025 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner-jdbc API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants