Bump DuckDB to v1.5.5 and build Windows binaries

.github/workflows/MainDistributionPipeline.yml

@@ -19,20 +19,20 @@ concurrency:
 jobs:
   duckdb-stable-build:
     name: Build extension binaries
-    uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v1.5.1
+    uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v1.5.5
     with:
-      duckdb_version: v1.5.1
-      ci_tools_version: v1.5.1
+      duckdb_version: v1.5.5
+      ci_tools_version: v1.5.5
       extension_name: proto_iceberg
-      # iceberg-cpp does not build for Windows or WebAssembly yet.
-      exclude_archs: 'windows_amd64;windows_amd64_mingw;wasm_mvp;wasm_eh;wasm_threads'
+      # windows_amd64_mingw builds with rtools42's GCC, which predates C++23; iceberg-cpp does not build for WebAssembly.
+      exclude_archs: 'windows_amd64_mingw;wasm_mvp;wasm_eh;wasm_threads'
 
   code-quality-check:
     name: Code Quality Check
-    uses: duckdb/extension-ci-tools/.github/workflows/_extension_code_quality.yml@v1.5.1
+    uses: duckdb/extension-ci-tools/.github/workflows/_extension_code_quality.yml@v1.5.5
     with:
-      duckdb_version: v1.5.1
-      ci_tools_version: v1.5.1
+      duckdb_version: v1.5.5
+      ci_tools_version: v1.5.5
       extension_name: proto_iceberg
       # FIXME: Disable tidy to prevent full compilation
       format_checks: 'format'

.gitmodules

@@ -5,9 +5,9 @@
 [submodule "extension-ci-tools"]
 	path = extension-ci-tools
 	url = https://github.com/duckdb/extension-ci-tools
-	branch = v1.5.1
+	branch = v1.5.5
 # TODO: Switch to upstream apache/iceberg-cpp once requirements merged
 [submodule "third_party/iceberg-cpp"]
 	path = third_party/iceberg-cpp
 	url = https://github.com/smaheshwar-pltr/iceberg-cpp.git
-	branch = duckdb-iceberg
+	branch = duckdb-proto-iceberg

README.md

@@ -39,7 +39,7 @@ GEN=ninja make release  # or: GEN=ninja make debug
 
 The first build takes longer: CMake downloads and builds iceberg-cpp and its vendored dependencies (Arrow, the AWS SDK and others) while configuring. Later builds skip this unless the submodule or its build settings change. Set `CMAKE_BUILD_PARALLEL_LEVEL` to limit build parallelism on machines with little memory.
 
-CI also builds loadable binaries for Linux (amd64, arm64) and macOS 13.3+ (amd64, arm64) with DuckDB's [extension-ci-tools](https://github.com/duckdb/extension-ci-tools); each run of the Main Extension Distribution Pipeline workflow attaches them as artifacts.
+CI also builds loadable binaries for Linux (amd64, arm64), macOS 13.3+ (amd64, arm64) and Windows (amd64) with DuckDB's [extension-ci-tools](https://github.com/duckdb/extension-ci-tools); each run of the Main Extension Distribution Pipeline workflow attaches them as artifacts.
 
 See [docs/UPDATING.md](docs/UPDATING.md) for updating DuckDB or iceberg-cpp.
 

cmake/duckdb_cxx23_patch.cmake

@@ -1,4 +1,4 @@
-# DuckDB v1.5.1's profiling_utils.hpp does not compile in a C++23 translation unit under libc++
+# DuckDB v1.5's profiling_utils.hpp does not compile in a C++23 translation unit under libc++
 # (Apple's standard library): QueryMetrics resets a unique_ptr<ActiveTimer> in an inline member
 # function before ActiveTimer is complete, and libc++ instantiates the constexpr unique_ptr members
 # eagerly in C++23. The patch moves those member functions below ActiveTimer's definition. It is
@@ -17,7 +17,7 @@ if(APPLE)
                             RESULT_VARIABLE result)
             if(NOT result EQUAL 0)
                 message(FATAL_ERROR "[proto_iceberg] Cannot apply ${patch} to the DuckDB tree at "
-                                    "${CMAKE_SOURCE_DIR}. Is it still DuckDB v1.5.1?")
+                                    "${CMAKE_SOURCE_DIR}. Is it still DuckDB v1.5?")
             endif()
             message(STATUS "[proto_iceberg] Applied ${patch}")
         endif()

cmake/iceberg_cpp.cmake

@@ -18,6 +18,8 @@ block()
     set(init_cache [==[
 set(CMAKE_POSITION_INDEPENDENT_CODE ON CACHE BOOL "")
 set(CMAKE_INSTALL_LIBDIR lib CACHE PATH "")
+# Lets the forwarded CMAKE_MSVC_RUNTIME_LIBRARY reach bundled dependencies that predate CMake 3.15.
+set(CMAKE_POLICY_DEFAULT_CMP0091 NEW CACHE STRING "")
 # Build every dependency from source rather than picking up system packages.
 set(FETCHCONTENT_TRY_FIND_PACKAGE_MODE NEVER CACHE STRING "")
 set(ICEBERG_BUILD_STATIC ON CACHE BOOL "")
@@ -40,6 +42,8 @@ set(ICEBERG_S3 ON CACHE BOOL "")
             CMAKE_OSX_ARCHITECTURES
             CMAKE_OSX_DEPLOYMENT_TARGET
             CMAKE_OSX_SYSROOT
+            # DuckDB links the static MSVC runtime, and all objects in the extension must agree.
+            CMAKE_MSVC_RUNTIME_LIBRARY
             # Set by the vcpkg toolchain, so that the sub-build uses the same installed packages.
             VCPKG_TARGET_TRIPLET
             VCPKG_HOST_TRIPLET
@@ -49,6 +53,11 @@ set(ICEBERG_S3 ON CACHE BOOL "")
         endif()
     endforeach()
 
+    # The AWS libraries Arrow bundles choose their MSVC runtime from their own option instead.
+    if(MSVC AND CMAKE_MSVC_RUNTIME_LIBRARY AND NOT CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "DLL")
+        string(APPEND init_cache "set(AWS_STATIC_MSVC_RUNTIME_LIBRARY ON CACHE BOOL \"\")\n")
+    endif()
+
     execute_process(COMMAND git rev-parse HEAD
                     WORKING_DIRECTORY "${source_dir}"
                     OUTPUT_VARIABLE revision

docs/UPDATING.md

@@ -5,7 +5,7 @@
 The extension targets a DuckDB release, which is pinned in several places that must move together. Check out submodule commits explicitly rather than using `make update` or `make pull`, which move every submodule to the tip of its tracked branch.
 
 - The `duckdb` submodule: check out the release tag, and set its `branch` in `.gitmodules` to the release branch.
-- The `extension-ci-tools` submodule: check out the branch named after the release (e.g. `v1.5.1`), and set its `branch` in `.gitmodules` to match.
+- The `extension-ci-tools` submodule: check out the branch named after the release (e.g. `v1.5.5`), and set its `branch` in `.gitmodules` to match.
 - `.github/workflows/MainDistributionPipeline.yml`: the reusable workflow refs and the `duckdb_version` and `ci_tools_version` inputs.
 - `extension_config.cmake`: the httpfs `GIT_TAG`, which should match `duckdb/.github/config/extensions/httpfs.cmake`.
 - `cmake/duckdb_cxx23.patch`: check whether it is still needed. On macOS, configuring fails if it no longer applies.

extension_config.cmake

@@ -1,5 +1,5 @@
 # Extensions built by `make`: this one, plus httpfs, which the SQL tests use to read table data from S3.
-# httpfs is pinned to the commit DuckDB v1.5.1 builds against.
+# httpfs is pinned to the commit DuckDB v1.5.5 builds against.
 
 duckdb_extension_load(proto_iceberg
     SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}
@@ -8,5 +8,5 @@ duckdb_extension_load(proto_iceberg
 
 duckdb_extension_load(httpfs
     GIT_URL https://github.com/duckdb/duckdb-httpfs
-    GIT_TAG 7e86e7a5e5a1f01f458361bebdfa9b0a9a73a619
+    GIT_TAG 827222fb45a043a7a852d1f7aae46901492a3cda
 )

src/include/proto_iceberg_catalog.hpp

@@ -7,16 +7,16 @@
 #include "duckdb/parser/parsed_data/attach_info.hpp"
 #include "duckdb/storage/storage_extension.hpp"
 
-#include "iceberg/catalog/rest/rest_catalog.h"
+#include "iceberg/catalog.h"
 
 namespace duckdb {
 
 class ProtoIcebergSchemaEntry;
 
 class ProtoIcebergCatalog : public Catalog {
 public:
-	ProtoIcebergCatalog(AttachedDatabase &db_p, string catalog_uri,
-	                    std::shared_ptr<iceberg::rest::RestCatalog> rest_catalog, string default_schema);
+	ProtoIcebergCatalog(AttachedDatabase &db_p, string catalog_uri, std::shared_ptr<iceberg::Catalog> rest_catalog,
+	                    string default_schema);
 
 	static unique_ptr<Catalog> Attach(optional_ptr<StorageExtensionInfo> storage_info, ClientContext &context,
 	                                  AttachedDatabase &db, const string &name, AttachInfo &info,
@@ -32,7 +32,7 @@ class ProtoIcebergCatalog : public Catalog {
 
 	/// Acquires exclusive use of the REST catalog. Every call into it, including through a loaded iceberg::Table (e.g.
 	/// Table::Refresh()), must hold the returned guard, and only for the duration of that call.
-	Mutex<std::shared_ptr<iceberg::rest::RestCatalog>>::Guard LockRestCatalog() {
+	Mutex<std::shared_ptr<iceberg::Catalog>>::Guard LockRestCatalog() {
 		return rest_catalog_.Lock();
 	}
 
@@ -93,7 +93,7 @@ class ProtoIcebergCatalog : public Catalog {
 	string default_schema_;
 	// TODO: Drop the lock once iceberg-cpp documents RestCatalog as thread-safe. It currently isn't: its HttpClient
 	//   shares one libcurl connection cache across all requests.
-	Mutex<std::shared_ptr<iceberg::rest::RestCatalog>> rest_catalog_;
+	Mutex<std::shared_ptr<iceberg::Catalog>> rest_catalog_;
 };
 
 } // namespace duckdb

src/include/s3_conversion.hpp

@@ -4,8 +4,11 @@
 
 #include "duckdb/common/case_insensitive_map.hpp"
 #include "duckdb/common/types/value.hpp"
+#include "iceberg/storage_credential.h"
 
+#include <span>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 
 namespace duckdb::conversion {
@@ -21,4 +24,13 @@ ConvertS3SecretToIcebergProperties(const case_insensitive_tree_t<Value> &secret)
 [[nodiscard]] case_insensitive_map_t<Value>
 ConvertIcebergPropertiesToS3Secret(const std::unordered_map<std::string, std::string> &properties);
 
+/// Overlays the vended S3 storage credential whose prefix is the longest match for location onto FileIO properties.
+///
+/// REST catalogs vend credentials either in the table config, which iceberg-cpp merges into the FileIO properties,
+/// or as storage credentials scoped to location prefixes, which it keeps separately. Matching follows iceberg-cpp:
+/// non-S3 prefixes are skipped and S3 scheme aliases such as s3a:// compare equal to s3://.
+[[nodiscard]] std::unordered_map<std::string, std::string>
+MergeStorageCredential(std::unordered_map<std::string, std::string> properties,
+                       std::span<const iceberg::StorageCredential> credentials, std::string_view location);
+
 } // namespace duckdb::conversion

src/proto_iceberg_catalog.cpp

@@ -11,8 +11,7 @@
 namespace duckdb {
 
 ProtoIcebergCatalog::ProtoIcebergCatalog(AttachedDatabase &db_p, string catalog_uri,
-                                         std::shared_ptr<iceberg::rest::RestCatalog> rest_catalog,
-                                         string default_schema)
+                                         std::shared_ptr<iceberg::Catalog> rest_catalog, string default_schema)
     : Catalog(db_p), catalog_uri_(std::move(catalog_uri)), default_schema_(std::move(default_schema)),
       rest_catalog_(std::in_place, std::move(rest_catalog)) {
 }

src/proto_iceberg_catalog_attach.cpp

@@ -9,6 +9,7 @@
 #include "duckdb/main/secret/secret.hpp"
 
 #include "iceberg/catalog/rest/catalog_properties.h"
+#include "iceberg/catalog/rest/rest_catalog.h"
 
 #include <unordered_map>
 
@@ -27,6 +28,9 @@ using constants::kWarehouse;
 
 const string kUri = "uri";
 const string kHeaderAuthorization = "header.Authorization";
+/// Asks the catalog to vend storage credentials with each loaded table. Some catalogs only vend when asked.
+const string kHeaderAccessDelegation = "header.X-Iceberg-Access-Delegation";
+const string kVendedCredentials = "vended-credentials";
 /// Path to look up the user's S3 secret at: only secrets scoped to all S3 paths match it, not per-bucket ones or the
 /// per-table secrets this extension creates.
 const string kS3RootPath = "s3://";
@@ -134,10 +138,15 @@ unique_ptr<Catalog> ProtoIcebergCatalog::Attach(optional_ptr<StorageExtensionInf
 	if (!params.token.empty()) {
 		catalog_props[kHeaderAuthorization] = kBearerPrefix + params.token;
 	}
+	catalog_props[kHeaderAccessDelegation] = kVendedCredentials;
+	// Metrics reports would add a request to the catalog server for every scan, outside the catalog lock.
+	catalog_props[iceberg::rest::RestCatalogProperties::kMetricsReportingEnabled.key()] = "false";
 
 	auto config = iceberg::rest::RestCatalogProperties::FromMap(catalog_props);
-	auto rest_catalog = UnwrapOrThrow(iceberg::rest::RestCatalog::Make(config),
-	                                  "Failed to create Iceberg REST catalog at %s", params.uri);
+	auto session_catalog = UnwrapOrThrow(iceberg::rest::RestCatalog::Make(config),
+	                                     "Failed to create Iceberg REST catalog at %s", params.uri);
+	auto rest_catalog =
+	    UnwrapOrThrow(session_catalog->AsCatalog(), "Failed to open Iceberg REST catalog at %s", params.uri);
 
 	auto catalog = make_uniq<ProtoIcebergCatalog>(db, std::move(params.uri), std::move(rest_catalog),
 	                                              std::move(params.default_schema));

src/proto_iceberg_extension.cpp

@@ -13,6 +13,8 @@
 #include "iceberg/arrow/arrow_register.h"
 #include "iceberg/avro/avro_register.h"
 
+#include <cstdlib>
+
 namespace duckdb {
 namespace {
 
@@ -53,6 +55,23 @@ void RegisterIcebergFileIO() {
 	iceberg::avro::RegisterAll();
 }
 
+/// Turns off the AWS SDK's logging unless the user configured it, before iceberg-cpp initializes Arrow's S3 support.
+///
+/// When Arrow finalizes S3 at exit, the AWS CRT can still be shutting down an event loop on another thread, which
+/// logs through the SDK's logger after the SDK has torn it down and crashes. With logging off the SDK installs no
+/// logger. Arrow reads ARROW_S3_LOG_LEVEL when S3 is first initialized. The variable is process-wide, so it also
+/// applies to other Arrow S3 users in the process and to child processes.
+void DisableAwsSdkLogging() {
+	static constexpr const char *kArrowS3LogLevel = "ARROW_S3_LOG_LEVEL";
+#ifdef _WIN32
+	if (!std::getenv(kArrowS3LogLevel)) {
+		_putenv_s(kArrowS3LogLevel, "off");
+	}
+#else
+	setenv(kArrowS3LogLevel, "off", /*overwrite=*/0);
+#endif
+}
+
 void RegisterIcebergSecretType(ExtensionLoader &loader) {
 	SecretType iceberg_secret_type;
 	iceberg_secret_type.name = kIcebergSecretType;
@@ -74,6 +93,7 @@ void RegisterIcebergSecretFunction(ExtensionLoader &loader) {
 void LoadInternal(ExtensionLoader &loader) {
 	auto &instance = loader.GetDatabaseInstance();
 
+	DisableAwsSdkLogging();
 	RegisterIcebergFileIO();
 
 	RegisterIcebergSecretType(loader);

src/proto_iceberg_table_entry.cpp

@@ -67,15 +67,20 @@ CreateSecretInput BuildScopedS3Secret(const string &catalog_name, const iceberg:
 
 	auto input = MakeBaseS3SecretInput();
 	input.name = GenerateScopedSecretName(catalog_name, table.name(), txn_id);
-	input.scope.push_back(std::move(scope_prefix));
+	input.scope.push_back(scope_prefix);
 	// Scope to write.data.path too, that may live outside the table's location
 	if (string write_data_path {table.properties().Get(iceberg::TableProperties::kWriteDataLocation)};
 	    !write_data_path.empty()) {
 		input.scope.push_back(scope_with_slash(std::move(write_data_path)));
 	}
 
-	// TODO: Respect storage credentials REST field, not just the credentials in IO properties
-	input.options = conversion::ConvertIcebergPropertiesToS3Secret(io->properties());
+	std::span<const iceberg::StorageCredential> credentials;
+	if (const auto *credentialed = io->AsSupportsStorageCredentials()) {
+		credentials = credentialed->credentials();
+	}
+	// Match on the slash-terminated table location, so that a credential for "<location>/" applies.
+	input.options = conversion::ConvertIcebergPropertiesToS3Secret(
+	    conversion::MergeStorageCredential(io->properties(), credentials, scope_prefix));
 	return input;
 }
 

src/s3_conversion.cpp

@@ -1,6 +1,7 @@
 #include "s3_conversion.hpp"
 
 #include "iceberg/arrow/s3/s3_properties.h"
+#include "iceberg/util/property_util.h"
 
 #include <array>
 #include <optional>
@@ -31,20 +32,9 @@ constexpr std::array<PlainPropertyMapping, 4> kPlainPropertyMappings = {{
     {"key_id", S3Properties::kAccessKeyId},
     {"secret", S3Properties::kSecretAccessKey},
     {"session_token", S3Properties::kSessionToken},
-    {"region", S3Properties::kRegion},
+    {"region", S3Properties::kClientRegion},
 }};
 
-/// Parses an iceberg-cpp boolean property, which is exactly "true" or "false".
-std::optional<bool> ParseBool(std::string_view value) {
-	if (value == "true") {
-		return true;
-	}
-	if (value == "false") {
-		return false;
-	}
-	return std::nullopt;
-}
-
 /// Returns whether an Iceberg S3 endpoint's http(s) scheme implies SSL, or nullopt if it has no such scheme.
 std::optional<bool> SchemeUsesSsl(std::string_view endpoint) {
 	if (endpoint.starts_with(kHttpsScheme)) {
@@ -56,6 +46,16 @@ std::optional<bool> SchemeUsesSsl(std::string_view endpoint) {
 	return std::nullopt;
 }
 
+/// Rewrites an S3 scheme alias such as s3a:// or S3:// to s3://, as iceberg-cpp does before matching credential
+/// prefixes.
+std::string CanonicalizeS3Scheme(std::string_view location) {
+	if (auto separator = location.find("://");
+	    separator != std::string_view::npos && iceberg::arrow::IsS3Scheme(location.substr(0, separator))) {
+		return "s3://" + std::string(location.substr(separator + 3));
+	}
+	return std::string(location);
+}
+
 /// Returns DuckDB's scheme-less form of an Iceberg S3 endpoint, e.g. host:9000/path for http://host:9000/path/.
 std::string_view ToDuckDBEndpoint(std::string_view endpoint) {
 	if (endpoint.starts_with(kHttpsScheme)) {
@@ -123,19 +123,47 @@ ConvertIcebergPropertiesToS3Secret(const std::unordered_map<std::string, std::st
 			options[string(duckdb_key)] = Value(string(value));
 		}
 	}
-	if (auto path_style = ParseBool(get(S3Properties::kPathStyleAccess))) {
+	// iceberg-cpp reads a present boolean as true only if it is "true", ignoring case.
+	if (auto path_style =
+	        iceberg::PropertyUtil::PropertyAsOptionalBoolean(properties, S3Properties::kPathStyleAccess)) {
 		options[kUrlStyle] = Value(*path_style ? kUrlStylePath : kUrlStyleVhost);
 	}
 	auto endpoint = get(S3Properties::kEndpoint);
 	if (auto duckdb_endpoint = ToDuckDBEndpoint(endpoint); !duckdb_endpoint.empty()) {
 		options[kEndpoint] = Value(string(duckdb_endpoint));
 	}
-	// The endpoint's scheme takes precedence over s3.ssl.enabled, as in iceberg-cpp: the AWS SDK uses an http(s)
-	// endpoint verbatim and only prefixes scheme-less ones with the scheme s3.ssl.enabled selects.
-	if (auto use_ssl = SchemeUsesSsl(endpoint).or_else([&] { return ParseBool(get(S3Properties::kSslEnabled)); })) {
+	// s3.ssl.enabled takes precedence over the endpoint's scheme, as in iceberg-cpp.
+	if (auto use_ssl =
+	        iceberg::PropertyUtil::PropertyAsOptionalBoolean(properties, S3Properties::kSslEnabled).or_else([&] {
+		        return SchemeUsesSsl(endpoint);
+	        })) {
 		options[kUseSsl] = Value::BOOLEAN(*use_ssl);
 	}
 	return options;
 }
 
+std::unordered_map<std::string, std::string>
+MergeStorageCredential(std::unordered_map<std::string, std::string> properties,
+                       std::span<const iceberg::StorageCredential> credentials, std::string_view location) {
+	const auto canonical_location = CanonicalizeS3Scheme(location);
+	const iceberg::StorageCredential *best = nullptr;
+	size_t best_length = 0;
+	for (const auto &credential : credentials) {
+		if (!iceberg::arrow::IsS3CredentialPrefix(credential.prefix)) {
+			continue;
+		}
+		auto prefix = CanonicalizeS3Scheme(credential.prefix);
+		if (prefix.size() > best_length && canonical_location.starts_with(prefix)) {
+			best = &credential;
+			best_length = prefix.size();
+		}
+	}
+	if (best) {
+		for (const auto &[key, value] : best->config) {
+			properties.insert_or_assign(key, value);
+		}
+	}
+	return properties;
+}
+
 } // namespace duckdb::conversion