Merge branch 'main' into instantiatesecretsparallel

Mytherin · Mytherin · commit 59daa3f14e41 · 2026-01-29T11:49:19.000+01:00
diff --git a/.github/workflows/MainDistributionPipeline.yml b/.github/workflows/MainDistributionPipeline.yml
@@ -17,7 +17,7 @@ jobs:
     uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@main
     with:
       extension_name: httpfs
-      duckdb_version: 9c2a64a
+      duckdb_version: cc7c620e58625f6b5f53f724b6a9645f4ee0be6f
       ci_tools_version: main
 
   duckdb-stable-deploy:
@@ -27,7 +27,7 @@ jobs:
     secrets: inherit
     with:
       extension_name: httpfs
-      duckdb_version: 9c2a64a
+      duckdb_version: cc7c620e58625f6b5f53f724b6a9645f4ee0be6f
       ci_tools_version: main
       deploy_latest: ${{ startsWith(github.ref, 'refs/heads/v') }}
       deploy_versioned: ${{ startsWith(github.ref, 'refs/heads/v') || github.ref == 'refs/heads/main' }}
@@ -37,7 +37,7 @@ jobs:
     uses: duckdb/extension-ci-tools/.github/workflows/_extension_code_quality.yml@main
     with:
       extension_name: httpfs
-      duckdb_version: 1f9ecad746
+      duckdb_version: cc7c620e58625f6b5f53f724b6a9645f4ee0be6f
       ci_tools_version: main
       extra_toolchains: 'python3'
       format_checks: 'format'
diff --git a/duckdb b/duckdb
@@ -1 +1 @@
-Subproject commit 93582ac638a28b840bc63fb23b10a3dcfc208bad
+Subproject commit cc7c620e58625f6b5f53f724b6a9645f4ee0be6f
diff --git a/extension-ci-tools b/extension-ci-tools
@@ -1 +1 @@
-Subproject commit 795096d04b009c0d087468439ebb526a5460dfac
+Subproject commit cc340e3e15220bd3ddda3d0044b9b9cb4552424b
diff --git a/src/httpfs_curl_client.cpp b/src/httpfs_curl_client.cpp
@@ -39,8 +39,6 @@ static std::string SelectCURLCertPath() {
 	return std::string();
 }
 
-static std::string cert_path = SelectCURLCertPath();
-
 static size_t RequestWriteCallback(void *contents, size_t size, size_t nmemb, void *userp) {
 	size_t totalSize = size * nmemb;
 	std::string *str = static_cast<std::string *>(userp);
@@ -97,6 +95,7 @@ CURLHandle::CURLHandle(const string &token, const string &cert_path) {
 	if (!cert_path.empty()) {
 		curl_easy_setopt(curl, CURLOPT_CAINFO, cert_path.c_str());
 	}
+	curl_easy_setopt(curl, CURLOPT_SSL_OPTIONS, CURLSSLOPT_AUTO_CLIENT_CERT | CURLSSLOPT_NATIVE_CA);
 }
 
 CURLHandle::~CURLHandle() {
@@ -117,6 +116,8 @@ class HTTPFSCurlClient : public HTTPClient {
 	HTTPFSCurlClient(HTTPFSParams &http_params, const string &proto_host_port) {
 		base_url = curl_url();
 		curl_url_set(base_url, CURLUPART_URL, proto_host_port.c_str(), 0);
+		stored_bearer_token = "";
+		stored_cert_file_path = "";
 		Initialize(http_params);
 	}
 	void Initialize(HTTPParams &http_p) override {
@@ -125,12 +126,25 @@ class HTTPFSCurlClient : public HTTPClient {
 		if (!http_params.bearer_token.empty()) {
 			bearer_token = http_params.bearer_token.c_str();
 		}
+
 		state = http_params.state;
 
-		// call curl_global_init if not already done by another HTTPFS Client
-		InitCurlGlobal();
+		std::string cert_file_path;
+		if (!http_params.ca_cert_file.empty()) {
+			cert_file_path = http_params.ca_cert_file;
+		}
+
+		if (!curl || (stored_bearer_token != bearer_token) || (stored_cert_file_path != cert_file_path)) {
+			// call curl_global_init if not already done by another HTTPFS Client
+			InitCurlGlobal();
+
+			stored_cert_file_path = cert_file_path;
 
-		curl = make_uniq<CURLHandle>(bearer_token, SelectCURLCertPath());
+			if (cert_file_path.empty()) {
+				cert_file_path = SelectCURLCertPath();
+			}
+			curl = make_uniq<CURLHandle>(bearer_token, cert_file_path);
+		}
 		request_info = make_uniq<RequestInfo>();
 
 		// set curl options
@@ -140,6 +154,8 @@ class HTTPFSCurlClient : public HTTPClient {
 		// Curl re-uses connections by default
 		if (!http_params.keep_alive) {
 			curl_easy_setopt(*curl, CURLOPT_FORBID_REUSE, 1L);
+		} else {
+			curl_easy_setopt(*curl, CURLOPT_FORBID_REUSE, 0L);
 		}
 
 		if (http_params.enable_curl_server_cert_verification) {
@@ -167,6 +183,11 @@ class HTTPFSCurlClient : public HTTPClient {
 		curl_easy_setopt(*curl, CURLOPT_WRITEFUNCTION, RequestWriteCallback);
 		curl_easy_setopt(*curl, CURLOPT_WRITEDATA, &request_info->body);
 
+		// Reset PROXY-related settings, so they are set only on proxy actually being there
+		curl_easy_setopt(*curl, CURLOPT_PROXY, NULL);
+		curl_easy_setopt(*curl, CURLOPT_PROXYUSERNAME, NULL);
+		curl_easy_setopt(*curl, CURLOPT_PROXYPASSWORD, NULL);
+
 		if (!http_params.http_proxy.empty()) {
 			curl_easy_setopt(*curl, CURLOPT_PROXY,
 			                 StringUtil::Format("%s:%d", http_params.http_proxy, http_params.http_proxy_port).c_str());
@@ -452,6 +473,8 @@ class HTTPFSCurlClient : public HTTPClient {
 	optional_ptr<HTTPState> state;
 	unique_ptr<RequestInfo> request_info;
 	CURLU *base_url = nullptr;
+	string stored_bearer_token;
+	string stored_cert_file_path;
 
 	static std::mutex &GetRefLock() {
 		static std::mutex mtx;
diff --git a/src/include/s3fs.hpp b/src/include/s3fs.hpp
@@ -223,6 +223,7 @@ class S3FileSystem : public HTTPFileSystem {
 		return false;
 	}
 	void RemoveFile(const string &filename, optional_ptr<FileOpener> opener = nullptr) override;
+	void RemoveFiles(const vector<string> &filenames, optional_ptr<FileOpener> opener = nullptr) override;
 	void RemoveDirectory(const string &directory, optional_ptr<FileOpener> opener = nullptr) override;
 	void FileSync(FileHandle &handle) override;
 	void Write(FileHandle &handle, void *buffer, int64_t nr_bytes, idx_t location) override;
diff --git a/src/s3fs.cpp b/src/s3fs.cpp
@@ -12,6 +12,8 @@
 #include "http_state.hpp"
 
 #include "duckdb/common/string_util.hpp"
+#include "duckdb/common/crypto/md5.hpp"
+#include "duckdb/common/types/blob.hpp"
 #include "duckdb/function/scalar/string_common.hpp"
 #include "duckdb/main/secret/secret_manager.hpp"
 #include "duckdb/storage/buffer_manager.hpp"
@@ -1028,21 +1030,114 @@ void S3FileSystem::RemoveFile(const string &path, optional_ptr<FileOpener> opene
 	}
 }
 
+// Forward declaration for FindTagContents (defined later in file)
+optional_idx FindTagContents(const string &response, const string &tag, idx_t cur_pos, string &result);
+
+void S3FileSystem::RemoveFiles(const vector<string> &paths, optional_ptr<FileOpener> opener) {
+	if (paths.empty()) {
+		return;
+	}
+
+	struct BucketUrlInfo {
+		string prefix;
+		string http_proto;
+		string host;
+		string path;
+		S3AuthParams auth_params;
+	};
+
+	unordered_map<string, vector<string>> keys_by_bucket;
+	unordered_map<string, BucketUrlInfo> url_info_by_bucket;
+
+	for (auto &path : paths) {
+		FileOpenerInfo info = {path};
+		S3AuthParams auth_params = S3AuthParams::ReadFrom(opener, info);
+		auto parsed_url = S3UrlParse(path, auth_params);
+		ReadQueryParams(parsed_url.query_param, auth_params);
+
+		const string &bucket = parsed_url.bucket;
+		if (keys_by_bucket.find(bucket) == keys_by_bucket.end()) {
+			string bucket_path = parsed_url.path.substr(0, parsed_url.path.length() - parsed_url.key.length() - 1);
+			if (bucket_path.empty()) {
+				bucket_path = "/";
+			}
+			url_info_by_bucket[bucket] = {parsed_url.prefix, parsed_url.http_proto, parsed_url.host, bucket_path,
+			                              auth_params};
+		}
+
+		keys_by_bucket[bucket].push_back(parsed_url.key);
+	}
+
+	constexpr idx_t MAX_KEYS_PER_REQUEST = 1000;
+
+	for (auto &bucket_entry : keys_by_bucket) {
+		const string &bucket = bucket_entry.first;
+		const vector<string> &keys = bucket_entry.second;
+		const auto &url_info = url_info_by_bucket[bucket];
+
+		for (idx_t batch_start = 0; batch_start < keys.size(); batch_start += MAX_KEYS_PER_REQUEST) {
+			idx_t batch_end = MinValue<idx_t>(batch_start + MAX_KEYS_PER_REQUEST, keys.size());
+
+			std::stringstream xml_body;
+			xml_body << "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
+			xml_body << "<Delete xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">";
+
+			for (idx_t i = batch_start; i < batch_end; i++) {
+				xml_body << "<Object><Key>" << keys[i] << "</Key></Object>";
+			}
+
+			xml_body << "<Quiet>true</Quiet>";
+			xml_body << "</Delete>";
+
+			string body = xml_body.str();
+
+			MD5Context md5_context;
+			md5_context.Add(body);
+			data_t md5_hash[MD5Context::MD5_HASH_LENGTH_BINARY];
+			md5_context.Finish(md5_hash);
+
+			string_t md5_blob(const_char_ptr_cast(md5_hash), MD5Context::MD5_HASH_LENGTH_BINARY);
+			string content_md5 = Blob::ToBase64(md5_blob);
+
+			const string http_query_param_for_sig = "delete=";
+			const string http_query_param_for_url = "delete";
+			auto payload_hash = GetPayloadHash(const_cast<char *>(body.data()), body.length());
+
+			auto headers = CreateS3Header(url_info.path, http_query_param_for_sig, url_info.host, "s3", "POST",
+			                              url_info.auth_params, "", "", payload_hash, "");
+			headers["Content-MD5"] = content_md5;
+			headers["Content-Type"] = "application/xml";
+
+			string http_url = url_info.http_proto + url_info.host + S3FileSystem::UrlEncode(url_info.path) + "?" +
+			                  http_query_param_for_url;
+			string bucket_url = url_info.prefix + bucket + "/";
+			auto handle = OpenFile(bucket_url, FileFlags::FILE_FLAGS_READ, opener);
+
+			string result;
+			auto res = HTTPFileSystem::PostRequest(*handle, http_url, headers, result, const_cast<char *>(body.data()),
+			                                       body.length());
+
+			if (res->status != HTTPStatusCode::OK_200) {
+				throw IOException("Failed to remove files: HTTP %d (%s)\n%s", static_cast<int>(res->status),
+				                  res->GetError(), result);
+			}
+
+			idx_t cur_pos = 0;
+			string error_content;
+			auto error_pos = FindTagContents(result, "Error", cur_pos, error_content);
+			if (error_pos.IsValid()) {
+				throw IOException("Failed to remove files: %s", error_content);
+			}
+		}
+	}
+}
+
 void S3FileSystem::RemoveDirectory(const string &path, optional_ptr<FileOpener> opener) {
+	vector<string> files_to_remove;
 	ListFiles(
-	    path,
-	    [&](const string &file, bool is_dir) {
-		    try {
-			    this->RemoveFile(file, opener);
-		    } catch (IOException &e) {
-			    string errmsg(e.what());
-			    if (errmsg.find("No such file or directory") != std::string::npos) {
-				    return;
-			    }
-			    throw;
-		    }
-	    },
-	    opener.get());
+	    path, [&](const string &file, bool is_dir) { files_to_remove.push_back(file); }, opener.get());
+
+	RemoveFiles(files_to_remove, opener);
 }
 
 void S3FileSystem::FileSync(FileHandle &handle) {
@@ -1137,7 +1232,7 @@ struct S3GlobResult : public LazyMultiFileList {
 };
 
 S3GlobResult::S3GlobResult(S3FileSystem &fs, const string &glob_pattern_p, optional_ptr<FileOpener> opener)
-    : glob_pattern(glob_pattern_p), opener(opener) {
+    : LazyMultiFileList(FileOpener::TryGetClientContext(opener)), glob_pattern(glob_pattern_p), opener(opener) {
 	if (!opener) {
 		throw InternalException("Cannot S3 Glob without FileOpener");
 	}
diff --git a/test/sql/copy/s3/s3_remove_files.test b/test/sql/copy/s3/s3_remove_files.test
@@ -0,0 +1,76 @@
+# name: test/sql/copy/s3/s3_remove_files.test
+# description: Test RemoveFiles functionality via COPY OVERWRITE on S3
+# group: [s3]
+
+require parquet
+
+require httpfs
+
+require-env S3_TEST_SERVER_AVAILABLE 1
+
+require-env AWS_DEFAULT_REGION
+
+require-env AWS_ACCESS_KEY_ID
+
+require-env AWS_SECRET_ACCESS_KEY
+
+require-env DUCKDB_S3_ENDPOINT
+
+require-env DUCKDB_S3_USE_SSL
+
+# override the default behaviour of skipping HTTP errors and connection failures: this test fails on connection issues
+set ignore_error_messages
+
+foreach url_style path vhost
+
+statement ok
+SET s3_url_style='${url_style}'
+
+# Step 1: Clean up any leftover files from previous test runs (OVERWRITE clears the directory)
+# and create initial partitioned file (part_col=1)
+statement ok
+COPY (SELECT 1 as part_col, 'initial_1' as value) TO 's3://test-bucket/remove_files_test' (FORMAT PARQUET, PARTITION_BY (part_col), OVERWRITE);
+
+statement ok
+COPY (SELECT 2 as part_col, 'initial_2' as value) TO 's3://test-bucket/remove_files_test' (FORMAT PARQUET, PARTITION_BY (part_col), OVERWRITE_OR_IGNORE);
+
+# Step 2: Verify both partitions exist
+query I
+SELECT count(*) FROM glob('s3://test-bucket/remove_files_test/**/*.parquet')
+----
+2
+
+query IT
+SELECT part_col, value FROM 's3://test-bucket/remove_files_test/**/*.parquet' ORDER BY part_col
+----
+1	initial_1
+2	initial_2
+
+# Step 3: OVERWRITE with completely different data (part_col=99)
+# This should delete all existing files via RemoveFiles and create new ones
+statement ok
+COPY (SELECT 99 as part_col, 'new_data' as value) TO 's3://test-bucket/remove_files_test' (FORMAT PARQUET, PARTITION_BY (part_col), OVERWRITE);
+
+# Step 4: Verify OLD partitions are removed
+query I
+SELECT count(*) FROM glob('s3://test-bucket/remove_files_test/part_col=1/*.parquet')
+----
+0
+
+query I
+SELECT count(*) FROM glob('s3://test-bucket/remove_files_test/part_col=2/*.parquet')
+----
+0
+
+# Step 5: Verify only NEW partition exists
+query I
+SELECT count(*) FROM glob('s3://test-bucket/remove_files_test/**/*.parquet')
+----
+1
+
+query IT
+SELECT part_col, value FROM 's3://test-bucket/remove_files_test/**/*.parquet'
+----
+99	new_data
+
+endloop
diff --git a/test/sql/httpfs/ca_cert_file.test b/test/sql/httpfs/ca_cert_file.test
@@ -0,0 +1,16 @@
+# name: test/sql/httpfs/ca_cert_file.test
+# description: Test that invalid ca_cert_file causes SSL connection to fail
+# group: [httpfs]
+
+require httpfs
+
+statement ok
+SET enable_server_cert_verification = true;
+
+statement ok
+SET ca_cert_file = 'README.md';
+
+statement error
+FROM read_blob('https://duckdb.org/')
+----
+<REGEX>:.*IO Error.*SSL.*

Original file line number	Diff line number	Diff line change
`@@ -223,6 +223,7 @@ class S3FileSystem : public HTTPFileSystem {`
`223`	`223`	`return false;`
`224`	`224`	`}`
`225`	`225`	`void RemoveFile(const string &filename, optional_ptr<FileOpener> opener = nullptr) override;`
	`226`	`+ void RemoveFiles(const vector<string> &filenames, optional_ptr<FileOpener> opener = nullptr) override;`
`226`	`227`	`void RemoveDirectory(const string &directory, optional_ptr<FileOpener> opener = nullptr) override;`
`227`	`228`	`void FileSync(FileHandle &handle) override;`
`228`	`229`	`void Write(FileHandle &handle, void *buffer, int64_t nr_bytes, idx_t location) override;`