Remove useless setting

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},
-                   })
-
 }

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)
-
 }

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_,
+    ASTPtr database_engine_definition_)
-    ContextPtr context_)
     : 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=<warehouse_name>' 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=<type>' in CREATE DATABASE query");
-        }
-    }
 }
 
 std::shared_ptr<Iceberg::ICatalog> DatabaseIceberg::getCatalog(ContextPtr) const
@@ -127,11 +113,11 @@ std::shared_ptr<Iceberg::ICatalog> DatabaseIceberg::getCatalog(ContextPtr) const
     return catalog_impl;
 }
 
-std::shared_ptr<StorageObjectStorage::Configuration> DatabaseIceberg::getConfiguration() const
+std::shared_ptr<StorageObjectStorage::Configuration> 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(),
+            database_engine_define->clone());
-            args.context);
     };
     factory.registerDatabase("Iceberg", create_fn, { .supports_arguments = true, .supports_settings = true });
 }

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_,
+        ASTPtr database_engine_definition_);
-        ContextPtr context_);
 
     String getEngineName() const override { return "Iceberg"; }
 
@@ -57,9 +56,9 @@ private:
 
     mutable std::shared_ptr<Iceberg::ICatalog> catalog_impl;
 
-    void validateSettings(const ContextPtr & context_);
+    void validateSettings();
     std::shared_ptr<Iceberg::ICatalog> getCatalog(ContextPtr context_) const;
-    std::shared_ptr<StorageObjectStorage::Configuration> getConfiguration() const;
+    std::shared_ptr<StorageObjectStorage::Configuration> getConfiguration(DatabaseIcebergStorageType type) const;
     std::string getStorageEndpointForTable(const Iceberg::TableMetadata & table_metadata) const;
 };
 

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)             \

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)
 

src/Databases/Iceberg/DatabaseIcebergStorageType.h

@@ -0,0 +1,14 @@
+#include <Core/Types.h>
+
+namespace DB
+{
+
+enum class DatabaseIcebergStorageType : uint8_t
+{
+    S3,
+    Azure,
+    Local,
+    HDFS,
+};
+
+}

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<StorageType>(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<IStorageCredentials> TableMetadata::getStorageCredentials() cons
 
     return storage_credentials;
 }
+
+StorageType TableMetadata::getStorageType() const
+{
+    return parseStorageTypeFromLocation(location_without_path);
+}
+
 }

src/Databases/Iceberg/ICatalog.h

@@ -3,10 +3,12 @@
 #include <Core/NamesAndTypes.h>
 #include <Core/SettingsEnums.h>
 #include <Databases/Iceberg/StorageCredentials.h>
+#include <Databases/Iceberg/DatabaseIcebergStorageType.h>
 
 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/`

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<StorageType>(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"))