Skip to content

Commit

Permalink
Merge pull request #59 from carlopi/patch
Browse files Browse the repository at this point in the history
Apply shared_ptr patch and bump duckdb
  • Loading branch information
samansmink authored May 17, 2024
2 parents 0962377 + c471069 commit 49b63dc
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 55 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/LocalTesting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ jobs:

- uses: actions/setup-python@v2
with:
python-version: '3.7'
python-version: '3.11'

- name: Setup vcpkg
uses: lukka/run-vcpkg@v11
Expand Down Expand Up @@ -178,7 +178,7 @@ jobs:

- uses: actions/setup-python@v2
with:
python-version: '3.7'
python-version: '3.11'

- name: Build extension
run: |
Expand Down Expand Up @@ -208,4 +208,4 @@ jobs:
if: always()
shell: bash
run: |
cat azurite_log.txt
cat azurite_log.txt
4 changes: 2 additions & 2 deletions .github/workflows/MainDistributionPipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ concurrency:
jobs:
duckdb-stable-build:
name: Build extension binaries
uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@v0.10.1
uses: duckdb/extension-ci-tools/.github/workflows/_extension_distribution.yml@main
with:
extension_name: azure
duckdb_version: main
Expand All @@ -23,7 +23,7 @@ jobs:
duckdb-stable-deploy:
name: Deploy extension binaries
needs: duckdb-stable-build
uses: duckdb/extension-ci-tools/.github/workflows/_extension_deploy.yml@v0.10.1
uses: duckdb/extension-ci-tools/.github/workflows/_extension_deploy.yml@main
secrets: inherit
with:
extension_name: azure
Expand Down
2 changes: 1 addition & 1 deletion duckdb
Submodule duckdb updated 1157 files
12 changes: 7 additions & 5 deletions src/azure_blob_filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include "azure_storage_account_client.hpp"
#include "duckdb.hpp"
#include "duckdb/common/exception.hpp"
#include "duckdb/common/helper.hpp"
#include "duckdb/common/shared_ptr.hpp"
#include "duckdb/common/http_state.hpp"
#include "duckdb/common/file_opener.hpp"
#include "duckdb/common/string_util.hpp"
Expand Down Expand Up @@ -201,13 +203,13 @@ void AzureBlobStorageFileSystem::ReadRange(AzureFileHandle &handle, idx_t file_o
}
}

std::shared_ptr<AzureContextState> AzureBlobStorageFileSystem::CreateStorageContext(optional_ptr<FileOpener> opener,
const string &path,
const AzureParsedUrl &parsed_url) {
shared_ptr<AzureContextState> AzureBlobStorageFileSystem::CreateStorageContext(optional_ptr<FileOpener> opener,
const string &path,
const AzureParsedUrl &parsed_url) {
auto azure_read_options = ParseAzureReadOptions(opener);

return std::make_shared<AzureBlobContextState>(ConnectToBlobStorageAccount(opener, path, parsed_url),
azure_read_options);
return make_shared_ptr<AzureBlobContextState>(ConnectToBlobStorageAccount(opener, path, parsed_url),
azure_read_options);
}

} // namespace duckdb
12 changes: 7 additions & 5 deletions src/azure_dfs_filesystem.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "azure_dfs_filesystem.hpp"
#include "azure_storage_account_client.hpp"
#include "duckdb/common/exception.hpp"
#include "duckdb/common/helper.hpp"
#include "duckdb/common/shared_ptr.hpp"
#include "duckdb/function/scalar/string_functions.hpp"
#include <algorithm>
#include <azure/storage/blobs/blob_options.hpp>
Expand Down Expand Up @@ -185,13 +187,13 @@ void AzureDfsStorageFileSystem::ReadRange(AzureFileHandle &handle, idx_t file_of
}
}

std::shared_ptr<AzureContextState> AzureDfsStorageFileSystem::CreateStorageContext(optional_ptr<FileOpener> opener,
const string &path,
const AzureParsedUrl &parsed_url) {
shared_ptr<AzureContextState> AzureDfsStorageFileSystem::CreateStorageContext(optional_ptr<FileOpener> opener,
const string &path,
const AzureParsedUrl &parsed_url) {
auto azure_read_options = ParseAzureReadOptions(opener);

return std::make_shared<AzureDfsContextState>(ConnectToDfsStorageAccount(opener, path, parsed_url),
azure_read_options);
return make_shared_ptr<AzureDfsContextState>(ConnectToDfsStorageAccount(opener, path, parsed_url),
azure_read_options);
}

} // namespace duckdb
15 changes: 8 additions & 7 deletions src/azure_filesystem.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "azure_filesystem.hpp"
#include "duckdb/common/exception.hpp"
#include "duckdb/common/shared_ptr.hpp"
#include "duckdb/common/types/value.hpp"
#include "duckdb/main/client_context.hpp"
#include <azure/storage/common/storage_exception.hpp>
Expand Down Expand Up @@ -53,8 +54,8 @@ void AzureStorageFileSystem::LoadFileInfo(AzureFileHandle &handle) {
}
}

unique_ptr<FileHandle> AzureStorageFileSystem::OpenFile(const string &path,FileOpenFlags flags,
optional_ptr<FileOpener> opener) {
unique_ptr<FileHandle> AzureStorageFileSystem::OpenFile(const string &path, FileOpenFlags flags,
optional_ptr<FileOpener> opener) {
D_ASSERT(flags.Compression() == FileCompressionType::UNCOMPRESSED);

if (flags.OpenForWriting()) {
Expand Down Expand Up @@ -153,16 +154,16 @@ int64_t AzureStorageFileSystem::Read(FileHandle &handle, void *buffer, int64_t n
return nr_bytes;
}

std::shared_ptr<AzureContextState> AzureStorageFileSystem::GetOrCreateStorageContext(optional_ptr<FileOpener> opener,
const string &path,
const AzureParsedUrl &parsed_url) {
shared_ptr<AzureContextState> AzureStorageFileSystem::GetOrCreateStorageContext(optional_ptr<FileOpener> opener,
const string &path,
const AzureParsedUrl &parsed_url) {
Value value;
bool azure_context_caching = true;
if (FileOpener::TryGetCurrentSetting(opener, "azure_context_caching", value)) {
azure_context_caching = value.GetValue<bool>();
}

std::shared_ptr<AzureContextState> result;
shared_ptr<AzureContextState> result;
if (azure_context_caching) {
auto client_context = FileOpener::TryGetClientContext(opener);

Expand All @@ -182,7 +183,7 @@ std::shared_ptr<AzureContextState> AzureStorageFileSystem::GetOrCreateStorageCon
result = CreateStorageContext(opener, path, parsed_url);
registered_state[context_key] = result;
} else {
result = std::shared_ptr<AzureContextState>(storage_account_it->second, azure_context_state);
result = shared_ptr<AzureContextState>(storage_account_it->second, azure_context_state);
}
}
} else {
Expand Down
30 changes: 18 additions & 12 deletions src/azure_storage_account_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include "duckdb/catalog/catalog_transaction.hpp"
#include "duckdb/common/enums/statement_type.hpp"
#include "duckdb/common/exception.hpp"
#include "duckdb/common/shared_ptr.hpp"
#include "duckdb/common/helper.hpp"
#include "duckdb/common/file_opener.hpp"
#include "duckdb/common/string_util.hpp"
#include "duckdb/main/client_context.hpp"
Expand Down Expand Up @@ -75,12 +77,12 @@ static std::string AccountUrl(const AzureParsedUrl &azure_parsed_url) {

template <typename T>
static T ToClientOptions(const Azure::Core::Http::Policies::TransportOptions &transport_options,
std::shared_ptr<HTTPState> http_state) {
shared_ptr<HTTPState> http_state) {
static_assert(std::is_base_of<Azure::Core::_internal::ClientOptions, T>::value,
"type parameter must be an Azure ClientOptions");
T options;
options.Transport = transport_options;
if (nullptr != http_state) {
if (http_state != nullptr) {
// Because we mainly want to have stats on what has been needed and not on
// what has been used on the network, we register the policy on `PerOperationPolicies`
// part and not the `PerRetryPolicies`. Network issues will result in retry that can
Expand All @@ -92,13 +94,13 @@ static T ToClientOptions(const Azure::Core::Http::Policies::TransportOptions &tr

static Azure::Storage::Blobs::BlobClientOptions
ToBlobClientOptions(const Azure::Core::Http::Policies::TransportOptions &transport_options,
std::shared_ptr<HTTPState> http_state) {
shared_ptr<HTTPState> http_state) {
return ToClientOptions<Azure::Storage::Blobs::BlobClientOptions>(transport_options, std::move(http_state));
}

static Azure::Storage::Files::DataLake::DataLakeClientOptions
ToDfsClientOptions(const Azure::Core::Http::Policies::TransportOptions &transport_options,
std::shared_ptr<HTTPState> http_state) {
shared_ptr<HTTPState> http_state) {
return ToClientOptions<Azure::Storage::Files::DataLake::DataLakeClientOptions>(transport_options,
std::move(http_state));
}
Expand All @@ -110,14 +112,14 @@ ToTokenCredentialOptions(const Azure::Core::Http::Policies::TransportOptions &tr
return options;
}

static std::shared_ptr<HTTPState> GetHttpState(optional_ptr<FileOpener> opener) {
static shared_ptr<HTTPState> GetHttpState(optional_ptr<FileOpener> opener) {
Value value;
bool enable_http_stats = false;
if (FileOpener::TryGetCurrentSetting(opener, "azure_http_stats", value)) {
enable_http_stats = value.GetValue<bool>();
}

std::shared_ptr<HTTPState> http_state;
shared_ptr<HTTPState> http_state;
if (enable_http_stats) {
http_state = HTTPState::TryGetState(opener);
}
Expand Down Expand Up @@ -168,7 +170,7 @@ CreateClientCredential(const std::string &tenant_id, const std::string &client_i
auto credential_options = ToTokenCredentialOptions(transport_options);
if (!client_secret.empty()) {
return std::make_shared<Azure::Identity::ClientSecretCredential>(tenant_id, client_id, client_secret,
credential_options);
credential_options);
} else if (!client_certificate_path.empty()) {
return std::make_shared<Azure::Identity::ClientCertificateCredential>(
tenant_id, client_id, client_certificate_path, credential_options);
Expand Down Expand Up @@ -408,8 +410,9 @@ GetDfsStorageAccountClientFromServicePrincipalProvider(optional_ptr<FileOpener>
return Azure::Storage::Files::DataLake::DataLakeServiceClient(account_url, token_credential, dfs_options);
}

static Azure::Storage::Blobs::BlobServiceClient
GetBlobStorageAccountClient(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, const AzureParsedUrl &azure_parsed_url) {
static Azure::Storage::Blobs::BlobServiceClient GetBlobStorageAccountClient(optional_ptr<FileOpener> opener,
const KeyValueSecret &secret,
const AzureParsedUrl &azure_parsed_url) {
auto &provider = secret.GetProvider();
// default provider
if (provider == "config") {
Expand All @@ -424,7 +427,8 @@ GetBlobStorageAccountClient(optional_ptr<FileOpener> opener, const KeyValueSecre
}

static Azure::Storage::Files::DataLake::DataLakeServiceClient
GetDfsStorageAccountClient(optional_ptr<FileOpener> opener, const KeyValueSecret &secret, const AzureParsedUrl &azure_parsed_url) {
GetDfsStorageAccountClient(optional_ptr<FileOpener> opener, const KeyValueSecret &secret,
const AzureParsedUrl &azure_parsed_url) {
auto &provider = secret.GetProvider();
// default provider
if (provider == "config") {
Expand Down Expand Up @@ -505,7 +509,8 @@ const SecretMatch LookupSecret(optional_ptr<FileOpener> opener, const std::strin
return {};
}

Azure::Storage::Blobs::BlobServiceClient ConnectToBlobStorageAccount(optional_ptr<FileOpener> opener, const std::string &path,
Azure::Storage::Blobs::BlobServiceClient ConnectToBlobStorageAccount(optional_ptr<FileOpener> opener,
const std::string &path,
const AzureParsedUrl &azure_parsed_url) {

auto secret_match = LookupSecret(opener, path);
Expand All @@ -519,7 +524,8 @@ Azure::Storage::Blobs::BlobServiceClient ConnectToBlobStorageAccount(optional_pt
}

Azure::Storage::Files::DataLake::DataLakeServiceClient
ConnectToDfsStorageAccount(optional_ptr<FileOpener> opener, const std::string &path, const AzureParsedUrl &azure_parsed_url) {
ConnectToDfsStorageAccount(optional_ptr<FileOpener> opener, const std::string &path,
const AzureParsedUrl &azure_parsed_url) {
auto secret_match = LookupSecret(opener, path);
if (secret_match.HasMatch()) {
const auto &base_secret = secret_match.GetSecret();
Expand Down
7 changes: 4 additions & 3 deletions src/http_state_policy.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "http_state_policy.hpp"
#include <azure/core/http/http.hpp>
#include "duckdb/common/shared_ptr.hpp"
#include <memory>
#include <string>
#include <utility>
Expand All @@ -8,7 +9,7 @@ const static std::string CONTENT_LENGTH = "content-length";

namespace duckdb {

HttpStatePolicy::HttpStatePolicy(std::shared_ptr<HTTPState> http_state) : http_state(std::move(http_state)) {
HttpStatePolicy::HttpStatePolicy(shared_ptr<HTTPState> http_state) : http_state(std::move(http_state)) {
}

std::unique_ptr<Azure::Core::Http::RawResponse>
Expand All @@ -34,12 +35,12 @@ HttpStatePolicy::Send(Azure::Core::Http::Request &request, Azure::Core::Http::Po
}

const auto *body_stream = request.GetBodyStream();
if (nullptr != body_stream) {
if (body_stream != nullptr) {
http_state->total_bytes_sent += body_stream->Length();
}

auto result = next_policy.Send(request, context);
if (nullptr != result) {
if (result != nullptr) {
const auto &response_body = result->GetBody();
if (response_body.size() != 0) {
http_state->total_bytes_received += response_body.size();
Expand Down
10 changes: 6 additions & 4 deletions src/include/azure_blob_filesystem.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#pragma once

#include "duckdb.hpp"
#include "duckdb/common/shared_ptr.hpp"
#include "duckdb/common/unique_ptr.hpp"
#include "azure_parsed_url.hpp"
#include "azure_filesystem.hpp"
#include <azure/storage/blobs/blob_client.hpp>
Expand Down Expand Up @@ -57,10 +59,10 @@ class AzureBlobStorageFileSystem : public AzureStorageFileSystem {
const string &GetContextPrefix() const override {
return PATH_PREFIX;
}
std::shared_ptr<AzureContextState> CreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url) override;
duckdb::unique_ptr<AzureFileHandle> CreateHandle(const string &path, FileOpenFlags flags,
optional_ptr<FileOpener> opener) override;
shared_ptr<AzureContextState> CreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url) override;
unique_ptr<AzureFileHandle> CreateHandle(const string &path, FileOpenFlags flags,
optional_ptr<FileOpener> opener) override;

void ReadRange(AzureFileHandle &handle, idx_t file_offset, char *buffer_out, idx_t buffer_out_len) override;
};
Expand Down
10 changes: 6 additions & 4 deletions src/include/azure_dfs_filesystem.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include "azure_filesystem.hpp"
#include "duckdb/common/file_opener.hpp"
#include "duckdb/common/shared_ptr.hpp"
#include "duckdb/common/unique_ptr.hpp"
#include <azure/storage/files/datalake/datalake_file_client.hpp>
#include <azure/storage/files/datalake/datalake_file_system_client.hpp>
#include <azure/storage/files/datalake/datalake_service_client.hpp>
Expand Down Expand Up @@ -55,10 +57,10 @@ class AzureDfsStorageFileSystem : public AzureStorageFileSystem {
const string &GetContextPrefix() const override {
return PATH_PREFIX;
}
std::shared_ptr<AzureContextState> CreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url) override;
duckdb::unique_ptr<AzureFileHandle> CreateHandle(const string &path, FileOpenFlags flags,
optional_ptr<FileOpener> opener) override;
shared_ptr<AzureContextState> CreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url) override;
unique_ptr<AzureFileHandle> CreateHandle(const string &path, FileOpenFlags flags,
optional_ptr<FileOpener> opener) override;

void ReadRange(AzureFileHandle &handle, idx_t file_offset, char *buffer_out, idx_t buffer_out_len) override;
};
Expand Down
11 changes: 6 additions & 5 deletions src/include/azure_filesystem.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "azure_parsed_url.hpp"
#include "duckdb/common/assert.hpp"
#include "duckdb/common/file_opener.hpp"
#include "duckdb/common/shared_ptr.hpp"
#include "duckdb/common/file_system.hpp"
#include "duckdb/main/client_context_state.hpp"
#include <azure/core/datetime.hpp>
Expand Down Expand Up @@ -99,14 +100,14 @@ class AzureStorageFileSystem : public FileSystem {

protected:
virtual duckdb::unique_ptr<AzureFileHandle> CreateHandle(const string &path, FileOpenFlags flags,
optional_ptr<FileOpener> opener) = 0;
optional_ptr<FileOpener> opener) = 0;
virtual void ReadRange(AzureFileHandle &handle, idx_t file_offset, char *buffer_out, idx_t buffer_out_len) = 0;

virtual const string &GetContextPrefix() const = 0;
std::shared_ptr<AzureContextState> GetOrCreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url);
virtual std::shared_ptr<AzureContextState> CreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url) = 0;
shared_ptr<AzureContextState> GetOrCreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url);
virtual shared_ptr<AzureContextState> CreateStorageContext(optional_ptr<FileOpener> opener, const string &path,
const AzureParsedUrl &parsed_url) = 0;

virtual void LoadRemoteFileInfo(AzureFileHandle &handle) = 0;
static AzureReadOptions ParseAzureReadOptions(optional_ptr<FileOpener> opener);
Expand Down
6 changes: 4 additions & 2 deletions src/include/azure_storage_account_client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@

namespace duckdb {

Azure::Storage::Blobs::BlobServiceClient ConnectToBlobStorageAccount(optional_ptr<FileOpener> opener, const std::string &path,
Azure::Storage::Blobs::BlobServiceClient ConnectToBlobStorageAccount(optional_ptr<FileOpener> opener,
const std::string &path,
const AzureParsedUrl &azure_parsed_url);

Azure::Storage::Files::DataLake::DataLakeServiceClient
ConnectToDfsStorageAccount(optional_ptr<FileOpener> opener, const std::string &path, const AzureParsedUrl &azure_parsed_url);
ConnectToDfsStorageAccount(optional_ptr<FileOpener> opener, const std::string &path,
const AzureParsedUrl &azure_parsed_url);

} // namespace duckdb
Loading

0 comments on commit 49b63dc

Please sign in to comment.