Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply shared_ptr patch and bump duckdb #59

Merged
merged 4 commits into from
May 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading