fix(query): uri path should not be percent-encoded (#17109)

databendlabs · Dec 25, 2024 · b8ee29b · b8ee29b
1 parent 5921c66
commit b8ee29b
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 6 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/src/query/ast/Cargo.toml b/src/query/ast/Cargo.toml
@@ -23,6 +23,7 @@ logos = { workspace = true }
 nom = { workspace = true }
 nom-rule = { workspace = true }
 ordered-float = { workspace = true }
+percent-encoding = { workspace = true }
 pratt = { workspace = true }
 pretty = { workspace = true }
 pretty_assertions = { workspace = true }

diff --git a/src/query/ast/src/ast/statements/copy.rs b/src/query/ast/src/ast/statements/copy.rs
@@ -22,6 +22,7 @@ use std::str::FromStr;
 use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 use itertools::Itertools;
+use percent_encoding::percent_decode_str;
 use url::Url;
 
 use crate::ast::quote::QuotedString;
@@ -477,7 +478,9 @@ impl UriLocation {
         let path = if parsed.path().is_empty() {
             "/".to_string()
         } else {
-            parsed.path().to_string()
+            percent_decode_str(parsed.path())
+                .decode_utf8_lossy()
+                .to_string()
         };
 
         Ok(Self {

diff --git a/src/query/sql/Cargo.toml b/src/query/sql/Cargo.toml
@@ -64,7 +64,6 @@ num-derive = { workspace = true }
 num-traits = { workspace = true }
 opendal = { workspace = true }
 parking_lot = { workspace = true }
-percent-encoding = { workspace = true }
 prqlc = { workspace = true }
 rand = { workspace = true }
 recursive = { workspace = true }

diff --git a/src/query/sql/src/planner/binder/location.rs b/src/query/sql/src/planner/binder/location.rs
@@ -42,7 +42,6 @@ use databend_common_storage::STDIN_FD;
 use opendal::raw::normalize_path;
 use opendal::raw::normalize_root;
 use opendal::Scheme;
-use percent_encoding::percent_decode_str;
 
 /// secure_omission will fix omitted endpoint url schemes into 'https://'
 #[inline]
@@ -538,10 +537,9 @@ pub async fn parse_uri_location(
         Scheme::Cos => parse_cos_params(l, root)?,
         Scheme::Http => {
             // Make sure path has been percent decoded before parse pattern.
-            let path = percent_decode_str(&l.path).decode_utf8_lossy();
             let cfg = StorageHttpConfig {
                 endpoint_url: format!("{}://{}", l.protocol, l.name),
-                paths: globiter::Pattern::parse(&path)
+                paths: globiter::Pattern::parse(&l.path)
                     .map_err(|err| {
                         Error::new(
                             ErrorKind::InvalidInput,

diff --git a/tests/suites/1_stateful/00_stage/00_0005_copy_into_location.result b/tests/suites/1_stateful/00_stage/00_0005_copy_into_location.result
@@ -0,0 +1,4 @@
+>>>> create or replace connection c_00_0005 storage_type='s3' access_key_id = 'minioadmin'  endpoint_url = 'http://127.0.0.1:9900' secret_access_key = 'minioadmin'
+>>>> copy into 's3://testbucket/c_00_0005/ab de/f' connection=(connection_name='c_00_0005') from (select 1) detailed_output=true use_raw_path=true single=true overwrite=true
+c_00_0005/ab de/f	374	1
+<<<<
diff --git a/tests/suites/1_stateful/00_stage/00_0005_copy_into_location.sh b/tests/suites/1_stateful/00_stage/00_0005_copy_into_location.sh
@@ -0,0 +1,9 @@
+#!/usr/bin/env bash
+
+CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+. "$CURDIR"/../../../shell_env.sh
+
+
+stmt "create or replace connection c_00_0005 storage_type='s3' access_key_id = 'minioadmin'  endpoint_url = 'http://127.0.0.1:9900' secret_access_key = 'minioadmin'"
+
+query "copy into 's3://testbucket/c_00_0005/ab de/f' connection=(connection_name='c_00_0005') from (select 1) detailed_output=true use_raw_path=true single=true overwrite=true"