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

Expose underlying *spanner.Client from SpannerConn #312

Closed
egonelbre opened this issue Nov 6, 2024 · 1 comment · Fixed by #313
Closed

Expose underlying *spanner.Client from SpannerConn #312

egonelbre opened this issue Nov 6, 2024 · 1 comment · Fixed by #313
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@egonelbre
Copy link
Contributor

egonelbre commented Nov 6, 2024

Currently there are several issues that have come up that could be worked around with using *spanner.Client directly.

For example:

By allowing to access *spanner.Client directly, it would make it easier to workaround issues that don't have yet parity in go-sql-spanner.

For workaround it's of course possible to use spanner.NewClient in addition to database/sql, but that would mean needing to manage multiple connection pools and also thread that access throughout the system.

It would be nice if you can use:

if err := conn.Raw(func(driverConn interface{}) error {
	spannerconn := driverConn.(spannerdriver.SpannerConn)
	client, err := spannerconn.UnderlyingClient()
	...
})
@egonelbre egonelbre added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Nov 6, 2024
egonelbre added a commit to egonelbre/go-sql-spanner that referenced this issue Nov 6, 2024
Sometimes the exposed features of go-sql-spanner are not sufficient to
get the necessary response. This adds an escape hatch to access the
underlying client.

Fixes googleapis#312
@egonelbre
Copy link
Contributor Author

I'm not sure whether it's useful to make it:

UnderlyingClient() (any, error)
// instead of
UnderlyingClient() (*spanner.Client, error)

It seems unlikely that it will start using some completely new client. However, v2/v3 might necessitate to use a new client type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant