Skip to content

Commit

Permalink
refactor: Remove transmute for object store path (#17395)
Browse files Browse the repository at this point in the history
  • Loading branch information
nameexhaustion authored Jul 3, 2024
1 parent d4cd078 commit d16d3ab
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 16 deletions.
16 changes: 4 additions & 12 deletions crates/polars-io/src/cloud/object_store_setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,9 @@ fn url_and_creds_to_key(url: &Url, options: Option<&CloudOptions>) -> String {
)
}

/// Simply construct an object_store `Path` struct from a string.
pub fn object_path_from_string(path: String) -> object_store::path::Path {
// We transmute because they don't expose a way to just create it from a string
// without encoding or decoding it. If one day we can't use this transmute hack
// anymore then we'll just have to `Path::from_url_path(percent_encode(path))`
{
const _: [(); std::mem::align_of::<String>()] =
[(); std::mem::align_of::<object_store::path::Path>()];
};

unsafe { std::mem::transmute::<String, object_store::path::Path>(path) }
/// Construct an object_store `Path` from a string without any encoding/decoding.
pub fn object_path_from_string(path: String) -> PolarsResult<object_store::path::Path> {
object_store::path::Path::parse(&path).map_err(to_compute_err)
}

/// Build an [`ObjectStore`] based on the URL and passed in url. Return the cloud location and an implementation of the object store.
Expand Down Expand Up @@ -147,7 +139,7 @@ mod test {
use super::object_path_from_string;

let path = "%25";
let out = object_path_from_string(path.to_string());
let out = object_path_from_string(path.to_string()).unwrap();

assert_eq!(out.as_ref(), path);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-io/src/file_cache/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub fn init_entries_from_uri_list<A: AsRef<[Arc<str>]>>(

let cloud_path = {
assert!(expansion.is_none(), "path should not contain wildcards");
object_path_from_string(prefix)
object_path_from_string(prefix)?
};

let object_store = object_store.clone();
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-io/src/ipc/ipc_reader_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl IpcReaderAsync {
// Any wildcards should already have been resolved here. Without this assertion they would
// be ignored.
debug_assert!(expansion.is_none(), "path should not contain wildcards");
object_path_from_string(prefix)
object_path_from_string(prefix)?
};

Ok(Self {
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-io/src/parquet/read/async_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl ParquetObjectStore {
// Any wildcards should already have been resolved here. Without this assertion they would
// be ignored.
debug_assert!(expansion.is_none(), "path should not contain wildcards");
let path = object_path_from_string(prefix);
let path = object_path_from_string(prefix)?;

Ok(ParquetObjectStore {
store: PolarsObjectStore::new(store),
Expand Down
2 changes: 1 addition & 1 deletion crates/polars-lazy/src/scan/file_list_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ fn expand_paths(
let (cloud_location, store) =
polars_io::cloud::build_object_store(path, cloud_options).await?;

let prefix = object_path_from_string(cloud_location.prefix.clone());
let prefix = object_path_from_string(cloud_location.prefix.clone())?;

let out = if !path.ends_with("/")
&& cloud_location.expansion.is_none()
Expand Down

0 comments on commit d16d3ab

Please sign in to comment.