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

Jump to bottom

spanner/spannertest: ping causes nil pointer dereference error #3639

Closed
olavloite opened this issue Feb 1, 2021 · 0 comments · Fixed by #3640
Closed

spanner/spannertest: ping causes nil pointer dereference error #3639

olavloite opened this issue Feb 1, 2021 · 0 comments · Fixed by #3640
Assignees
@olavloite
Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@olavloite
Copy link
Contributor

The ping method in the session pool is executed without a transaction selector. This will cause a nil pointer dereference error in the ExecuteSql method of inmem.go.

https://stackoverflow.com/questions/65987211/golang-spanner-test-library-crashes-after-a-while
@olavloite olavloite added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 1, 2021
@olavloite olavloite self-assigned this Feb 1, 2021
@product-auto-label product-auto-label bot added the api: spanner Issues related to the Spanner API. label Feb 1, 2021
@olavloite olavloite changed the title spanner/spansql: ping causes nil pointer dereference error spanner/spannertest: ping causes nil pointer dereference error Feb 1, 2021
olavloite added a commit that referenced this issue Feb 1, 2021
@olavloite
fix(spanner/spannertest): support queries in ExecuteSql
c284847 
Normal queries from the Spanner client use the ExecuteStreamingSql method,
while DML statements use ExecuteSql. This distinction was also built into
spannertest where ExecuteSql would only support DML statements and required
a transaction to be specified. The session pool however uses ExecuteSql to
execute a simple `SELECT 1` query without specifying any transaction. This
would cause a nil pointer dereference.

This PR introduces support for queries in the ExecuteSql method. The current
logic assumes that the statement is a query if the transaction is a single-
use read-only transaction.

Fixes #3639
@olavloite olavloite mentioned this issue Feb 1, 2021
Merged
gcf-merge-on-green bot pushed a commit that referenced this issue Feb 4, 2021
@olavloite
fix(spanner/spannertest): support queries in ExecuteSql (#3640)
8eede84 
Normal queries from the Spanner client use the ExecuteStreamingSql method,
while DML statements use ExecuteSql. This distinction was also built into
spannertest where ExecuteSql would only support DML statements and required
a transaction to be specified. The session pool however uses ExecuteSql to
execute a simple `SELECT 1` query without specifying any transaction. This
would cause a nil pointer dereference.

This PR introduces support for queries in the ExecuteSql method. The current
logic assumes that the statement is a query if the transaction is a single-
use read-only transaction.

Fixes #3639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@olavloite olavloite

Labels
api: spanner Issues related to the Spanner API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(spanner/spannertest): support queries in ExecuteSql googleapis/google-cloud-go
1 participant
@olavloite