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

AVRO-3748 [java] fix SeekableInputStream.skip #2203

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

steven-aerts
Copy link
Contributor

The implementation of SeekableInputStream.skip had a longstantind issue that it did a seek to a wrong position.
This issue is rather difficult to hit in avro as it is only used in corner cases of for example the FastReader.

What is the purpose of the change

  • Fixes AVRO-3748 a longstanding issue in SeekableInputStream.skip

Verifying this change

This change added a test to verify the new behavior

Documentation

  • Does this pull request introduce a new feature? (yes / no)

@github-actions github-actions bot added the Java Pull Requests for Java binding label Apr 27, 2023
@steven-aerts steven-aerts force-pushed the AVRO-3748 branch 2 times, most recently from eb9db9d to de24e81 Compare May 3, 2023 05:35
Copy link
Contributor

@clesaec clesaec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

w.write("someContent");
}
try(DataFileReader.SeekableInputStream stream =
new DataFileReader.SeekableInputStream(new SeekableFileInput(f))

Check warning

Code scanning / CodeQL

Potential input resource leak

This SeekableFileInput is not always closed on method exit.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

try(DataFileReader.SeekableInputStream stream =
new DataFileReader.SeekableInputStream(new SeekableFileInput(f))
) {
for (int i = 0; i < stream.length(); i++) {

Check failure

Code scanning / CodeQL

Comparison of narrow type with wide type in loop condition

Comparison between [expression](1) of type int and [expression](2) of wider type long.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

The implementation of SeekableInputStream.skip had a longstantind issue
that it did a seek to a wrong position.
This issue is rather difficult to hit in avro as it is only used in
corner cases of for example the FastReader.
@clesaec clesaec merged commit 33631e1 into apache:master Aug 16, 2023
clesaec added a commit that referenced this pull request Aug 16, 2023
clesaec added a commit that referenced this pull request Aug 16, 2023
RanbirK pushed a commit to RanbirK/avro that referenced this pull request May 13, 2024
The implementation of SeekableInputStream.skip had a longstantind issue
that it did a seek to a wrong position.
This issue is rather difficult to hit in avro as it is only used in
corner cases of for example the FastReader.
RanbirK pushed a commit to RanbirK/avro that referenced this pull request May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java Pull Requests for Java binding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants