xiangfu0 commented on code in PR #18861:
URL: https://github.com/apache/pinot/pull/18861#discussion_r3498628842
##########
pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java:
##########
@@ -390,6 +391,31 @@ public InputStream open(URI uri)
}
}
+ @Override
+ public InputStream openForRead(URI uri, long offset, long length)
+ throws IOException {
+ if (offset < 0 || length < 0) {
+ throw new IllegalArgumentException(
+ "offset and length must be non-negative, got offset=" + offset + ",
length=" + length);
+ }
+ try {
+ Blob blob = getBlob(new GcsUri(uri));
+ // ReadChannel.limit is the absolute, exclusive end position; seek
positions the start. This issues a
+ // ranged GET so only [offset, offset + length) is transferred
(truncated at end-of-file).
+ ReadChannel reader = blob.reader();
Review Comment:
`getBlob()` can return `null` for a missing object in this class, but unlike
`open()` this path never checks that before calling `blob.reader()`. A ranged
read against a missing GCS object will therefore fail with
`NullPointerException` instead of the new SPI's
`IOException`/`FileNotFoundException` contract. Please mirror the null check
from `open()` here and add a missing-object regression test.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
