You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kylebarron (via GitHub)" <gi...@apache.org> on 2024/11/18 16:45:06 UTC
[PR] Fix LocalFileSystem with range request that ends beyond end of file [arrow-rs]
kylebarron opened a new pull request, #6751:
URL: https://github.com/apache/arrow-rs/pull/6751
# Which issue does this PR close?
Closes #6749.
# Rationale for this change
Make `LocalFileSystem` act the same as other object store backends.
# What changes are included in this PR?
Allow `LocalFileSystem` to return a satisfiable byte range beyond the end of the file.
Add new test for satisfiable and non-satisfiable byte range.
# Are there any user-facing changes?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix LocalFileSystem with range request that ends beyond end of file [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #6751:
URL: https://github.com/apache/arrow-rs/pull/6751
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix LocalFileSystem with range request that ends beyond end of file [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6751:
URL: https://github.com/apache/arrow-rs/pull/6751#issuecomment-2640503611
❤️ --- thanks @kylebarron and @etseidl for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix LocalFileSystem with range request that ends beyond end of file [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6751:
URL: https://github.com/apache/arrow-rs/pull/6751#issuecomment-2494702657
🤔 the CI failures appear to be real
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix LocalFileSystem with range request that ends beyond end of file [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #6751:
URL: https://github.com/apache/arrow-rs/pull/6751#discussion_r1850747732
##########
object_store/src/local.rs:
##########
@@ -897,23 +897,26 @@ pub(crate) fn chunked_stream(
pub(crate) fn read_range(file: &mut File, path: &PathBuf, range: Range<usize>) -> Result<Bytes> {
let to_read = range.end - range.start;
- file.seek(SeekFrom::Start(range.start as u64))
+ let seek_idx = file
+ .seek(SeekFrom::Start(range.start as u64))
.context(SeekSnafu { path })?;
- let mut buf = Vec::with_capacity(to_read);
- let read = file
- .take(to_read as u64)
- .read_to_end(&mut buf)
- .context(UnableToReadBytesSnafu { path })?;
-
ensure!(
- read == to_read,
+ seek_idx == range.start as u64,
Review Comment:
It appears this will always be true, even if seeking beyond EOF.
##########
object_store/src/local.rs:
##########
@@ -897,23 +897,26 @@ pub(crate) fn chunked_stream(
pub(crate) fn read_range(file: &mut File, path: &PathBuf, range: Range<usize>) -> Result<Bytes> {
let to_read = range.end - range.start;
- file.seek(SeekFrom::Start(range.start as u64))
+ let seek_idx = file
+ .seek(SeekFrom::Start(range.start as u64))
.context(SeekSnafu { path })?;
- let mut buf = Vec::with_capacity(to_read);
- let read = file
- .take(to_read as u64)
- .read_to_end(&mut buf)
- .context(UnableToReadBytesSnafu { path })?;
-
ensure!(
- read == to_read,
+ seek_idx == range.start as u64,
OutOfRangeSnafu {
path,
- expected: to_read,
- actual: read
+ expected: range.start,
+ actual: seek_idx as usize
}
);
+
+ let mut buf = Vec::with_capacity(to_read);
+ file.take(to_read as u64)
+ .read_to_end(&mut buf)
+ .context(UnableToReadBytesSnafu { path })?;
Review Comment:
Likewise, this read will simply return 0 when reading beyond EOF.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix LocalFileSystem with range request that ends beyond end of file [arrow-rs]
Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on PR #6751:
URL: https://github.com/apache/arrow-rs/pull/6751#issuecomment-2637379266
I updated this to use `File::metadata` to access the length of the file and tests now pass locally for me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org