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

Makes the binary reader set the encoding version when seeked to a span. #622

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

tgregg
Copy link
Contributor

@tgregg tgregg commented Oct 20, 2023

Description of changes:

The SpanProvider and SeekableReader facets allow users to retrieve Span objects that point to the value at which a reader is currently positioned, and later seek a reader to that position.

Usually, users seek back to spans previously created by the same reader. However, sometimes users will create a span using one reader and then try to seek a different reader to that span. This works as long as both readers point at the same data. Existing tests only tested seeking to spans created by the same reader.

Before this change, if the reader receiving the span to seek to had not yet read any data (i.e., had not yet encountered an IVM), it would be in a state where it considered all type ID bytes invalid. This change fixes that by determining the span's Ion encoding version from its associated symbol table and setting the reader's state accordingly.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...n/ion/impl/IonReaderContinuableTopLevelBinary.java 94.44% <100.00%> (ø)
src/com/amazon/ion/impl/IonCursorBinary.java 86.49% <60.00%> (-0.19%) ⬇️

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@tgregg tgregg merged commit ab84b08 into master Nov 3, 2023
15 of 33 checks passed
@tgregg tgregg deleted the span-seek-sets-ion-version branch November 3, 2023 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants