Skip to content

Commit 8d39dc1

Browse files
fix: remove azure suffix range request (delta-io#1006)
resolves delta-io#968 (I hope) ## What changes are proposed in this pull request? TLDR: Fixes kernel (default-engine) error when trying to read on azure. `arrow-55` introduced [support for reading parquet metadata via suffix range requests](apache/arrow-rs#7334) but since azure [doesn't support suffix range requests](https://github.com/apache/arrow-rs-object-store/blob/9c43521d3c8e86fdfc4df788a5a63e45e23093b7/src/azure/client.rs#L912-L918) whenever anyone tried reading an azure table with default-engine based on arrow-55 it [blew up](delta-io#968). This PR mitigates by re-adding the original `head` request so that we can pass the `file_size` to the `ParquetObjectReader::with_file_size` which states that when this API is used, the reader will ensure only bounded range requests are used: ```rust /// Provide the byte size of this file. /// /// If provided, the file size will ensure that only bounded range requests are used. If file /// size is not provided, the reader will use suffix range requests to fetch the metadata. /// /// Providing this size up front is an important optimization to avoid extra calls when the /// underlying store does not support suffix range requests. /// /// The file size can be obtained using [`ObjectStore::list`] or [`ObjectStore::head`]. pub fn with_file_size(self, file_size: usize) -> Self { ... } ``` ## How was this change tested? tested manually.. painfully manually. spun up a public azure container thing and pointed our example code at it. was able to repro the following error without my changes and observe it going away after changes here applied. ``` Parquet( External( NotSupported { source: "Azure does not support suffix range requests", }, ), ) ``` obviously highlights our test gap. working on setting up some environments to test against S3/ADLS/GCS/etc.
1 parent bf012fc commit 8d39dc1

1 file changed

Lines changed: 22 additions & 1 deletion

File tree

kernel/src/engine/default/parquet.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,28 @@ impl FileOpener for ParquetOpener {
265265

266266
Ok(Box::pin(async move {
267267
#[cfg(feature = "arrow-55")]
268-
let mut reader = ParquetObjectReader::new(store, path);
268+
let mut reader = {
269+
use crate::object_store::ObjectStoreScheme;
270+
// HACK: unfortunately, `ParquetObjectReader` under the hood does a suffix range
271+
// request which isn't supported by Azure. For now we just detect if the URL is
272+
// pointing to azure and if so, do a HEAD request so we can pass in file size to the
273+
// reader which will cause the reader to avoid a suffix range request.
274+
// see also: https://github.com/delta-io/delta-kernel-rs/issues/968
275+
//
276+
// TODO(#1010): Note that we don't need this at all and can actually just _always_
277+
// do the `with_file_size` but need to (1) update our unit tests which often
278+
// hardcode size=0 and (2) update CDF execute which also hardcodes size=0.
279+
if let Ok((ObjectStoreScheme::MicrosoftAzure, _)) =
280+
ObjectStoreScheme::parse(&file_meta.location)
281+
{
282+
// also note doing HEAD then actual GET isn't atomic, and leaves us vulnerable
283+
// to file changing between the two calls.
284+
let meta = store.head(&path).await?;
285+
ParquetObjectReader::new(store, path).with_file_size(meta.size)
286+
} else {
287+
ParquetObjectReader::new(store, path)
288+
}
289+
};
269290
#[cfg(all(feature = "arrow-54", not(feature = "arrow-55")))]
270291
let mut reader = {
271292
// TODO avoid IO by converting passed file meta to ObjectMeta (no longer an issue

0 commit comments

Comments
 (0)