From bfb8cdd97c888579b08f50797d617a4787a1a36d Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Tue, 14 Nov 2023 16:47:23 +0900 Subject: [PATCH 1/7] GH-38699: [C++][FS][Azure] Implement `CreateDir()` It seems that we can't create a directory explicitly without hierarchical namespace support. It seems that Azure Blob Storage supports only virtual directory. There is no directory. If a file (blob) name has "/", it's treated that the file (blob) exists under a virtual directory. It seems that Azure Data Lake Storage Gen2 supports a real directory. See also: https://learn.microsoft.com/en-us/azure/storage/blobs/storage-blobs-introduction This change chooses the following behavior: * Container can be created with/without hierarchical namespace support. * Directory can be created with hierarchical namespace support. * Directory can't be created without hierarchical namespace support. (`arrow::Status::NotImplemented` is returned for this case.) Azurite doesn't support hierarchical namespace yet. So I can't test the implementation for hierarchical namespace yet. Sorry. --- cpp/src/arrow/filesystem/azurefs.cc | 111 ++++++++++++++++++++++- cpp/src/arrow/filesystem/azurefs_test.cc | 85 ++++++++++++++++- 2 files changed, 194 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 6359183d90b..2dfe7cced41 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -611,6 +611,110 @@ class AzureFileSystem::Impl { RETURN_NOT_OK(ptr->Init()); return ptr; } + + Status CreateDir(const AzurePath& path) { + if (path.container.empty()) { + return Status::Invalid("Cannot create an empty container"); + } + + if (path.path_to_file.empty()) { + auto container_client = + blob_service_client_->GetBlobContainerClient(path.container); + try { + auto response = container_client.Create(); + if (response.Value.Created) { + return Status::OK(); + } else { + const auto& body = response.RawResponse->GetBody(); + std::string_view body_text(reinterpret_cast(body.data()), + body.size()); + return Status::IOError("Failed to create a container: ", path.container, " (", + container_client.GetUrl(), + "): ", response.RawResponse->GetReasonPhrase(), " (", + static_cast(response.RawResponse->GetStatusCode()), + "): ", body_text); + } + } catch (const Azure::Storage::StorageException& exception) { + return internal::ExceptionToStatus( + "Failed to create a container: " + path.container + " (" + + container_client.GetUrl() + ")", + exception); + } + } + + ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled, + hierarchical_namespace_.Enabled(path.container)); + if (!hierarchical_namespace_enabled) { + return Status::NotImplemented( + "Cannot create a directory without hierarchical namespace: ", path.full_path); + } + auto directory_client = datalake_service_client_->GetFileSystemClient(path.container) + .GetDirectoryClient(path.path_to_file); + try { + auto response = directory_client.Create(); + if (response.Value.Created) { + return Status::OK(); + } else { + const auto& body = response.RawResponse->GetBody(); + std::string_view body_text(reinterpret_cast(body.data()), + body.size()); + return Status::IOError("Failed to create a directory: ", path.path_to_file, " (", + directory_client.GetUrl(), + "): ", response.RawResponse->GetReasonPhrase(), " (", + static_cast(response.RawResponse->GetStatusCode()), + "): ", body_text); + } + } catch (const Azure::Storage::StorageException& exception) { + return internal::ExceptionToStatus( + "Failed to create a directory: " + path.path_to_file + " (" + + directory_client.GetUrl() + ")", + exception); + } + } + + Status CreateDirRecursive(const AzurePath& path) { + if (path.container.empty()) { + return Status::Invalid("Cannot create an empty container"); + } + + auto container_client = blob_service_client_->GetBlobContainerClient(path.container); + try { + container_client.CreateIfNotExists(); + } catch (const Azure::Storage::StorageException& exception) { + return internal::ExceptionToStatus( + "Failed to create a container: " + path.container + " (" + + container_client.GetUrl() + ")", + exception); + } + + ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled, + hierarchical_namespace_.Enabled(path.container)); + if (!hierarchical_namespace_enabled) { + return Status::NotImplemented( + "Cannot create a directory without hierarchical namespace: ", path.full_path); + } + + std::string current_path; + for (const auto& part : path.path_to_file_parts) { + if (!current_path.empty()) { + current_path += internal::kSep; + } + current_path += part; + auto directory_client = + datalake_service_client_->GetFileSystemClient(path.container) + .GetDirectoryClient(current_path); + try { + directory_client.CreateIfNotExists(); + } catch (const Azure::Storage::StorageException& exception) { + return internal::ExceptionToStatus( + "Failed to create a directory: " + current_path + " (" + + directory_client.GetUrl() + ")", + exception); + } + } + + return Status::OK(); + } }; const AzureOptions& AzureFileSystem::options() const { return impl_->options(); } @@ -636,7 +740,12 @@ Result AzureFileSystem::GetFileInfo(const FileSelector& select) } Status AzureFileSystem::CreateDir(const std::string& path, bool recursive) { - return Status::NotImplemented("The Azure FileSystem is not fully implemented"); + ARROW_ASSIGN_OR_RAISE(auto p, AzurePath::FromString(path)); + if (recursive) { + return impl_->CreateDirRecursive(p); + } else { + return impl_->CreateDir(p); + } } Status AzureFileSystem::DeleteDir(const std::string& path) { diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index c08a4b50b77..673874eefbf 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -49,6 +49,7 @@ #include #include +#include "arrow/filesystem/path_util.h" #include "arrow/filesystem/test_util.h" #include "arrow/result.h" #include "arrow/testing/gtest_util.h" @@ -225,6 +226,10 @@ class AzureFileSystemTest : public ::testing::Test { return s; } + std::string RandomContainerName() { return RandomChars(32); } + + std::string RandomDirectoryName() { return RandomChars(32); } + void UploadLines(const std::vector& lines, const char* path_to_file, int total_size) { // TODO(GH-38333): Switch to using Azure filesystem to write once its implemented. @@ -396,6 +401,84 @@ TEST_F(AzureHierarchicalNamespaceFileSystemTest, GetFileInfoObject) { RunGetFileInfoObjectTest(); } +TEST_F(AzuriteFileSystemTest, CreateDirFailureNoContainer) { + ASSERT_RAISES(Invalid, fs_->CreateDir("", false)); +} + +TEST_F(AzuriteFileSystemTest, CreateDirSuccessContainerOnly) { + auto container_name = RandomContainerName(); + ASSERT_OK(fs_->CreateDir(container_name, false)); + arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); +} + +TEST_F(AzuriteFileSystemTest, CreateDirFailureContainerAndDirectory) { + const auto path = PreexistingContainerPath() + RandomDirectoryName(); + // Can't create a directory without hierarchical namespace support. + ASSERT_RAISES(NotImplemented, fs_->CreateDir(path, false)); +} + +TEST_F(AzureHierarchicalNamespaceFileSystemTest, CreateDirSuccessContainerAndDirectory) { + const auto path = PreexistingContainerPath() + RandomDirectoryName(); + ASSERT_OK(fs_->CreateDir(path, false)); + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::Directory); +} + +TEST_F(AzuriteFileSystemTest, CreateDirFailureDirectoryWithMissingContainer) { + const auto path = std::string("not-a-container/new-directory"); + ASSERT_RAISES(IOError, fs_->CreateDir(path, false)); +} + +TEST_F(AzuriteFileSystemTest, CreateDirRecursiveFailureNoContainer) { + ASSERT_RAISES(Invalid, fs_->CreateDir("", true)); +} + +TEST_F(AzureHierarchicalNamespaceFileSystemTest, CreateDirRecursiveSuccessContainerOnly) { + auto container_name = RandomContainerName(); + ASSERT_OK(fs_->CreateDir(container_name, true)); + arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); +} + +TEST_F(AzuriteFileSystemTest, CreateDirRecursiveFailureContainerOnly) { + auto container_name = RandomContainerName(); + ASSERT_RAISES(NotImplemented, fs_->CreateDir(container_name, true)); +} + +TEST_F(AzureHierarchicalNamespaceFileSystemTest, CreateDirRecursiveSuccessDirectoryOnly) { + const auto parent = PreexistingContainerPath() + RandomDirectoryName(); + const auto path = internal::ConcatAbstractPath(parent, "new-sub"); + ASSERT_OK(fs_->CreateDir(path, true)); + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::Directory); + arrow::fs::AssertFileInfo(fs_.get(), parent, FileType::Directory); +} + +TEST_F(AzuriteFileSystemTest, CreateDirRecursiveFailureDirectoryOnly) { + const auto parent = PreexistingContainerPath() + RandomDirectoryName(); + const auto path = internal::ConcatAbstractPath(parent, "new-sub"); + ASSERT_RAISES(NotImplemented, fs_->CreateDir(path, true)); +} + +TEST_F(AzureHierarchicalNamespaceFileSystemTest, + CreateDirRecursiveSuccessContainerAndDirectory) { + auto container_name = RandomContainerName(); + const auto parent = internal::ConcatAbstractPath(container_name, RandomDirectoryName()); + const auto path = internal::ConcatAbstractPath(parent, "new-sub"); + ASSERT_OK(fs_->CreateDir(path, true)); + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::Directory); + arrow::fs::AssertFileInfo(fs_.get(), parent, FileType::Directory); + arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); +} + +TEST_F(AzuriteFileSystemTest, CreateDirRecursiveFailureContainerAndDirectory) { + auto container_name = RandomContainerName(); + const auto parent = internal::ConcatAbstractPath(container_name, RandomDirectoryName()); + const auto path = internal::ConcatAbstractPath(parent, "new-sub"); + ASSERT_RAISES(NotImplemented, fs_->CreateDir(path, true)); +} + +TEST_F(AzuriteFileSystemTest, CreateDirUri) { + ASSERT_RAISES(Invalid, fs_->CreateDir("abfs://" + RandomContainerName(), true)); +} + TEST_F(AzuriteFileSystemTest, OpenInputStreamString) { std::shared_ptr stream; ASSERT_OK_AND_ASSIGN(stream, fs_->OpenInputStream(PreexistingObjectPath())); @@ -455,7 +538,7 @@ TEST_F(AzuriteFileSystemTest, OpenInputStreamInfoInvalid) { } TEST_F(AzuriteFileSystemTest, OpenInputStreamUri) { - ASSERT_RAISES(Invalid, fs_->OpenInputStream("abfss://" + PreexistingObjectPath())); + ASSERT_RAISES(Invalid, fs_->OpenInputStream("abfs://" + PreexistingObjectPath())); } TEST_F(AzuriteFileSystemTest, OpenInputStreamTrailingSlash) { From 5dcac692c8e47eaa720ffae5627f1ad2b88d6558 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 16 Nov 2023 12:27:45 +0900 Subject: [PATCH 2/7] Do nothing without hierarchical namespace support --- cpp/src/arrow/filesystem/azurefs.cc | 57 ++++++++++++++---------- cpp/src/arrow/filesystem/azurefs_test.cc | 30 +++++++++---- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 2dfe7cced41..acffbb0df83 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -148,6 +148,16 @@ Status ValidateFilePath(const AzurePath& path) { return Status::OK(); } +Status StatusFromErrorResponse(const std::string& url, + Azure::Core::Http::RawResponse* raw_response, + const std::string& context) { + const auto& body = raw_response->GetBody(); + std::string_view body_text(reinterpret_cast(body.data()), body.size()); + return Status::IOError(context, ": ", url, ": ", raw_response->GetReasonPhrase(), " (", + static_cast(raw_response->GetStatusCode()), + "): ", body_text); +} + template std::string FormatValue(typename TypeTraits::CType value) { struct StringAppender { @@ -625,19 +635,14 @@ class AzureFileSystem::Impl { if (response.Value.Created) { return Status::OK(); } else { - const auto& body = response.RawResponse->GetBody(); - std::string_view body_text(reinterpret_cast(body.data()), - body.size()); - return Status::IOError("Failed to create a container: ", path.container, " (", - container_client.GetUrl(), - "): ", response.RawResponse->GetReasonPhrase(), " (", - static_cast(response.RawResponse->GetStatusCode()), - "): ", body_text); + return StatusFromErrorResponse( + container_client.GetUrl(), response.RawResponse.get(), + "Failed to create a container: " + path.container); } } catch (const Azure::Storage::StorageException& exception) { return internal::ExceptionToStatus( - "Failed to create a container: " + path.container + " (" + - container_client.GetUrl() + ")", + "Failed to create a container: " + path.container + ": " + + container_client.GetUrl(), exception); } } @@ -645,9 +650,14 @@ class AzureFileSystem::Impl { ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled, hierarchical_namespace_.Enabled(path.container)); if (!hierarchical_namespace_enabled) { - return Status::NotImplemented( - "Cannot create a directory without hierarchical namespace: ", path.full_path); + // We can't create a directory without hierarchical namespace + // support. There is only "virtual directory" without + // hierarchical namespace support. And a "virtual directory" is + // (virtually) created a blob with ".../.../blob" blob name + // automatically. + return Status::OK(); } + auto directory_client = datalake_service_client_->GetFileSystemClient(path.container) .GetDirectoryClient(path.path_to_file); try { @@ -655,19 +665,14 @@ class AzureFileSystem::Impl { if (response.Value.Created) { return Status::OK(); } else { - const auto& body = response.RawResponse->GetBody(); - std::string_view body_text(reinterpret_cast(body.data()), - body.size()); - return Status::IOError("Failed to create a directory: ", path.path_to_file, " (", - directory_client.GetUrl(), - "): ", response.RawResponse->GetReasonPhrase(), " (", - static_cast(response.RawResponse->GetStatusCode()), - "): ", body_text); + return StatusFromErrorResponse( + directory_client.GetUrl(), response.RawResponse.get(), + "Failed to create a directory: " + path.path_to_file); } } catch (const Azure::Storage::StorageException& exception) { return internal::ExceptionToStatus( - "Failed to create a directory: " + path.path_to_file + " (" + - directory_client.GetUrl() + ")", + "Failed to create a directory: " + path.path_to_file + ": " + + directory_client.GetUrl(), exception); } } @@ -690,8 +695,12 @@ class AzureFileSystem::Impl { ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled, hierarchical_namespace_.Enabled(path.container)); if (!hierarchical_namespace_enabled) { - return Status::NotImplemented( - "Cannot create a directory without hierarchical namespace: ", path.full_path); + // We can't create a directory without hierarchical namespace + // support. There is only "virtual directory" without + // hierarchical namespace support. And a "virtual directory" is + // (virtually) created a blob with ".../.../blob" blob name + // automatically. + return Status::OK(); } std::string current_path; diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 673874eefbf..3214f81d138 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -411,10 +411,12 @@ TEST_F(AzuriteFileSystemTest, CreateDirSuccessContainerOnly) { arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); } -TEST_F(AzuriteFileSystemTest, CreateDirFailureContainerAndDirectory) { +TEST_F(AzuriteFileSystemTest, CreateDirSuccessContainerAndDirectory) { const auto path = PreexistingContainerPath() + RandomDirectoryName(); - // Can't create a directory without hierarchical namespace support. - ASSERT_RAISES(NotImplemented, fs_->CreateDir(path, false)); + ASSERT_OK(fs_->CreateDir(path, false)); + // There is only virtual directory without hierarchical namespace + // support. So the CreateDir() does nothing. + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::NotFound); } TEST_F(AzureHierarchicalNamespaceFileSystemTest, CreateDirSuccessContainerAndDirectory) { @@ -438,9 +440,10 @@ TEST_F(AzureHierarchicalNamespaceFileSystemTest, CreateDirRecursiveSuccessContai arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); } -TEST_F(AzuriteFileSystemTest, CreateDirRecursiveFailureContainerOnly) { +TEST_F(AzuriteFileSystemTest, CreateDirRecursiveSuccessContainerOnly) { auto container_name = RandomContainerName(); - ASSERT_RAISES(NotImplemented, fs_->CreateDir(container_name, true)); + ASSERT_OK(fs_->CreateDir(container_name, true)); + arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); } TEST_F(AzureHierarchicalNamespaceFileSystemTest, CreateDirRecursiveSuccessDirectoryOnly) { @@ -451,10 +454,14 @@ TEST_F(AzureHierarchicalNamespaceFileSystemTest, CreateDirRecursiveSuccessDirect arrow::fs::AssertFileInfo(fs_.get(), parent, FileType::Directory); } -TEST_F(AzuriteFileSystemTest, CreateDirRecursiveFailureDirectoryOnly) { +TEST_F(AzuriteFileSystemTest, CreateDirRecursiveSuccessDirectoryOnly) { const auto parent = PreexistingContainerPath() + RandomDirectoryName(); const auto path = internal::ConcatAbstractPath(parent, "new-sub"); - ASSERT_RAISES(NotImplemented, fs_->CreateDir(path, true)); + ASSERT_OK(fs_->CreateDir(path, true)); + // There is only virtual directory without hierarchical namespace + // support. So the CreateDir() does nothing. + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::NotFound); + arrow::fs::AssertFileInfo(fs_.get(), parent, FileType::NotFound); } TEST_F(AzureHierarchicalNamespaceFileSystemTest, @@ -468,11 +475,16 @@ TEST_F(AzureHierarchicalNamespaceFileSystemTest, arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); } -TEST_F(AzuriteFileSystemTest, CreateDirRecursiveFailureContainerAndDirectory) { +TEST_F(AzuriteFileSystemTest, CreateDirRecursiveSuccessContainerAndDirectory) { auto container_name = RandomContainerName(); const auto parent = internal::ConcatAbstractPath(container_name, RandomDirectoryName()); const auto path = internal::ConcatAbstractPath(parent, "new-sub"); - ASSERT_RAISES(NotImplemented, fs_->CreateDir(path, true)); + ASSERT_OK(fs_->CreateDir(path, true)); + // There is only virtual directory without hierarchical namespace + // support. So the CreateDir() does nothing. + arrow::fs::AssertFileInfo(fs_.get(), path, FileType::NotFound); + arrow::fs::AssertFileInfo(fs_.get(), parent, FileType::NotFound); + arrow::fs::AssertFileInfo(fs_.get(), container_name, FileType::Directory); } TEST_F(AzuriteFileSystemTest, CreateDirUri) { From aabc7b2c6ff5c171495fabffcc979750fecb87f8 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 16 Nov 2023 14:21:38 +0900 Subject: [PATCH 3/7] Add a comment how to enable tests with hierarchical namespace --- cpp/src/arrow/filesystem/azurefs_test.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 3214f81d138..ecf0a19f684 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -272,6 +272,22 @@ class AzureFlatNamespaceFileSystemTest : public AzureFileSystemTest { } }; +// How to enable this test: +// +// You need an Azure account. You should be able to create a free +// account at https://azure.microsoft.com/en-gb/free/ . You should be +// able to create a storage account through the portal Web UI. +// +// See also the official document how to create a storage account: +// https://learn.microsoft.com/en-us/azure/storage/blobs/create-data-lake-storage-account +// +// A few suggestions on configuration: +// +// * Use Standard general-purpose v2 not premium +// * Use LRS redundancy +// * Obviously you need to enable hierarchical namespace. +// * Set the default access tier to hot +// * SFTP, NFS and file shares are not required. class AzureHierarchicalNamespaceFileSystemTest : public AzureFileSystemTest { Result MakeOptions() override { AzureOptions options; From 6a16e9f3232461c25e289cea16b3529876970386 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 17 Nov 2023 12:46:09 +0900 Subject: [PATCH 4/7] Improve description about virtual directory Co-authored-by: Thomas Newton --- cpp/src/arrow/filesystem/azurefs.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index acffbb0df83..9c2a009317b 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -650,11 +650,9 @@ class AzureFileSystem::Impl { ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled, hierarchical_namespace_.Enabled(path.container)); if (!hierarchical_namespace_enabled) { - // We can't create a directory without hierarchical namespace - // support. There is only "virtual directory" without - // hierarchical namespace support. And a "virtual directory" is - // (virtually) created a blob with ".../.../blob" blob name - // automatically. + // Without hierarchical namespace enabled Azure blob storage has no directories. + // Therefore we can't, and don't need to create one. Simply creating a blob with `/` in the name + // implies directories. return Status::OK(); } From 4d312adaea0267cf2f96fca7734e115a3a7529a9 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 17 Nov 2023 12:58:37 +0900 Subject: [PATCH 5/7] Add a comment about response body --- cpp/src/arrow/filesystem/azurefs.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 9c2a009317b..8f33fe3387b 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -152,6 +152,9 @@ Status StatusFromErrorResponse(const std::string& url, Azure::Core::Http::RawResponse* raw_response, const std::string& context) { const auto& body = raw_response->GetBody(); + // There isn't an Azure specification that response body on error + // doesn't contain any binary data but we assume it. We hope that + // error response body has useful information for the error. std::string_view body_text(reinterpret_cast(body.data()), body.size()); return Status::IOError(context, ": ", url, ": ", raw_response->GetReasonPhrase(), " (", static_cast(raw_response->GetStatusCode()), From 60890c6363f7513eb52e8afb2fbeeb8e021ccca5 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 17 Nov 2023 12:58:58 +0900 Subject: [PATCH 6/7] Format --- cpp/src/arrow/filesystem/azurefs.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 8f33fe3387b..447496cdb8d 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -653,9 +653,9 @@ class AzureFileSystem::Impl { ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled, hierarchical_namespace_.Enabled(path.container)); if (!hierarchical_namespace_enabled) { - // Without hierarchical namespace enabled Azure blob storage has no directories. - // Therefore we can't, and don't need to create one. Simply creating a blob with `/` in the name - // implies directories. + // Without hierarchical namespace enabled Azure blob storage has no directories. + // Therefore we can't, and don't need to create one. Simply creating a blob with `/` + // in the name implies directories. return Status::OK(); } From 9dbc1c95d63614744c384cce17f3aa86b09f47e5 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 17 Nov 2023 13:02:49 +0900 Subject: [PATCH 7/7] Simplify --- cpp/src/arrow/filesystem/azurefs.cc | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 447496cdb8d..fdf119477ab 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -704,23 +704,15 @@ class AzureFileSystem::Impl { return Status::OK(); } - std::string current_path; - for (const auto& part : path.path_to_file_parts) { - if (!current_path.empty()) { - current_path += internal::kSep; - } - current_path += part; - auto directory_client = - datalake_service_client_->GetFileSystemClient(path.container) - .GetDirectoryClient(current_path); - try { - directory_client.CreateIfNotExists(); - } catch (const Azure::Storage::StorageException& exception) { - return internal::ExceptionToStatus( - "Failed to create a directory: " + current_path + " (" + - directory_client.GetUrl() + ")", - exception); - } + auto directory_client = datalake_service_client_->GetFileSystemClient(path.container) + .GetDirectoryClient(path.path_to_file); + try { + directory_client.CreateIfNotExists(); + } catch (const Azure::Storage::StorageException& exception) { + return internal::ExceptionToStatus( + "Failed to create a directory: " + path.path_to_file + " (" + + directory_client.GetUrl() + ")", + exception); } return Status::OK();