Merge pull request #220 from Mytherin/regionhandling

S3: Improve handling around regions, and improve S3 error display

Author: Mytherin

src/httpfs.cpp

@@ -286,13 +286,7 @@ unique_ptr<HTTPResponse> HTTPFileSystem::GetRangeRequest(FileHandle &handle, str
 	    url, header_map, hfh.http_params,
 	    [&](const HTTPResponse &response) {
 		    if (static_cast<int>(response.status) >= 400) {
-			    string error =
-			        "HTTP GET error on '" + url + "' (HTTP " + to_string(static_cast<int>(response.status)) + ")";
-			    if (response.status == HTTPStatusCode::RangeNotSatisfiable_416) {
-				    error += " This could mean the file was changed. Try disabling the duckdb http metadata cache "
-				             "if enabled, and confirm the server supports range requests.";
-			    }
-			    throw HTTPException(response, error);
+			    throw GetHTTPError(handle, response, url);
 		    }
 		    if (static_cast<int>(response.status) < 300) { // done redirecting
 			    out_offset = 0;
@@ -863,6 +857,20 @@ void HTTPFileHandle::AllocateReadBuffer(optional_ptr<FileOpener> opener) {
 	read_buffer = allocator.Allocate(INITIAL_READ_BUFFER_LEN);
 }
 
+void HTTPFileHandle::InitializeFromCacheEntry(const HTTPMetadataCacheEntry &cache_entry) {
+	last_modified = cache_entry.last_modified;
+	length = cache_entry.length;
+	etag = cache_entry.etag;
+}
+
+HTTPMetadataCacheEntry HTTPFileHandle::GetCacheEntry() const {
+	HTTPMetadataCacheEntry result;
+	result.length = length;
+	result.last_modified = last_modified;
+	result.etag = etag;
+	return result;
+}
+
 void HTTPFileHandle::Initialize(optional_ptr<FileOpener> opener) {
 	auto &hfs = file_system.Cast<HTTPFileSystem>();
 	http_params.state = HTTPState::TryGetState(opener);
@@ -888,9 +896,7 @@ void HTTPFileHandle::Initialize(optional_ptr<FileOpener> opener) {
 			bool found = current_cache->Find(path, value);
 
 			if (found) {
-				last_modified = value.last_modified;
-				length = value.length;
-				etag = value.etag;
+				InitializeFromCacheEntry(value);
 
 				if (flags.OpenForReading() && !SkipBuffer()) {
 					AllocateReadBuffer(opener);
@@ -908,7 +914,7 @@ void HTTPFileHandle::Initialize(optional_ptr<FileOpener> opener) {
 			FullDownload(hfs, should_write_cache);
 		}
 		if (should_write_cache) {
-			current_cache->Insert(path, {length, last_modified, etag});
+			current_cache->Insert(path, GetCacheEntry());
 		}
 
 		if (!SkipBuffer()) {

src/include/http_metadata_cache.hpp

@@ -19,6 +19,7 @@ struct HTTPMetadataCacheEntry {
 	idx_t length;
 	timestamp_t last_modified;
 	string etag;
+	unordered_map<string, string> properties;
 };
 
 // Simple cache with a max age for an entry to be valid

src/include/httpfs.hpp

@@ -129,6 +129,9 @@ class HTTPFileHandle : public FileHandle {
 	//! TODO: make base function virtual?
 	void TryAddLogger(FileOpener &opener);
 
+	virtual void InitializeFromCacheEntry(const HTTPMetadataCacheEntry &cache_entry);
+	virtual HTTPMetadataCacheEntry GetCacheEntry() const;
+
 public:
 	//! Fully downloads a file
 	void FullDownload(HTTPFileSystem &hfs, bool &should_write_cache);

src/include/s3fs.hpp

@@ -62,6 +62,10 @@ struct S3AuthParams {
 
 	static S3AuthParams ReadFrom(optional_ptr<FileOpener> opener, FileOpenerInfo &info);
 	static S3AuthParams ReadFrom(S3KeyValueReader &secret_reader, const std::string &file_path);
+	void SetRegion(string region_p);
+
+private:
+	void InitializeEndpoint();
 };
 
 struct AWSEnvironmentCredentialsProvider {
@@ -140,17 +144,7 @@ class S3FileHandle : public HTTPFileHandle {
 
 public:
 	S3FileHandle(FileSystem &fs, const OpenFileInfo &file, FileOpenFlags flags, unique_ptr<HTTPParams> http_params_p,
-	             const S3AuthParams &auth_params_p, const S3ConfigParams &config_params_p)
-	    : HTTPFileHandle(fs, file, flags, std::move(http_params_p)), auth_params(auth_params_p),
-	      config_params(config_params_p), uploads_in_progress(0), parts_uploaded(0), upload_finalized(false),
-	      uploader_has_error(false), upload_exception(nullptr) {
-		auto_fallback_to_full_file_download = false;
-		if (flags.OpenForReading() && flags.OpenForWriting()) {
-			throw NotImplementedException("Cannot open an HTTP file for both reading and writing");
-		} else if (flags.OpenForAppending()) {
-			throw NotImplementedException("Cannot open an HTTP file for appending");
-		}
-	}
+	             const S3AuthParams &auth_params_p, const S3ConfigParams &config_params_p);
 	~S3FileHandle() override;
 
 	S3AuthParams auth_params;
@@ -163,6 +157,10 @@ class S3FileHandle : public HTTPFileHandle {
 
 	shared_ptr<S3WriteBuffer> GetBuffer(uint16_t write_buffer_idx);
 
+protected:
+	void InitializeFromCacheEntry(const HTTPMetadataCacheEntry &cache_entry) override;
+	HTTPMetadataCacheEntry GetCacheEntry() const override;
+
 protected:
 	string multipart_upload_id;
 	size_t part_size;
@@ -258,6 +256,7 @@ class S3FileSystem : public HTTPFileSystem {
 	}
 
 	static string GetS3BadRequestError(const S3AuthParams &s3_auth_params, string correct_region = "");
+	static string ParseS3Error(const string &error);
 	static string GetS3AuthError(const S3AuthParams &s3_auth_params);
 	static string GetGCSAuthError(const S3AuthParams &s3_auth_params);
 	static HTTPException GetS3Error(const S3AuthParams &s3_auth_params, const HTTPResponse &response,
@@ -289,7 +288,7 @@ class S3FileSystem : public HTTPFileSystem {
 
 // Helper class to do s3 ListObjectV2 api call https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html
 struct AWSListObjectV2 {
-	static string Request(const string &path, HTTPParams &http_params, const S3AuthParams &s3_auth_params,
+	static string Request(const string &path, HTTPParams &http_params, S3AuthParams &s3_auth_params,
 	                      string &continuation_token, optional_idx max_keys = optional_idx());
 	static void ParseFileList(string &aws_response, vector<OpenFileInfo> &result);
 	static vector<string> ParseCommonPrefix(string &aws_response);