Re: [PR] Percent Decode URL Paths (#8009) [arrow-datafusion]

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #8012:
URL: https://github.com/apache/arrow-datafusion/pull/8012#discussion_r1378212990


##########
datafusion/core/src/datasource/listing/url.rs:
##########
@@ -317,8 +315,27 @@ mod tests {
         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");
+        let err = ListingTableUrl::parse("file:///foo/😺").unwrap_err();

Review Comment:
   It seems very reasonable to treat urls like `file://foo`, as urls (rather than paths with a `file://` prefix)



##########
datafusion/core/src/datasource/listing/url.rs:
##########
@@ -138,15 +137,13 @@ impl ListingTableUrl {
         .map_err(|_| DataFusionError::Internal(format!("Can not open path: {s}")))?;
         // TODO: Currently we do not have an IO-related error variant that accepts ()
         //       or a string. Once we have such a variant, change the error type above.
-        Ok(Self::new(url, glob))
+        Self::try_new(url, glob)
     }
 
     /// Creates a new [`ListingTableUrl`] from a url and optional glob expression
-    fn new(url: Url, glob: Option<Pattern>) -> Self {
-        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 }
+    fn try_new(url: Url, glob: Option<Pattern>) -> Result<Self> {
+        let prefix = Path::from_url_path(url.path())?;

Review Comment:
   Would it be possible to update the documentation on `fn parse` to explain the percent escaping behavior (you could simply copy the very nice description from this PR perhaps)
   



-- 
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