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

fix: query could hang transaction if ResultSet#next() is not called #643

Merged
merged 1 commit into from Nov 18, 2020

Conversation

olavloite
Copy link
Contributor

@olavloite olavloite commented Nov 17, 2020

If the first statement of a read/write transaction was a query or a read operation, and the application would not call ResultSet#next() on the return result, the transaction would hang indefinitely as the query would be marked as the one that should initiate the transaction (inline the BeginTransaction option). The query would however never be executed, as the actual query execution is deferred until the first call to ResultSet#next().

Fixes #641

If the first statement of a read/write transaction was a query or a read operation,
and the application would not call ResultSet#next() on the return result, the transaction
would hang indefinetely as the query would be marked as the one that should initiate the
transaction (inline the BeginTransaction option). The query would however never be
executed, as the actual query execution is deferred until the first call to ResultSet#next().

Fixes #641
@olavloite olavloite requested a review from as a code owner Nov 17, 2020
@product-auto-label product-auto-label bot added the api: spanner label Nov 17, 2020
@google-cla google-cla bot added the cla: yes label Nov 17, 2020
@codecov
Copy link

@codecov codecov bot commented Nov 17, 2020

Codecov Report

Merging #643 (39a3a85) into master (79fad94) will increase coverage by 0.01%.
The diff coverage is 94.73%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #643      +/-   ##
============================================
+ Coverage     84.04%   84.06%   +0.01%     
+ Complexity     2503     2498       -5     
============================================
  Files           141      141              
  Lines         13810    13812       +2     
  Branches       1318     1317       -1     
============================================
+ Hits          11607    11611       +4     
+ Misses         1657     1655       -2     
  Partials        546      546              
Impacted Files Coverage Δ Complexity Δ
...va/com/google/cloud/spanner/AbstractResultSet.java 83.48% <85.71%> (+0.09%) 28.00 <0.00> (ø)
.../com/google/cloud/spanner/AbstractReadContext.java 86.15% <100.00%> (-0.05%) 47.00 <0.00> (-6.00)
...om/google/cloud/spanner/TransactionRunnerImpl.java 84.27% <100.00%> (ø) 9.00 <0.00> (ø)
.../google/cloud/spanner/SpannerExceptionFactory.java 82.92% <0.00%> (+2.43%) 40.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79fad94...39a3a85. Read the comment docs.

@olavloite olavloite requested a review from thiagotnunes Nov 17, 2020
@thiagotnunes thiagotnunes merged commit 48f92e3 into master Nov 18, 2020
21 checks passed
@thiagotnunes thiagotnunes deleted the issue-641 branch Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants