Make RawBatchValuesIteratorAdapter length equal to its internal BatchValuesIter length #1142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The length of this iterator should be equal to the length of its internal `BatchValuesIter`. In other words, the amount of serialization contexts should not affect this length. Otherwise, the caller is not able to detect a situation when value count is different than statement count.
|
|
wprzytula
previously approved these changes
Dec 7, 2024
Functional programming and iterators are the supreme, concise style!
wprzytula
approved these changes
Dec 7, 2024
wprzytula
pushed a commit
to wprzytula/scylla-rust-driver
that referenced
this pull request
Dec 11, 2024
Make RawBatchValuesIteratorAdapter length equal to its internal BatchValuesIter length (cherry picked from commit d7a02a4)
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When executing batch statement, the amount of statements should be equal to amount of batch values.
This is checked in
scylla-cql/src/frame/request/batch.rsin impl ofBatch, specifically it'sdo_serializemethod.This method iterates over statements, serializes the statement and serializes the values for the statement.
If the value iterator returns
None, then there were not enough values, so it returns an error.After serialization it verifies that the iterator is finished - if it is not, then there are too many values.
The problem is that the length of
RawBatchValuesIteratorAdapteris limited to the amountof statements, because all of it's methods first try to get next serialization context, and if it is not present, return
None.This limits the length of this iterator to the amount of statement, so the second check (the one after serializing all the statements) couldn't do it's job.
The fix is to make sure that the length of
RawBatchValuesIteratorAdapteris always equal to the length of its internalBatchValuesIterator.This is done by:
self.contexts.next()inis_empty_nextandskip_nextserialize_next.Fixes #1114
Pre-review checklist
I have provided docstrings for the public items that I want to introduce.I have adjusted the documentation in./docs/source/.Fixes:annotations to PR description.