Revert "Use @subkanthi way"

ianton-ru · ianton-ru · commit 68a487fc8ad0 · 2026-03-18T12:54:03.000+01:00
This reverts commit ddd34ac.
diff --git a/src/IO/S3/URI.cpp b/src/IO/S3/URI.cpp
@@ -16,12 +16,12 @@ namespace DB
 
 struct URIConverter
 {
-    static void modifyURI(Poco::URI & uri, std::unordered_map<std::string, std::string> mapper)
+    static void modifyURI(Poco::URI & uri, std::unordered_map<std::string, std::string> mapper, bool enable_url_encoding)
     {
         Macros macros({{"bucket", uri.getHost()}});
         uri = macros.expand(mapper[uri.getScheme()]).empty()
             ? uri
-            : Poco::URI(macros.expand(mapper[uri.getScheme()]) + uri.getPathAndQuery());
+            : Poco::URI(macros.expand(mapper[uri.getScheme()]) + uri.getPathAndQuery(), enable_url_encoding);
     }
 };
 
@@ -33,7 +33,7 @@ namespace ErrorCodes
 namespace S3
 {
 
-URI::URI(const std::string & uri_, bool allow_archive_path_syntax)
+URI::URI(const std::string & uri_, bool allow_archive_path_syntax, bool enable_url_encoding)
 {
     /// Case when bucket name represented in domain name of S3 URL.
     /// E.g. (https://bucket-name.s3.region.amazonaws.com/key)
@@ -55,7 +55,7 @@ URI::URI(const std::string & uri_, bool allow_archive_path_syntax)
     else
         uri_str = uri_;
 
-    uri = Poco::URI(uri_str);
+    uri = Poco::URI(uri_str, enable_url_encoding);
 
     std::unordered_map<std::string, std::string> mapper;
     auto context = Context::getGlobalContextInstance();
@@ -77,7 +77,7 @@ URI::URI(const std::string & uri_, bool allow_archive_path_syntax)
         }
 
         if (!mapper.empty())
-            URIConverter::modifyURI(uri, mapper);
+            URIConverter::modifyURI(uri, mapper, enable_url_encoding);
     }
 
     storage_name = "S3";
diff --git a/src/IO/S3/URI.h b/src/IO/S3/URI.h
@@ -36,7 +36,7 @@ struct URI
     bool is_virtual_hosted_style;
 
     URI() = default;
-    explicit URI(const std::string & uri_, bool allow_archive_path_syntax = false);
+    explicit URI(const std::string & uri_, bool allow_archive_path_syntax = false, bool enable_url_encoding = true);
     void addRegionToURI(const std::string & region);
 
     static void validateBucket(const std::string & bucket, const Poco::URI & uri);
diff --git a/src/Storages/ObjectStorage/Utils.cpp b/src/Storages/ObjectStorage/Utils.cpp
@@ -342,9 +342,9 @@ std::pair<DB::ObjectStoragePtr, std::string> resolveObjectStorageForPath(
             normalized_path = "s3://" + target_decomposed.authority + "/" + target_decomposed.key;
         }
         // enable_url_encoding=false, path from metadata must have correct encoding already
-        S3::URI s3_uri(normalized_path);
+        S3::URI s3_uri(normalized_path, /*allow_archive_path_syntax*/ false, /*enable_url_encoding*/ false);
 
-        std::string key_to_use = target_decomposed.key;
+        std::string key_to_use = s3_uri.key;
 
         bool use_base_storage = false;
         if (base_storage->getType() == ObjectStorageType::S3)
@@ -366,7 +366,7 @@ std::pair<DB::ObjectStoragePtr, std::string> resolveObjectStorageForPath(
             {
                 normalized_table_location = "s3://" + table_location_decomposed.authority + "/" + table_location_decomposed.key;
             }
-            S3::URI base_s3_uri(normalized_table_location);
+            S3::URI base_s3_uri(normalized_table_location, /*allow_archive_path_syntax*/ false, /*enable_url_encoding*/ false);
 
             if (s3URIMatches(s3_uri, base_s3_uri.bucket, base_s3_uri.endpoint, target_scheme_normalized))
                 use_base_storage = true;

Original file line number	Diff line number	Diff line change
`@@ -16,12 +16,12 @@ namespace DB`
`16`	`16`
`17`	`17`	`struct URIConverter`
`18`	`18`	`{`
`19`		`- static void modifyURI(Poco::URI & uri, std::unordered_map<std::string, std::string> mapper)`
	`19`	`+ static void modifyURI(Poco::URI & uri, std::unordered_map<std::string, std::string> mapper, bool enable_url_encoding)`
`20`	`20`	`{`
`21`	`21`	`Macros macros({{"bucket", uri.getHost()}});`
`22`	`22`	`uri = macros.expand(mapper[uri.getScheme()]).empty()`
`23`	`23`	`? uri`
`24`		`- : Poco::URI(macros.expand(mapper[uri.getScheme()]) + uri.getPathAndQuery());`
	`24`	`+ : Poco::URI(macros.expand(mapper[uri.getScheme()]) + uri.getPathAndQuery(), enable_url_encoding);`
`25`	`25`	`}`
`26`	`26`	`};`
`27`	`27`
`@@ -33,7 +33,7 @@ namespace ErrorCodes`
`33`	`33`	`namespace S3`
`34`	`34`	`{`
`35`	`35`
`36`		`-URI::URI(const std::string & uri_, bool allow_archive_path_syntax)`
	`36`	`+URI::URI(const std::string & uri_, bool allow_archive_path_syntax, bool enable_url_encoding)`
`37`	`37`	`{`
`38`	`38`	`/// Case when bucket name represented in domain name of S3 URL.`
`39`	`39`	`/// E.g. (https://bucket-name.s3.region.amazonaws.com/key)`
`@@ -55,7 +55,7 @@ URI::URI(const std::string & uri_, bool allow_archive_path_syntax)`
`55`	`55`	`else`
`56`	`56`	`uri_str = uri_;`
`57`	`57`
`58`		`- uri = Poco::URI(uri_str);`
	`58`	`+ uri = Poco::URI(uri_str, enable_url_encoding);`
`59`	`59`
`60`	`60`	`std::unordered_map<std::string, std::string> mapper;`
`61`	`61`	`auto context = Context::getGlobalContextInstance();`
`@@ -77,7 +77,7 @@ URI::URI(const std::string & uri_, bool allow_archive_path_syntax)`
`77`	`77`	`}`
`78`	`78`
`79`	`79`	`if (!mapper.empty())`
`80`		`- URIConverter::modifyURI(uri, mapper);`
	`80`	`+ URIConverter::modifyURI(uri, mapper, enable_url_encoding);`
`81`	`81`	`}`
`82`	`82`
`83`	`83`	`storage_name = "S3";`
Original file line number	Diff line number	Diff line change
`@@ -342,9 +342,9 @@ std::pair<DB::ObjectStoragePtr, std::string> resolveObjectStorageForPath(`
`342`	`342`	`normalized_path = "s3://" + target_decomposed.authority + "/" + target_decomposed.key;`
`343`	`343`	`}`
`344`	`344`	`// enable_url_encoding=false, path from metadata must have correct encoding already`
`345`		`- S3::URI s3_uri(normalized_path);`
	`345`	`+ S3::URI s3_uri(normalized_path, /allow_archive_path_syntax/ false, /enable_url_encoding/ false);`
`346`	`346`
`347`		`- std::string key_to_use = target_decomposed.key;`
	`347`	`+ std::string key_to_use = s3_uri.key;`
`348`	`348`
`349`	`349`	`bool use_base_storage = false;`
`350`	`350`	`if (base_storage->getType() == ObjectStorageType::S3)`
`@@ -366,7 +366,7 @@ std::pair<DB::ObjectStoragePtr, std::string> resolveObjectStorageForPath(`
`366`	`366`	`{`
`367`	`367`	`normalized_table_location = "s3://" + table_location_decomposed.authority + "/" + table_location_decomposed.key;`
`368`	`368`	`}`
`369`		`- S3::URI base_s3_uri(normalized_table_location);`
	`369`	`+ S3::URI base_s3_uri(normalized_table_location, /allow_archive_path_syntax/ false, /enable_url_encoding/ false);`
`370`	`370`
`371`	`371`	`if (s3URIMatches(s3_uri, base_s3_uri.bucket, base_s3_uri.endpoint, target_scheme_normalized))`
`372`	`372`	`use_base_storage = true;`