diff --git a/src/Core/SettingsEnums.cpp b/src/Core/SettingsEnums.cpp index 8f7ff6f149c..01b9229ebb7 100644 --- a/src/Core/SettingsEnums.cpp +++ b/src/Core/SettingsEnums.cpp @@ -283,11 +283,4 @@ IMPLEMENT_SETTING_ENUM( IMPLEMENT_SETTING_ENUM(DatabaseIcebergCatalogType, ErrorCodes::BAD_ARGUMENTS, {{"rest", DatabaseIcebergCatalogType::REST}}) -IMPLEMENT_SETTING_ENUM(DatabaseIcebergStorageType, ErrorCodes::BAD_ARGUMENTS, - {{"s3", DatabaseIcebergStorageType::S3}, - {"azure", DatabaseIcebergStorageType::Azure}, - {"hdfs", DatabaseIcebergStorageType::HDFS}, - {"local", DatabaseIcebergStorageType::Local}, - }) - } diff --git a/src/Core/SettingsEnums.h b/src/Core/SettingsEnums.h index 33ef8e9623f..aefbc45d411 100644 --- a/src/Core/SettingsEnums.h +++ b/src/Core/SettingsEnums.h @@ -366,14 +366,4 @@ enum class DatabaseIcebergCatalogType : uint8_t DECLARE_SETTING_ENUM(DatabaseIcebergCatalogType) -enum class DatabaseIcebergStorageType : uint8_t -{ - S3, - Azure, - Local, - HDFS, -}; - -DECLARE_SETTING_ENUM(DatabaseIcebergStorageType) - } diff --git a/src/Databases/Iceberg/DatabaseIceberg.cpp b/src/Databases/Iceberg/DatabaseIceberg.cpp index 8544239d6b1..62af8288933 100644 --- a/src/Databases/Iceberg/DatabaseIceberg.cpp +++ b/src/Databases/Iceberg/DatabaseIceberg.cpp @@ -30,7 +30,6 @@ namespace DB namespace DatabaseIcebergSetting { extern const DatabaseIcebergSettingsDatabaseIcebergCatalogType catalog_type; - extern const DatabaseIcebergSettingsDatabaseIcebergStorageType storage_type; extern const DatabaseIcebergSettingsString warehouse; extern const DatabaseIcebergSettingsString catalog_credential; extern const DatabaseIcebergSettingsString auth_header; @@ -72,18 +71,17 @@ DatabaseIceberg::DatabaseIceberg( const std::string & database_name_, const std::string & url_, const DatabaseIcebergSettings & settings_, - ASTPtr database_engine_definition_, - ContextPtr context_) + ASTPtr database_engine_definition_) : IDatabase(database_name_) , url(url_) , settings(settings_) , database_engine_definition(database_engine_definition_) , log(getLogger("DatabaseIceberg(" + database_name_ + ")")) { - validateSettings(context_); + validateSettings(); } -void DatabaseIceberg::validateSettings(const ContextPtr & context_) +void DatabaseIceberg::validateSettings() { if (settings[DatabaseIcebergSetting::warehouse].value.empty()) { @@ -91,18 +89,6 @@ void DatabaseIceberg::validateSettings(const ContextPtr & context_) ErrorCodes::BAD_ARGUMENTS, "`warehouse` setting cannot be empty. " "Please specify 'SETTINGS warehouse=' in the CREATE DATABASE query"); } - - if (!settings[DatabaseIcebergSetting::storage_type].changed) - { - auto catalog = getCatalog(context_); - const auto storage_type = catalog->getStorageType(); - if (!storage_type) - { - throw Exception( - ErrorCodes::BAD_ARGUMENTS, "Storage type is not found in catalog config. " - "Please specify it manually via 'SETTINGS storage_type=' in CREATE DATABASE query"); - } - } } std::shared_ptr DatabaseIceberg::getCatalog(ContextPtr) const @@ -127,11 +113,11 @@ std::shared_ptr DatabaseIceberg::getCatalog(ContextPtr) const return catalog_impl; } -std::shared_ptr DatabaseIceberg::getConfiguration() const +std::shared_ptr DatabaseIceberg::getConfiguration(DatabaseIcebergStorageType type) const { /// TODO: add tests for azure, local storage types. - switch (settings[DatabaseIcebergSetting::storage_type].value) + switch (type) { #if USE_AWS_S3 case DB::DatabaseIcebergStorageType::S3: @@ -234,7 +220,15 @@ StoragePtr DatabaseIceberg::tryGetTable(const String & name, ContextPtr context_ LOG_TEST(log, "Using table endpoint: {}", table_endpoint); const auto columns = ColumnsDescription(table_metadata.getSchema()); - const auto configuration = getConfiguration(); + + DatabaseIcebergStorageType storage_type; + auto storage_type_from_catalog = catalog->getStorageType(); + if (storage_type_from_catalog.has_value()) + storage_type = storage_type_from_catalog.value(); + else + storage_type = table_metadata.getStorageType(); + + const auto configuration = getConfiguration(storage_type); /// with_table_structure = false: because there will be /// no table structure in table definition AST. @@ -382,8 +376,7 @@ void registerDatabaseIceberg(DatabaseFactory & factory) args.database_name, url, database_settings, - database_engine_define->clone(), - args.context); + database_engine_define->clone()); }; factory.registerDatabase("Iceberg", create_fn, { .supports_arguments = true, .supports_settings = true }); } diff --git a/src/Databases/Iceberg/DatabaseIceberg.h b/src/Databases/Iceberg/DatabaseIceberg.h index 4b9929a3a76..5138d0f639c 100644 --- a/src/Databases/Iceberg/DatabaseIceberg.h +++ b/src/Databases/Iceberg/DatabaseIceberg.h @@ -18,8 +18,7 @@ public: const std::string & database_name_, const std::string & url_, const DatabaseIcebergSettings & settings_, - ASTPtr database_engine_definition_, - ContextPtr context_); + ASTPtr database_engine_definition_); String getEngineName() const override { return "Iceberg"; } @@ -57,9 +56,9 @@ private: mutable std::shared_ptr catalog_impl; - void validateSettings(const ContextPtr & context_); + void validateSettings(); std::shared_ptr getCatalog(ContextPtr context_) const; - std::shared_ptr getConfiguration() const; + std::shared_ptr getConfiguration(DatabaseIcebergStorageType type) const; std::string getStorageEndpointForTable(const Iceberg::TableMetadata & table_metadata) const; }; diff --git a/src/Databases/Iceberg/DatabaseIcebergSettings.cpp b/src/Databases/Iceberg/DatabaseIcebergSettings.cpp index 33374edbb6d..37b4909106b 100644 --- a/src/Databases/Iceberg/DatabaseIcebergSettings.cpp +++ b/src/Databases/Iceberg/DatabaseIcebergSettings.cpp @@ -16,7 +16,6 @@ namespace ErrorCodes #define DATABASE_ICEBERG_RELATED_SETTINGS(DECLARE, ALIAS) \ DECLARE(DatabaseIcebergCatalogType, catalog_type, DatabaseIcebergCatalogType::REST, "Catalog type", 0) \ - DECLARE(DatabaseIcebergStorageType, storage_type, DatabaseIcebergStorageType::S3, "Storage type: S3, Local, Azure, HDFS", 0) \ DECLARE(String, catalog_credential, "", "", 0) \ DECLARE(Bool, vended_credentials, true, "Use vended credentials (storage credentials) from catalog", 0) \ DECLARE(String, auth_scope, "PRINCIPAL_ROLE:ALL", "Authorization scope for client credentials or token exchange", 0) \ diff --git a/src/Databases/Iceberg/DatabaseIcebergSettings.h b/src/Databases/Iceberg/DatabaseIcebergSettings.h index 4e5bc0defba..041a99c6d83 100644 --- a/src/Databases/Iceberg/DatabaseIcebergSettings.h +++ b/src/Databases/Iceberg/DatabaseIcebergSettings.h @@ -18,7 +18,6 @@ class SettingsChanges; M(CLASS_NAME, UInt64) \ M(CLASS_NAME, Bool) \ M(CLASS_NAME, DatabaseIcebergCatalogType) \ - M(CLASS_NAME, DatabaseIcebergStorageType) DATABASE_ICEBERG_SETTINGS_SUPPORTED_TYPES(DatabaseIcebergSettings, DECLARE_SETTING_TRAIT) diff --git a/src/Databases/Iceberg/DatabaseIcebergStorageType.h b/src/Databases/Iceberg/DatabaseIcebergStorageType.h new file mode 100644 index 00000000000..cc3c8f8cb1d --- /dev/null +++ b/src/Databases/Iceberg/DatabaseIcebergStorageType.h @@ -0,0 +1,14 @@ +#include + +namespace DB +{ + +enum class DatabaseIcebergStorageType : uint8_t +{ + S3, + Azure, + Local, + HDFS, +}; + +} diff --git a/src/Databases/Iceberg/ICatalog.cpp b/src/Databases/Iceberg/ICatalog.cpp index 97b2bfeeef9..4568cf95ac0 100644 --- a/src/Databases/Iceberg/ICatalog.cpp +++ b/src/Databases/Iceberg/ICatalog.cpp @@ -12,6 +12,32 @@ namespace DB::ErrorCodes namespace Iceberg { +StorageType parseStorageTypeFromLocation(const std::string & location) +{ + /// Table location in catalog metadata always starts with one of s3://, file://, etc. + /// So just extract this part of the path and deduce storage type from it. + + auto pos = location.find("://"); + if (pos == std::string::npos) + { + throw DB::Exception( + DB::ErrorCodes::NOT_IMPLEMENTED, + "Unexpected path format: {}", location); + } + + auto storage_type_str = location.substr(0, pos); + auto storage_type = magic_enum::enum_cast(Poco::toUpper(storage_type_str)); + + if (!storage_type) + { + throw DB::Exception( + DB::ErrorCodes::NOT_IMPLEMENTED, + "Unsupported storage type: {}", storage_type_str); + } + + return *storage_type; +} + void TableMetadata::setLocation(const std::string & location_) { if (!with_location) @@ -83,4 +109,10 @@ std::shared_ptr TableMetadata::getStorageCredentials() cons return storage_credentials; } + +StorageType TableMetadata::getStorageType() const +{ + return parseStorageTypeFromLocation(location_without_path); +} + } diff --git a/src/Databases/Iceberg/ICatalog.h b/src/Databases/Iceberg/ICatalog.h index 9657ef6ba41..45d9f056477 100644 --- a/src/Databases/Iceberg/ICatalog.h +++ b/src/Databases/Iceberg/ICatalog.h @@ -3,10 +3,12 @@ #include #include #include +#include namespace Iceberg { using StorageType = DB::DatabaseIcebergStorageType; +StorageType parseStorageTypeFromLocation(const std::string & location); /// A class representing table metadata, /// which was received from Catalog. @@ -32,6 +34,8 @@ public: bool requiresSchema() const { return with_schema; } bool requiresCredentials() const { return with_storage_credentials; } + StorageType getStorageType() const; + private: /// Starts with s3://, file://, etc. /// For example, `s3://bucket/` diff --git a/src/Databases/Iceberg/RestCatalog.cpp b/src/Databases/Iceberg/RestCatalog.cpp index b138cc1c14b..460fce78696 100644 --- a/src/Databases/Iceberg/RestCatalog.cpp +++ b/src/Databases/Iceberg/RestCatalog.cpp @@ -75,32 +75,6 @@ DB::HTTPHeaderEntry parseAuthHeader(const std::string & auth_header) return DB::HTTPHeaderEntry(auth_header.substr(0, pos), auth_header.substr(pos + 1)); } -StorageType parseStorageTypeFromLocation(const std::string & location) -{ - /// Table location in catalog metadata always starts with one of s3://, file://, etc. - /// So just extract this part of the path and deduce storage type from it. - - auto pos = location.find("://"); - if (pos == std::string::npos) - { - throw DB::Exception( - DB::ErrorCodes::NOT_IMPLEMENTED, - "Unexpected path format: {}", location); - } - - auto storage_type_str = location.substr(0, pos); - auto storage_type = magic_enum::enum_cast(Poco::toUpper(storage_type_str)); - - if (!storage_type) - { - throw DB::Exception( - DB::ErrorCodes::NOT_IMPLEMENTED, - "Unsupported storage type: {}", storage_type_str); - } - - return *storage_type; -} - std::string correctAPIURI(const std::string & uri) { if (uri.ends_with("v1"))