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.

Sign up for GitHub

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

Fix ListingTableUrl to decode percent #3750

Merged
merged 5 commits into from
Oct 11, 2022
Merged

Fix ListingTableUrl to decode percent #3750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0ee83dd
fix: ListingTabUrl prefix decoding
unvalley Oct 7, 2022
92ef8be
chore: remove waste change
unvalley Oct 7, 2022
65ba95e
fix: use from instead of parse
unvalley Oct 7, 2022
439f6dc
test: add test cases for prefix
unvalley Oct 7, 2022
7338da4
chore: cargo fmt
unvalley Oct 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions datafusion/core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ ordered-float = "3.0"
parking_lot = "0.12"
parquet = { version = "24.0.0", features = ["arrow", "async"] }
paste = "^1.0"
percent-encoding = "2.2.0"
Copy link
Contributor Author

@unvalley unvalley Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

datafusion/core uses url crate by servo.
percent-encoding is also published by the same repository.

pin-project-lite = "^0.2.7"
pyo3 = { version = "0.17.1", optional = true }
rand = "0.8"
Expand Down
14 changes: 13 additions & 1 deletion datafusion/core/src/datasource/listing/url.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use glob::Pattern;
use itertools::Itertools;
use object_store::path::Path;
use object_store::{ObjectMeta, ObjectStore};
use percent_encoding;
use url::Url;

/// A parsed URL identifying files for a listing table, see [`ListingTableUrl::parse`]
Expand Down Expand Up @@ -108,7 +109,9 @@ impl ListingTableUrl {

/// Creates a new [`ListingTableUrl`] from a url and optional glob expression
fn new(url: Url, glob: Option<Pattern>) -> Self {
let prefix = Path::parse(url.path()).expect("should be URL safe");
let decoded_path =
percent_encoding::percent_decode_str(url.path()).decode_utf8_lossy();
let prefix = Path::from(decoded_path.as_ref());
Self { url, prefix, glob }
}

Expand Down Expand Up @@ -246,6 +249,15 @@ mod tests {
let url = ListingTableUrl::parse("file:///foo").unwrap();
let child = Path::parse("/foob/bar").unwrap();
assert!(url.strip_prefix(&child).is_none());

let url = ListingTableUrl::parse("file:///foo/ bar").unwrap();
assert_eq!(url.prefix.as_ref(), "foo/ bar");

let url = ListingTableUrl::parse("file:///foo/bar?").unwrap();
assert_eq!(url.prefix.as_ref(), "foo/bar");

let url = ListingTableUrl::parse("file:///foo/😺").unwrap();
assert_eq!(url.prefix.as_ref(), "foo/%F0%9F%98%BA");
}

#[test]
Expand Down