diff --git a/src/Client/BuzzHouse/Generator/SessionSettings.cpp b/src/Client/BuzzHouse/Generator/SessionSettings.cpp index cdc597a90780..19dc85643cf9 100644 --- a/src/Client/BuzzHouse/Generator/SessionSettings.cpp +++ b/src/Client/BuzzHouse/Generator/SessionSettings.cpp @@ -873,7 +873,7 @@ std::unordered_map serverSettings = { [](RandomGenerator & rg, FuzzConfig &) { return std::to_string(rg.thresholdGenerator(0.3, 0.2, 0, 10800)); }, {}, false)}, - {"iceberg_delete_data_on_drop", trueOrFalseSettingNoOracle}, + {"data_lake_delete_data_on_drop", trueOrFalseSettingNoOracle}, {"iceberg_expire_default_min_snapshots_to_keep", CHSetting( [](RandomGenerator & rg, FuzzConfig &) { return std::to_string(rg.thresholdGenerator(0.2, 0.2, 0, 10)); }, {}, false)}, diff --git a/src/Core/Settings.cpp b/src/Core/Settings.cpp index 5d0f3cc34cc2..4b7c085db584 100644 --- a/src/Core/Settings.cpp +++ b/src/Core/Settings.cpp @@ -5551,9 +5551,9 @@ Possible values: - manifest_file_entry - Everything above + traversed avro manifest files entries. )", 0) \ \ - DECLARE(Bool, iceberg_delete_data_on_drop, false, R"( -Whether to delete all iceberg files on drop or not. -)", 0) \ + DECLARE_WITH_ALIAS(Bool, data_lake_delete_data_on_drop, false, R"( +Whether to delete the underlying data files when dropping a data lake table. For catalog databases the catalog is asked to purge the data (`purgeRequested=true`); for self-managed tables ClickHouse removes the files directly. +)", 0, iceberg_delete_data_on_drop) \ DECLARE(Int64, iceberg_expire_default_min_snapshots_to_keep, 1, R"( Default value for Iceberg table property `history.expire.min-snapshots-to-keep` used by `expire_snapshots` when that property is absent. )", 0) \ diff --git a/src/Core/SettingsChangesHistory.cpp b/src/Core/SettingsChangesHistory.cpp index 008ba54f43d2..9a61ec6784ee 100644 --- a/src/Core/SettingsChangesHistory.cpp +++ b/src/Core/SettingsChangesHistory.cpp @@ -42,6 +42,7 @@ const VersionToSettingsChangesMap & getSettingsChangesHistory() addSettingsChanges(settings_changes_history, "26.6.2.20001.altinityantalya", { {"use_puffin_files_cache", false, true, "Enables cache of parsed Puffin file content such as deletion vectors."}, + {"data_lake_delete_data_on_drop", false, false, "New setting that unifies dropping of data lake data; the released `iceberg_delete_data_on_drop` is kept as an alias for it."}, }); addSettingsChanges(settings_changes_history, "26.6", diff --git a/src/Databases/DataLake/Common.cpp b/src/Databases/DataLake/Common.cpp index 8946d3412d70..220df15010d4 100644 --- a/src/Databases/DataLake/Common.cpp +++ b/src/Databases/DataLake/Common.cpp @@ -16,6 +16,9 @@ #include +#include +#include + namespace DB::ErrorCodes { extern const int BAD_ARGUMENTS; @@ -121,4 +124,91 @@ std::pair parseTableName(const std::string & name) return {namespace_name, table_name}; } +String constructTableLocation( + const String & location_scheme, + const String & storage_endpoint, + const String & namespace_name, + const String & table_name, + DB::S3UriStyle uri_style) +{ + Poco::URI uri(storage_endpoint); + auto path = uri.getPath(); + while (path.starts_with('/')) + path.erase(0, 1); + while (path.ends_with('/')) + path.pop_back(); + + if (location_scheme == "abfss") + { + /// Azure: `abfss://@/`. `storage_endpoint` is + /// `https:////` or `abfss://@/` + String container = uri.getUserInfo(); + String account_host = uri.getHost(); + String extra_path = path; + + if (container.empty()) + { + auto first_slash = extra_path.find('/'); + if (first_slash == String::npos) + { + container = std::move(extra_path); + extra_path.clear(); + } + else + { + container = extra_path.substr(0, first_slash); + extra_path = extra_path.substr(first_slash + 1); + } + } + + if (account_host.empty() || container.empty()) + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, + "`storage_endpoint` ({}) for Azure must include both account host and container " + "(expected https://.dfs.core.windows.net/[/] or " + "abfss://@.dfs.core.windows.net[/])", + storage_endpoint); + + if (extra_path.empty()) + return fmt::format("abfss://{}@{}/{}/{}", container, account_host, namespace_name, table_name); + return fmt::format("abfss://{}@{}/{}/{}/{}", container, account_host, extra_path, namespace_name, table_name); + } + + if (location_scheme == "s3") + { + if (uri_style == DB::S3UriStyle::VIRTUAL_HOSTED) + /// A virtual-hosted host cannot be split into bucket and service unambiguously + /// (`s3.us-east-1.amazonaws.com`, an IP host and a dotted bucket name all look alike). + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, + "CREATE TABLE with `storage_uri_style = 'virtual_hosted'` cannot derive the bucket from " + "`storage_endpoint` ({}); set `default_base_location` (a full s3:///) instead.", + storage_endpoint); + + if (path.empty()) + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, + "`storage_endpoint` ({}) does not contain a bucket; " + "CREATE TABLE in DataLakeCatalog requires `storage_endpoint` to include a non-empty bucket path.", + storage_endpoint); + return fmt::format("s3://{}/{}/{}", path, namespace_name, table_name); + } + + /// HDFS / file / other schemes that may have `authority`. + String authority = uri.getAuthority(); + if (authority.empty()) + { + if (path.empty()) + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, + "`storage_endpoint` ({}) does not contain a path", + storage_endpoint); + return fmt::format("{}:///{}/{}/{}", location_scheme, path, namespace_name, table_name); + } + + if (path.empty()) + return fmt::format("{}://{}/{}/{}", location_scheme, authority, namespace_name, table_name); + return fmt::format("{}://{}/{}/{}/{}", location_scheme, authority, path, namespace_name, table_name); +} + } diff --git a/src/Databases/DataLake/Common.h b/src/Databases/DataLake/Common.h index 9b0dd7c626a6..7f6d2bc781d6 100644 --- a/src/Databases/DataLake/Common.h +++ b/src/Databases/DataLake/Common.h @@ -1,12 +1,39 @@ #pragma once #include +#include #include #include +#include + +namespace DB::ErrorCodes +{ +extern const int TABLE_ALREADY_EXISTS; +} namespace DataLake { +/// Thrown by a `CREATE TABLE` that registered nothing in the data lake catalog: the table name, or the +/// location the table would use, is already taken. `InterpreterCreateQuery` matches on this type rather +/// than on its `TABLE_ALREADY_EXISTS` code, which would also swallow unrelated exceptions from below. +class TableAlreadyExistsInCatalogException : public DB::Exception +{ +public: + template + explicit TableAlreadyExistsInCatalogException(FormatStringHelper fmt, Args &&... args) + : DB::Exception(DB::ErrorCodes::TABLE_ALREADY_EXISTS, std::move(fmt), std::forward(args)...) + { + } + + TableAlreadyExistsInCatalogException * clone() const override { return new TableAlreadyExistsInCatalogException(*this); } + void rethrow() const override { throw *this; } /// NOLINT(bugprone-exception-copy-constructor-throws,cert-err60-cpp) + +private: + const char * name() const noexcept override { return "DataLake::TableAlreadyExistsInCatalogException"; } + const char * className() const noexcept override { return "DataLake::TableAlreadyExistsInCatalogException"; } +}; + String trim(const String & str); std::vector splitTypeArguments(const String & type_str); @@ -19,4 +46,11 @@ DB::DataTypePtr getType(const String & type_name, bool nullable, DB::ContextPtr /// `E` is a table name. std::pair parseTableName(const std::string & name); +String constructTableLocation( + const String & location_scheme, + const String & storage_endpoint, + const String & namespace_name, + const String & table_name, + DB::S3UriStyle uri_style = DB::S3UriStyle::AUTO); + } diff --git a/src/Databases/DataLake/DatabaseDataLake.cpp b/src/Databases/DataLake/DatabaseDataLake.cpp index ba2fc89c7799..6a44e3d1a103 100644 --- a/src/Databases/DataLake/DatabaseDataLake.cpp +++ b/src/Databases/DataLake/DatabaseDataLake.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #if USE_AVRO && USE_PARQUET @@ -44,14 +45,19 @@ #include #include +#include #include #include #include #include #include +#include +#include +#include #include #include +#include namespace DB { @@ -63,6 +69,7 @@ namespace DatabaseDataLakeSetting extern const DatabaseDataLakeSettingsString auth_header; extern const DatabaseDataLakeSettingsString auth_scope; extern const DatabaseDataLakeSettingsString storage_endpoint; + extern const DatabaseDataLakeSettingsString default_base_location; extern const DatabaseDataLakeSettingsS3UriStyle storage_uri_style; extern const DatabaseDataLakeSettingsString oauth_server_uri; extern const DatabaseDataLakeSettingsBool oauth_server_use_request_body; @@ -107,7 +114,8 @@ namespace Setting extern const SettingsBool parallel_replicas_for_cluster_engines; extern const SettingsString cluster_for_parallel_replicas; extern const SettingsBool database_datalake_require_metadata_access; - + extern const SettingsBool data_lake_delete_data_on_drop; + extern const SettingsString iceberg_metadata_compression_method; } namespace DataLakeStorageSetting @@ -131,6 +139,61 @@ namespace FailPoints extern const char datalake_try_get_table_return_nullptr[]; } +namespace +{ + +String getLocationSchemeForTableCreation(const std::shared_ptr & catalog) +{ + if (auto storage_type = catalog->getStorageType(); storage_type.has_value()) + return DataLake::storageTypeToScheme(*storage_type); + + /// Fall back only for catalogs whose backing storage is fixed. + /// REST/Hive/Glue/Paimon/Unity can be backed by anything, so we refuse to guess. + switch (catalog->getCatalogType()) + { + case DatabaseDataLakeCatalogType::ICEBERG_ONELAKE: + return "abfss"; /// Azure-only + case DatabaseDataLakeCatalogType::ICEBERG_BIGLAKE: + return "s3"; /// GCS via S3 API + case DatabaseDataLakeCatalogType::ICEBERG_REST: + case DatabaseDataLakeCatalogType::S3_TABLES: + case DatabaseDataLakeCatalogType::ICEBERG_HIVE: + case DatabaseDataLakeCatalogType::GLUE: + case DatabaseDataLakeCatalogType::PAIMON_REST: + case DatabaseDataLakeCatalogType::UNITY: + case DatabaseDataLakeCatalogType::NONE: + throw Exception( + ErrorCodes::BAD_ARGUMENTS, + "Cannot determine storage scheme for CREATE TABLE for catalog type '{}': the catalog does not " + "report a backing storage type. Set `default_base_location` on the database or configure " + "the catalog to expose `default-base-location`.", + catalog->getCatalogType()); + } + + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unexpected catalog type in CREATE TABLE location scheme resolution"); +} + +/// The storage backend a catalog is pinned to when creating a table: what the catalog reports, or the +/// backend `getLocationSchemeForTableCreation` falls back to for services whose storage is fixed. +/// A `OneLake` catalog that does not expose `default-base-location` is still Azure-only. +std::optional getFixedStorageTypeForTableCreation(const std::shared_ptr & catalog) +{ + if (auto storage_type = catalog->getStorageType(); storage_type.has_value()) + return storage_type; + + switch (catalog->getCatalogType()) + { + case DatabaseDataLakeCatalogType::ICEBERG_ONELAKE: + return DatabaseDataLakeStorageType::Azure; + case DatabaseDataLakeCatalogType::ICEBERG_BIGLAKE: + return DatabaseDataLakeStorageType::S3; /// GCS via the S3 API + default: + return {}; + } +} + +} + DatabaseDataLake::DatabaseDataLake( const std::string & database_name_, const std::string & url_, @@ -678,9 +741,8 @@ StoragePtr DatabaseDataLake::tryGetTableImpl(const String & name, ContextPtr con if (!metadata_location.empty()) { metadata_location = table_metadata.getMetadataLocation(metadata_location); + (*storage_settings)[DB::DataLakeStorageSetting::iceberg_metadata_file_path] = metadata_location; } - - (*storage_settings)[DB::DataLakeStorageSetting::iceberg_metadata_file_path] = metadata_location; } const auto configuration = getConfiguration(storage_type, storage_settings); @@ -779,16 +841,191 @@ StoragePtr DatabaseDataLake::tryGetTableImpl(const String & name, ContextPtr con return storage_cluster; } -void DatabaseDataLake::dropTable( /// NOLINT +void DatabaseDataLake::validateCreateTableEngine(const String & engine_name) const +{ + /// `Iceberg` picks its backend from the optional `disk` setting, which the storage factory resolves + /// only after this database-level validation, so a fixed-backend catalog cannot accept it here. + if (engine_name == "Iceberg" && getFixedStorageTypeForTableCreation(getCatalog()).has_value()) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "The generic 'Iceberg' engine is not supported for a DataLakeCatalog with a fixed storage backend. " + "Use the matching backend-specific Iceberg engine instead"); + + /// Unrecognized names pin no backend and are accepted here; they are rejected by the storage factory. + std::optional engine_backend; + if (engine_name == "IcebergS3") + engine_backend = DatabaseDataLakeStorageType::S3; + else if (engine_name == "IcebergAzure") + engine_backend = DatabaseDataLakeStorageType::Azure; + else if (engine_name == "IcebergHDFS") + engine_backend = DatabaseDataLakeStorageType::HDFS; + else if (engine_name == "IcebergLocal") + engine_backend = DatabaseDataLakeStorageType::Local; + + if (!engine_backend.has_value()) + return; + + /// A catalog without a fixed backend reopens the table using its own location, so any backend fits. + auto catalog_storage_type = getFixedStorageTypeForTableCreation(getCatalog()); + if (!catalog_storage_type.has_value() || *catalog_storage_type == *engine_backend) + return; + + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Table engine '{}' uses the {} storage backend, but this DataLakeCatalog stores tables on {}. " + "The table would be reopened with the catalog's storage backend and become unreadable " + "immediately after creation. Use a matching Iceberg engine or the generic 'Iceberg' engine", + engine_name, *engine_backend, *catalog_storage_type); +} + +void DatabaseDataLake::createTable( ContextPtr context_, const String & name, - bool /*sync*/) + const StoragePtr & table, + const ASTPtr & query) { - auto table = tryGetTable(name, context_); + /// Engine-clause path: `IcebergMetadata::createInitial` has already written the metadata and + /// registered the table; a path there that registers nothing throws instead of returning. if (table) - table->drop(); + return; + + auto catalog = getCatalog(); + const auto & create = query->as(); + const auto [namespace_name, table_name] = DataLake::parseTableName(name); + + ColumnsDescription columns; + if (create.columns_list && create.columns_list->columns) + { + for (const auto & child : create.columns_list->columns->children) + { + const auto * col_decl = child->as(); + if (!col_decl || !col_decl->getType()) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Invalid column declaration in CREATE TABLE"); + + if (col_decl->default_specifier != ColumnDefaultSpecifier::Empty) + throw Exception( + ErrorCodes::BAD_ARGUMENTS, + "Column '{}': {} is not yet supported by DataLakeCatalog table creation", + col_decl->name, + toString(col_decl->default_specifier)); + + if (col_decl->getComment() || col_decl->getCodec() || col_decl->getTTL() + || col_decl->getStatisticsDesc() || col_decl->getSettings() + || col_decl->primary_key_specifier) + throw Exception( + ErrorCodes::BAD_ARGUMENTS, + "Column '{}': COMMENT, CODEC, TTL, STATISTICS, SETTINGS, and PRIMARY KEY are not supported by DataLakeCatalog table creation", + col_decl->name); + + columns.add(ColumnDescription(col_decl->name, DataTypeFactory::instance().get(col_decl->getType()))); + } + } + + if (columns.empty()) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Cannot create table without columns"); + + if (create.columns_list + && ((create.columns_list->indices && !create.columns_list->indices->children.empty()) + || (create.columns_list->constraints && !create.columns_list->constraints->children.empty()) + || (create.columns_list->projections && !create.columns_list->projections->children.empty()) + || create.columns_list->primary_key + || create.columns_list->primary_key_from_columns)) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "DataLakeCatalog CREATE TABLE does not support PRIMARY KEY, indices, constraints, or projections"); + + ASTPtr partition_by; + ASTPtr order_by; + if (create.storage) + { + if (create.storage->primary_key || create.storage->sample_by + || create.storage->ttl_table || create.storage->unique_key + || create.storage->settings) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "DataLakeCatalog CREATE TABLE supports only PARTITION BY and ORDER BY; " + "PRIMARY KEY, SAMPLE BY, TTL, UNIQUE KEY, and engine SETTINGS are not supported"); + + if (create.storage->partition_by) + partition_by = create.storage->partition_by->clone(); + if (create.storage->order_by) + order_by = create.storage->order_by->clone(); + } + + String base_location = catalog->getDefaultBaseLocation(); + if (base_location.empty()) + base_location = settings[DatabaseDataLakeSetting::default_base_location].value; + + String location; + if (!base_location.empty()) + { + if (auto catalog_storage_type = getFixedStorageTypeForTableCreation(catalog); catalog_storage_type.has_value() + && DataLake::parseStorageTypeFromLocation(base_location) != *catalog_storage_type) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "`default_base_location` uses the {} storage backend, but this DataLakeCatalog stores tables on {}. " + "The table would be reopened with the catalog's storage backend and become unreadable " + "immediately after creation", + DataLake::parseStorageTypeFromLocation(base_location), *catalog_storage_type); + + while (base_location.ends_with('/')) + base_location.pop_back(); + location = fmt::format("{}/{}/{}", base_location, namespace_name, table_name); + } else - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Cannot drop table {} because it does not exist", name); + { + const auto storage_endpoint = settings[DatabaseDataLakeSetting::storage_endpoint].value; + if (storage_endpoint.empty()) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "CREATE TABLE in DataLakeCatalog requires `default_base_location` or `storage_endpoint`"); + location = DataLake::constructTableLocation( + getLocationSchemeForTableCreation(catalog), storage_endpoint, namespace_name, table_name, + settings[DatabaseDataLakeSetting::storage_uri_style]); + } + + auto [metadata_content, metadata_str] = Iceberg::createEmptyMetadataFile( + location, + columns, + partition_by, + order_by, + context_); + + /// Catalogs that write the initial metadata file themselves (empty `metadata_path`) must honour + /// `iceberg_metadata_compression_method`, as `IcebergMetadata::createInitial` does. + const auto compression_method_str = context_->getSettingsRef()[Setting::iceberg_metadata_compression_method].value; + const auto compression_method = chooseCompressionMethod(compression_method_str, compression_method_str); + + /// Register the namespace before `createTable`, which requires it to exist and, for catalogs that + /// write the initial metadata file themselves, must not be preceded by any file written to storage + /// (see `ICatalog::createTable`). Do it after all local validation, so a rejected `CREATE` leaves no + /// trace in the catalog. The namespace's default location must be the namespace base, not this first + /// table's directory, or later tables created without an explicit location would land under it. + String namespace_location = location; + if (const String table_suffix = "/" + table_name; namespace_location.ends_with(table_suffix)) + namespace_location.resize(namespace_location.size() - table_suffix.size()); + catalog->createNamespaceIfNotExists(namespace_name, namespace_location); + + const bool created = catalog->createTable( + namespace_name, table_name, /* metadata_path */ "", metadata_content, compression_method, create.if_not_exists); + if (!created) + { + /// `IF NOT EXISTS`, and another client registered this name between the existence check in + /// `doCreateTable` and this call. Nothing was created here, and the caller must be able to tell. + throw DataLake::TableAlreadyExistsInCatalogException( + "Table {}.{} already exists in the catalog", namespace_name, table_name); + } + + LOG_INFO(log, "Created table {}.{}", namespace_name, table_name); +} + +void DatabaseDataLake::dropTable( /// NOLINT + ContextPtr context_, + const String & name, + bool /*sync*/, + bool if_exists) +{ + auto catalog = getCatalog(); + const auto [namespace_name, table_name] = DataLake::parseTableName(name); + + bool purge = context_->getSettingsRef()[Setting::data_lake_delete_data_on_drop]; + catalog->dropTable(namespace_name, table_name, purge, if_exists); + + LOG_INFO(log, "Dropped table {}.{} from DataLakeCatalog (purge={})", namespace_name, table_name, purge); } DatabaseTablesIteratorPtr DatabaseDataLake::getTablesIterator( @@ -1268,6 +1505,7 @@ The following settings are supported: | `auth_header` | Custom HTTP header for authentication with the catalog service | | `auth_scope` | OAuth2 scope for authentication (if using OAuth) | | `storage_endpoint` | Endpoint URL for the underlying storage | +| `default_base_location` | Base URI for new tables when the catalog does not report `default-base-location`. New tables are placed under `//` (e.g. `s3://warehouse/data`) | | `oauth_server_uri` | URI of the OAuth2 authorization server for authentication | | `vended_credentials` | Boolean indicating whether to use vended credentials from the catalog (supports AWS S3 and Azure ADLS Gen2) | | `vended_credentials_cache_ttl` | Maximum cache entry lifetime (in seconds) for vended credentials (REST catalogs only). Default `300`; `0` disables caching. | @@ -1277,6 +1515,95 @@ The following settings are supported: | `dlf_access_key_id` | Access key ID for DLF access | | `dlf_access_key_secret` | Access key Secret for DLF access | +## Creating tables {#creating-tables} + +An Iceberg table in a `DataLakeCatalog` database can be created directly from ClickHouse. + +:::note +`CREATE TABLE` and `DROP TABLE` require a catalog that can perform catalog mutations. They are supported +for Iceberg REST catalogs (including OneLake, BigLake, and Delta Sharing) and for the AWS Glue catalog. +Other catalog types (Unity, Hive Metastore, Paimon REST) are read-only and reject these statements. +::: + +The location of a newly created table comes from `default_base_location` (a full `s3://bucket/prefix`) when +set, otherwise the bucket is derived from `storage_endpoint`. With `storage_uri_style = 'virtual_hosted'` the +bucket cannot be derived from the endpoint unambiguously, so `default_base_location` is required for +`CREATE TABLE`. + +The table name must be quoted with backticks and include the namespace separated by a dot: + +```sql +CREATE TABLE catalog_db.`namespace.table_name` +( + id Int64, + name String, + value Float64 +) +PARTITION BY id +ORDER BY name +SETTINGS allow_database_iceberg = 1; +``` + +Iceberg accepts only a fixed set of partition transforms, so `PARTITION BY` +must use one of the following expressions: + +| Expression | Iceberg transform | +|-------------------------------|-------------------| +| `` | `identity` | +| `toYearNumSinceEpoch()` | `year` | +| `toMonthNumSinceEpoch()` | `month` | +| `toRelativeDayNum()` | `day` | +| `toRelativeHourNum()` | `hour` | +| `icebergTruncate(N, )` | `truncate[N]` | +| `icebergBucket(N, )` | `bucket[N]` | + +Composite partitioning is supported via `PARTITION BY (expr1, expr2, ...)`. +Other expressions (e.g. `toYYYYMM`, `intDiv`) are rejected at `CREATE TABLE`. + +Only the column names and types, `PARTITION BY`, and `ORDER BY` are persisted into the Iceberg +table metadata. Anything else — the storage clauses `PRIMARY KEY`, `SAMPLE BY`, `TTL`, and +`UNIQUE KEY`; indices, constraints, and projections; and the column modifiers `DEFAULT`, +`MATERIALIZED`, `ALIAS`, `EPHEMERAL`, `COMMENT`, `CODEC`, `TTL`, `STATISTICS`, and `SETTINGS` — +is rejected rather than silently dropped. This applies both with and without an explicit +`ENGINE` clause. Engine `SETTINGS` are accepted only together with an explicit Iceberg engine, +where they are the engine's storage settings (e.g. `iceberg_format_version`). + +You can also create an Iceberg table that inherits the schema of an existing table: + +```sql +CREATE TABLE catalog_db.`namespace.table_name` +AS other_db.source_table +SETTINGS allow_database_iceberg = 1; +``` + +If the source table's `PARTITION BY` and `ORDER BY` use only the expressions +listed above, they are copied into the new Iceberg table. + +## Dropping tables {#dropping-tables} + +Tables can be dropped from a `DataLakeCatalog` database. +`DROP TABLE` sends a delete request to the remote catalog, which removes +the table entry from the catalog. + +```sql +DROP TABLE catalog_db.`namespace.table_name` +``` + +By default, ClickHouse does not request the catalog to delete the underlying data. In order to do it, use the `data_lake_delete_data_on_drop` setting: + +```sql +DROP TABLE catalog_db.`namespace.table_name` +SETTINGS data_lake_delete_data_on_drop = 1 +``` + +:::note +Whether data files are actually deleted depends on the catalog itself. +The `purgeRequested` flag is sent to the catalog, but the catalog may choose to ignore it. +For the Glue catalog, `DROP TABLE` only removes the catalog entry and does not delete the underlying data +files, so `DROP TABLE` with `data_lake_delete_data_on_drop = 1` is rejected instead of silently leaving the +data behind. +::: + ## Examples {#examples} See below sections for examples of using the `DataLakeCatalog` engine: diff --git a/src/Databases/DataLake/DatabaseDataLake.h b/src/Databases/DataLake/DatabaseDataLake.h index bd5fc6ac44f0..f9b5ee1ccb29 100644 --- a/src/Databases/DataLake/DatabaseDataLake.h +++ b/src/Databases/DataLake/DatabaseDataLake.h @@ -56,16 +56,19 @@ class DatabaseDataLake final : public IDatabase, WithContext std::vector> getTablesForBackup(const FilterByNameFunction &, const ContextPtr &) const override { return {}; } + void validateCreateTableEngine(const String & engine_name) const override; + void createTable( - ContextPtr /*context*/, - const String & /*name*/, + ContextPtr context, + const String & name, const StoragePtr & /*table*/, - const ASTPtr & /*query*/) override {} + const ASTPtr & query) override; void dropTable( /// NOLINT ContextPtr context_, const String & name, - bool /*sync*/) override; + bool /*sync*/, + bool if_exists) override; std::shared_ptr getCatalog() const; protected: diff --git a/src/Databases/DataLake/DatabaseDataLakeSettings.cpp b/src/Databases/DataLake/DatabaseDataLakeSettings.cpp index 216d32290c24..3514738daf50 100644 --- a/src/Databases/DataLake/DatabaseDataLakeSettings.cpp +++ b/src/Databases/DataLake/DatabaseDataLakeSettings.cpp @@ -33,6 +33,7 @@ namespace ErrorCodes DECLARE(String, aws_role_session_name, "", "Role session name for AWS connection for Glue catalog", 0) \ DECLARE(String, aws_external_id, "", "External id for the AWS STS AssumeRole trust policy for Glue catalog", 0) \ DECLARE(String, storage_endpoint, "", "Object storage endpoint", 0) \ + DECLARE(String, default_base_location, "", "Base URI under which CREATE TABLE places new tables. Used only when the catalog does not report `default-base-location`", 0) \ DECLARE(S3UriStyle, storage_uri_style, S3UriStyle::AUTO, "URL style used when constructing object storage URLs from catalog-provided table locations. Use 'virtual_hosted' when the object storage server requires the bucket in the hostname (e.g. https://bucket.endpoint.com/path/)", 0) \ DECLARE(String, onelake_tenant_id, "", "Tenant id from azure", 0) \ DECLARE(String, onelake_client_id, "", "Client id from azure", 0) \ diff --git a/src/Databases/DataLake/GlueCatalog.cpp b/src/Databases/DataLake/GlueCatalog.cpp index a8dda240e7ce..25d992448864 100644 --- a/src/Databases/DataLake/GlueCatalog.cpp +++ b/src/Databases/DataLake/GlueCatalog.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -49,9 +50,12 @@ #include #include #include +#include #include #include +#include #include +#include #include #include @@ -61,6 +65,8 @@ namespace DB::ErrorCodes extern const int DATALAKE_DATABASE_ERROR; extern const int FAULT_INJECTED; extern const int CATALOG_NAMESPACE_DISABLED; + extern const int NOT_IMPLEMENTED; + extern const int S3_ERROR; } namespace DB::FailPoints @@ -380,7 +386,7 @@ bool GlueCatalog::tryGetTableMetadata( auto setup_specific_properties = [&] { const auto & table_params = table_outcome.GetParameters(); - if (table_params.contains("metadata_location")) + if (table_params.contains("metadata_location") && !table_params.at("metadata_location").empty()) { result.setDataLakeSpecificProperties(DataLakeSpecificProperties{.iceberg_metadata_file_location = table_params.at("metadata_location")}); } @@ -640,8 +646,25 @@ String GlueCatalog::resolveMetadataPathFromTableLocation(const String & table_lo } } -void GlueCatalog::createNamespaceIfNotExists(const String & namespace_name) const +void GlueCatalog::createNamespaceIfNotExists(const String & namespace_name, const String & /*location*/) const { + /// `CreateDatabase` may be denied to a principal that is still allowed to create tables in a + /// pre-provisioned namespace, so it must not be called when there is nothing to create. + Aws::Glue::Model::GetDatabaseRequest get_request; + get_request.SetName(namespace_name); + + auto get_outcome = glue_client->GetDatabase(get_request); + if (get_outcome.IsSuccess()) + return; + + if (get_outcome.GetError().GetErrorType() != Aws::Glue::GlueErrors::ENTITY_NOT_FOUND) + { + throw DB::Exception( + DB::ErrorCodes::DATALAKE_DATABASE_ERROR, + "Exception calling GetDatabase for namespace {}: {}", + namespace_name, get_outcome.GetError().GetMessage()); + } + Aws::Glue::Model::CreateDatabaseRequest create_request; Aws::Glue::Model::DatabaseInput db_input; db_input.SetName(namespace_name); @@ -652,14 +675,81 @@ void GlueCatalog::createNamespaceIfNotExists(const String & namespace_name) cons glue_client->CreateDatabase(create_request); } -void GlueCatalog::createTable(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr /*metadata_content*/) const +bool GlueCatalog::createTable( + const String & namespace_name, + const String & table_name, + const String & new_metadata_path, + Poco::JSON::Object::Ptr metadata_content, + DB::CompressionMethod metadata_compression_method, + bool if_not_exists) const { if (!isNamespaceAllowed(namespace_name)) throw DB::Exception(DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED, "Failed to create table {}, namespace {} is filtered by `namespaces` database parameter", table_name, namespace_name); - createNamespaceIfNotExists(namespace_name); + String effective_metadata_path = new_metadata_path; + + DB::ObjectStoragePtr written_metadata_storage; + String written_metadata_file; + + /// The initial metadata file staged below must not outlive a registration that did not happen: it is + /// written with `If-None-Match: *`, so a leftover permanently blocks every retry. Removing it is safe + /// because only this call can have created it - a writer that lost the `If-None-Match` race returns + /// before staging anything. + bool registered = false; + SCOPE_EXIT_SAFE({ + if (!registered && written_metadata_storage) + { + LOG_INFO( + log, + "Table {}.{} was not registered in the Glue catalog, removing the staged initial metadata file {}", + namespace_name, + table_name, + written_metadata_file); + written_metadata_storage->removeObjectIfExists(DB::StoredObject(written_metadata_file)); + } + }); + + if (effective_metadata_path.empty() && metadata_content && metadata_content->has("location")) + { + String table_location = metadata_content->getValue("location"); + while (table_location.ends_with('/')) + table_location = table_location.substr(0, table_location.size() - 1); + + TableMetadata dummy_metadata; + auto [object_storage, bucket_name, table_path] = createObjectStorageForEarlyTableAccess(table_location, dummy_metadata); + + /// Name the file exactly like `IcebergMetadata::createInitial` does, so other engines can + /// locate the metadata file. + String compression_suffix = DB::toContentEncodingName(metadata_compression_method); + if (!compression_suffix.empty()) + compression_suffix = "." + compression_suffix; + + String metadata_filename = fmt::format("{}/metadata/v1{}.metadata.json", table_path, compression_suffix); + + std::ostringstream oss; // STYLE_CHECK_ALLOW_STD_STRING_STREAM + Poco::JSON::Stringifier::stringify(metadata_content, oss, 4); + String metadata_str = DB::removeEscapedSlashes(oss.str()); + + try + { + DB::Iceberg::writeMessageToFile(metadata_str, metadata_filename, object_storage, getContext(), "*", "", metadata_compression_method); + } + catch (const DB::Exception & e) + { + /// The write is guarded by `If-None-Match: *`, so S3 answers `PreconditionFailed` once the + /// initial metadata file is there - someone else created this table first. + if (if_not_exists && e.code() == DB::ErrorCodes::S3_ERROR && e.message().contains("PreconditionFailed")) + return false; + throw; + } + + written_metadata_storage = object_storage; + written_metadata_file = metadata_filename; + + effective_metadata_path = "s3://" + bucket_name + "/" + metadata_filename; + } Aws::Glue::Model::CreateTableRequest request; request.SetDatabaseName(namespace_name); @@ -668,18 +758,21 @@ void GlueCatalog::createTable(const String & namespace_name, const String & tabl table_input.SetName(table_name); Aws::Glue::Model::StorageDescriptor sd; - fs::path original_path = new_metadata_path; + if (!effective_metadata_path.empty()) + { + fs::path original_path = effective_metadata_path; - fs::path parent = original_path.parent_path(); - fs::path grandparent = parent.parent_path(); + fs::path parent = original_path.parent_path(); + fs::path grandparent = parent.parent_path(); - sd.SetLocation(grandparent.c_str()); + sd.SetLocation(grandparent.c_str()); + } table_input.SetStorageDescriptor(sd); table_input.SetTableType("ICEBERG"); Aws::Map parameters; - parameters["metadata_location"] = new_metadata_path; + parameters["metadata_location"] = effective_metadata_path; parameters["table_type"] = "ICEBERG"; table_input.SetParameters(parameters); @@ -695,7 +788,16 @@ void GlueCatalog::createTable(const String & namespace_name, const String & tabl } if (!response.IsSuccess()) - throw DB::Exception(DB::ErrorCodes::DATALAKE_DATABASE_ERROR, "Can not create metadata in glue catalog: {}", response.GetError().GetMessage()); + { + /// The staged metadata file is removed by the scope guard, so `IF NOT EXISTS` leaves nothing behind. + if (if_not_exists && response.GetError().GetErrorType() == Aws::Glue::GlueErrors::ALREADY_EXISTS) + return false; + throw DB::Exception( + DB::ErrorCodes::DATALAKE_DATABASE_ERROR, "Can not create metadata in glue catalog: {}", response.GetError().GetMessage()); + } + + registered = true; + return true; } bool GlueCatalog::updateMetadata(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr /*new_snapshot*/) const @@ -753,13 +855,27 @@ bool GlueCatalog::updateSchema( return updateMetadata(namespace_name, table_name, new_metadata_path, nullptr); } -void GlueCatalog::dropTable(const String & namespace_name, const String & table_name) const +void GlueCatalog::dropTable(const String & namespace_name, const String & table_name, bool purge, bool if_exists) const { if (!isNamespaceAllowed(namespace_name)) throw DB::Exception(DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED, "Failed to drop table {}, namespace {} is filtered by `namespaces` database parameter", table_name, namespace_name); + /// Glue's `DeleteTable` removes only the catalog entry; the client-side purge of the data files is not + /// implemented, so reject a requested purge rather than silently ignore it and orphan the data. + /// TODO: implement the client-side purge so `data_lake_delete_data_on_drop` can be honored for Glue. + if (purge) + { + if (if_exists && !existsTable(namespace_name, table_name)) + return; + + throw DB::Exception( + DB::ErrorCodes::NOT_IMPLEMENTED, + "data_lake_delete_data_on_drop is not supported for the Glue catalog: dropping only removes the Glue " + "catalog entry and does not delete the underlying data files"); + } + Aws::Glue::Model::DeleteTableRequest request; request.SetDatabaseName(namespace_name); request.SetName(table_name); @@ -772,7 +888,9 @@ void GlueCatalog::dropTable(const String & namespace_name, const String & table_ response = glue_client->DeleteTable(request); } - if (!response.IsSuccess()) + /// `EntityNotFoundException` means the table is already gone - someone else dropped it first. + if (!response.IsSuccess() + && !(if_exists && response.GetError().GetErrorType() == Aws::Glue::GlueErrors::ENTITY_NOT_FOUND)) throw DB::Exception( DB::ErrorCodes::DATALAKE_DATABASE_ERROR, "Can not delete table from glue catalog: {}", diff --git a/src/Databases/DataLake/GlueCatalog.h b/src/Databases/DataLake/GlueCatalog.h index 8d10ba0c8667..913f943b98aa 100644 --- a/src/Databases/DataLake/GlueCatalog.h +++ b/src/Databases/DataLake/GlueCatalog.h @@ -66,7 +66,13 @@ class GlueCatalog final : public ICatalog, private DB::WithContext return DB::DatabaseDataLakeCatalogType::GLUE; } - void createTable(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr metadata_content) const override; + bool createTable( + const String & namespace_name, + const String & table_name, + const String & new_metadata_path, + Poco::JSON::Object::Ptr metadata_content, + DB::CompressionMethod metadata_compression_method, + bool if_not_exists) const override; bool updateMetadata(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr new_snapshot) const override; @@ -79,7 +85,7 @@ class GlueCatalog final : public ICatalog, private DB::WithContext Int32 new_last_column_id, Poco::JSON::Object::Ptr metadata = nullptr) const override; - void dropTable(const String & namespace_name, const String & table_name) const override; + void dropTable(const String & namespace_name, const String & table_name, bool purge, bool if_exists) const override; /// Returns a callback that re-vends fresh AWS credentials from the configured /// credentials provider chain. Invoked by `ReadBufferFromS3` when an S3 call @@ -95,8 +101,10 @@ class GlueCatalog final : public ICatalog, private DB::WithContext const String & column_name, const String & glue_column_type); + /// Glue namespaces carry no default location, so `location` is ignored. + void createNamespaceIfNotExists(const String & namespace_name, const String & location) const override; + private: - void createNamespaceIfNotExists(const String & namespace_name) const; std::unique_ptr glue_client; const LoggerPtr log; diff --git a/src/Databases/DataLake/ICatalog.cpp b/src/Databases/DataLake/ICatalog.cpp index 62cf44930225..2eab9740d7c7 100644 --- a/src/Databases/DataLake/ICatalog.cpp +++ b/src/Databases/DataLake/ICatalog.cpp @@ -81,6 +81,25 @@ StorageType parseStorageTypeFromString(const std::string & type) return *storage_type; } +std::string storageTypeToScheme(StorageType type) +{ + switch (type) + { + case StorageType::S3: + return "s3"; + case StorageType::Azure: + return "abfss"; + case StorageType::Local: + return "file"; + case StorageType::HDFS: + return "hdfs"; + case StorageType::Other: + throw DB::Exception( + DB::ErrorCodes::BAD_ARGUMENTS, + "Cannot determine URI scheme for storage type 'Other'"); + } +} + void TableMetadata::setLocation(const std::string & location_) { if (!with_location) @@ -335,9 +354,17 @@ DB::SettingsChanges CatalogSettings::allChanged() const return changes; } -void ICatalog::createTable(const String & /*namespace_name*/, const String & /*table_name*/, const String & /*new_metadata_path*/, Poco::JSON::Object::Ptr /*metadata_content*/) const +bool ICatalog::createTable( + const String & /*namespace_name*/, + const String & /*table_name*/, + const String & /*new_metadata_path*/, + Poco::JSON::Object::Ptr /*metadata_content*/, + DB::CompressionMethod /*metadata_compression_method*/, + bool /*if_not_exists*/) const { - throw DB::Exception(DB::ErrorCodes::NOT_IMPLEMENTED, "createTable is not implemented"); + throw DB::Exception(DB::ErrorCodes::NOT_IMPLEMENTED, + "CREATE TABLE is not supported for this DataLakeCatalog catalog type; " + "it is available only for Iceberg REST (including OneLake, BigLake, Delta Sharing) and Glue catalogs"); } bool ICatalog::updateMetadata(const String & /*namespace_name*/, const String & /*table_name*/, const String & /*new_metadata_path*/, Poco::JSON::Object::Ptr /*new_snapshot*/) const @@ -357,9 +384,16 @@ bool ICatalog::updateSchema( throw DB::Exception(DB::ErrorCodes::NOT_IMPLEMENTED, "updateSchema is not implemented"); } -void ICatalog::dropTable(const String & /*namespace_name*/, const String & /*table_name*/) const +void ICatalog::createNamespaceIfNotExists(const String & /*namespace_name*/, const String & /*location*/) const +{ + throw DB::Exception(DB::ErrorCodes::NOT_IMPLEMENTED, "createNamespaceIfNotExists is not implemented"); +} + +void ICatalog::dropTable(const String & /*namespace_name*/, const String & /*table_name*/, bool /*purge*/, bool /*if_exists*/) const { - throw DB::Exception(DB::ErrorCodes::NOT_IMPLEMENTED, "dropTable is not implemented"); + throw DB::Exception(DB::ErrorCodes::NOT_IMPLEMENTED, + "DROP TABLE is not supported for this DataLakeCatalog catalog type; " + "it is available only for Iceberg REST (including OneLake, BigLake, Delta Sharing) and Glue catalogs"); } } diff --git a/src/Databases/DataLake/ICatalog.h b/src/Databases/DataLake/ICatalog.h index f77bfcff0405..6b9062122c5b 100644 --- a/src/Databases/DataLake/ICatalog.h +++ b/src/Databases/DataLake/ICatalog.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ namespace DataLake using StorageType = DB::DatabaseDataLakeStorageType; StorageType parseStorageTypeFromLocation(const std::string & location); StorageType parseStorageTypeFromString(const std::string &type); +std::string storageTypeToScheme(StorageType type); struct DataLakeSpecificProperties { @@ -189,8 +191,24 @@ class ICatalog /// E.g. one of S3, Azure, Local, HDFS. virtual std::optional getStorageType() const = 0; - /// Creates new table in catalog. - virtual void createTable(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr metadata_content) const; + /// Catalog-wide base location for new tables, e.g. `s3://warehouse/data`. Empty if unknown. + virtual String getDefaultBaseLocation() const { return ""; } + + /// Creates new table in catalog. Callers must ensure the namespace exists before + /// writing any table files to storage: a catalog that shares its storage view with + /// the data refuses to create a namespace over a plain directory those files create. + /// `metadata_compression_method` applies only to catalogs that write the initial metadata file + /// themselves (`new_metadata_path` is empty): they must name it `v1..metadata.json` and compress + /// it accordingly, like `DB::IcebergMetadata::createInitial` does. + /// Returns `true` if this call created the table, `false` if `if_not_exists` is set and the shared + /// catalog reported that another client had already created it. + virtual bool createTable( + const String & namespace_name, + const String & table_name, + const String & new_metadata_path, + Poco::JSON::Object::Ptr metadata_content, + DB::CompressionMethod metadata_compression_method, + bool if_not_exists) const; /// Updates metadata in catalog. virtual bool updateMetadata(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr new_snapshot) const; @@ -211,8 +229,15 @@ class ICatalog Int32 new_last_column_id, Poco::JSON::Object::Ptr metadata = nullptr) const; + /// Register `namespace_name` if the catalog does not have it yet. `location` is the namespace's + /// default location (its base directory), and is empty when the caller has no base to offer - a + /// table's own directory must never be used, or later tables created in the namespace without an + /// explicit location would land inside that first table. + virtual void createNamespaceIfNotExists(const String & namespace_name, const String & location) const; + /// Drop table from catalog. - virtual void dropTable(const String & namespace_name, const String & table_name) const; + /// If `purge`, the catalog is also asked to delete the underlying data files. + virtual void dropTable(const String & namespace_name, const String & table_name, bool purge, bool if_exists) const; /// Does the catalog support transactions or anything like that? /// For example, the Iceberg REST catalog supports atomic operations "compare if snapshot X is equal to" and "add new snapshot Y". diff --git a/src/Databases/DataLake/RestCatalog.cpp b/src/Databases/DataLake/RestCatalog.cpp index 58bf459992f0..2730c0b78f19 100644 --- a/src/Databases/DataLake/RestCatalog.cpp +++ b/src/Databases/DataLake/RestCatalog.cpp @@ -176,6 +176,22 @@ String encodeNamespaceForURI(const String & namespace_name) return encoded; } +/// Per Iceberg REST spec, `namespace` is a JSON array of segments. Split `ns.a.b` on dots. +Poco::JSON::Array::Ptr namespaceToJSONArray(const String & namespace_name) +{ + Poco::JSON::Array::Ptr segments = new Poco::JSON::Array; + size_t start = 0; + while (start <= namespace_name.size()) + { + size_t dot = namespace_name.find('.', start); + if (dot == String::npos) + dot = namespace_name.size(); + segments->add(namespace_name.substr(start, dot - start)); + start = dot + 1; + } + return segments; +} + std::unordered_set getAllowedBigLakeMetadataServiceHosts( const Poco::Util::AbstractConfiguration & config) { @@ -191,7 +207,6 @@ std::unordered_set getAllowedBigLakeMetadataServiceHosts( return allowed; } - } namespace @@ -291,9 +306,7 @@ Poco::JSON::Object::Ptr buildUpdateSchemaRequestBody( { Poco::JSON::Object::Ptr identifier = new Poco::JSON::Object; identifier->set("name", table_name); - Poco::JSON::Array::Ptr namespaces = new Poco::JSON::Array; - namespaces->add(namespace_name); - identifier->set("namespace", namespaces); + identifier->set("namespace", namespaceToJSONArray(namespace_name)); request_body->set("identifier", identifier); } @@ -372,9 +385,7 @@ Poco::JSON::Object::Ptr buildUpdateMetadataRequestBody( { Poco::JSON::Object::Ptr identifier = new Poco::JSON::Object; identifier->set("name", table_name); - Poco::JSON::Array::Ptr namespaces = new Poco::JSON::Array; - namespaces->add(namespace_name); - identifier->set("namespace", namespaces); + identifier->set("namespace", namespaceToJSONArray(namespace_name)); request_body->set("identifier", identifier); } @@ -897,6 +908,11 @@ std::optional RestCatalog::getStorageType() const return parseStorageTypeFromLocation(config.default_base_location); } +String RestCatalog::getDefaultBaseLocation() const +{ + return config.default_base_location; +} + DB::ReadWriteBufferFromHTTPPtr RestCatalog::createReadBuffer( const std::string & endpoint, const Poco::URI::QueryParameters & params, @@ -1642,11 +1658,10 @@ void RestCatalog::createNamespaceIfNotExists(const String & namespace_name, cons const std::string endpoint = (base_url / config.prefix / NAMESPACES_ENDPOINT).generic_string(); Poco::JSON::Object::Ptr request_body = new Poco::JSON::Object; - { - Poco::JSON::Array::Ptr namespaces = new Poco::JSON::Array; - namespaces->add(namespace_name); - request_body->set("namespace", namespaces); - } + request_body->set("namespace", namespaceToJSONArray(namespace_name)); + + /// The caller leaves `location` empty when it has no namespace base to offer. + if (!location.empty()) { Poco::JSON::Object::Ptr properties = new Poco::JSON::Object; properties->set("location", location); @@ -1665,19 +1680,26 @@ void RestCatalog::createNamespaceIfNotExists(const String & namespace_name, cons } } -void RestCatalog::createTable(const String & namespace_name, const String & table_name, const String & /*new_metadata_path*/, Poco::JSON::Object::Ptr metadata_content) const +/// `metadata_compression_method` is unused: the REST server writes and names the initial metadata file itself. +bool RestCatalog::createTable( + const String & namespace_name, + const String & table_name, + const String & /*new_metadata_path*/, + Poco::JSON::Object::Ptr metadata_content, + DB::CompressionMethod /*metadata_compression_method*/, + bool if_not_exists) const { if (!allowed_namespaces.isNamespaceAllowed(namespace_name, /*nested*/ false)) throw DB::Exception(DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED, "Failed to create table {}, namespace {} is filtered by `namespaces` database parameter", table_name, namespace_name); - createNamespaceIfNotExists(namespace_name, metadata_content->getValue("location")); + const String location = metadata_content->getValue("location"); const std::string endpoint = (base_url / config.prefix / NAMESPACES_ENDPOINT / encodeNamespaceForURI(namespace_name) / "tables").generic_string(); Poco::JSON::Object::Ptr request_body = new Poco::JSON::Object; request_body->set("name", table_name); - request_body->set("location", metadata_content->getValue("location")); + request_body->set("location", location); { Poco::JSON::Object::Ptr initial_schema = metadata_content->getArray("schemas")->getObject(0); Poco::JSON::Array::Ptr identifier_fields = new Poco::JSON::Array; @@ -1686,13 +1708,21 @@ void RestCatalog::createTable(const String & namespace_name, const String & tabl } request_body->set("partition-spec", metadata_content->getArray("partition-specs")->get(0)); + if (metadata_content->has("sort-orders")) { - Poco::JSON::Object::Ptr write_order = new Poco::JSON::Object; - write_order->set("order-id", 0); - Poco::JSON::Array::Ptr fields = new Poco::JSON::Array; - write_order->set("fields", fields); - request_body->set("write-order", write_order); + if (auto sort_orders = metadata_content->getArray("sort-orders"); sort_orders->size() > 0) + { + auto sort_order = sort_orders->getObject(0); + auto fields = sort_order->getArray("fields"); + if (fields && fields->size() > 0) + { + if (sort_order->getValue("order-id") == 0) + sort_order->set("order-id", 1); + request_body->set("write-order", sort_order); + } + } } + request_body->set("stage-create", false); Poco::JSON::Object::Ptr properties = new Poco::JSON::Object; @@ -1709,8 +1739,13 @@ void RestCatalog::createTable(const String & namespace_name, const String & tabl } catch (const DB::HTTPException & ex) { + /// `409` means the catalog already has a table with this name. + if (if_not_exists && ex.getHTTPStatus() == Poco::Net::HTTPResponse::HTTPStatus::HTTP_CONFLICT) + return false; throw DB::Exception(DB::ErrorCodes::DATALAKE_DATABASE_ERROR, "Failed to create table {}", ex.displayText()); } + + return true; } @@ -1785,16 +1820,17 @@ bool RestCatalog::updateSchema( return true; } -void RestCatalog::dropTable(const String & namespace_name, const String & table_name) const +void RestCatalog::dropTable(const String & namespace_name, const String & table_name, bool purge, bool if_exists) const { if (!allowed_namespaces.isNamespaceAllowed(namespace_name, /*nested*/ false)) throw DB::Exception(DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED, "Failed to drop table {}, namespace {} is filtered by `namespaces` database parameter", table_name, namespace_name); - const std::string endpoint - = (base_url / config.prefix / NAMESPACES_ENDPOINT / encodeNamespaceForURI(namespace_name) / "tables" / table_name).generic_string() - + "?purgeRequested=False"; + /// Same URL shape as `createTable`, `updateMetadata` and `getTableMetadataImpl`. + const std::string base_endpoint + = (base_url / config.prefix / NAMESPACES_ENDPOINT / encodeNamespaceForURI(namespace_name) / "tables" / table_name).generic_string(); + const std::string endpoint = fmt::format("{}?purgeRequested={}", base_endpoint, purge ? "true" : "false"); Poco::JSON::Object::Ptr request_body = nullptr; try @@ -1805,6 +1841,9 @@ void RestCatalog::dropTable(const String & namespace_name, const String & table_ } catch (const DB::HTTPException & ex) { + /// `404` means the table is already gone. + if (if_exists && ex.getHTTPStatus() == Poco::Net::HTTPResponse::HTTPStatus::HTTP_NOT_FOUND) + return; throw DB::Exception(DB::ErrorCodes::DATALAKE_DATABASE_ERROR, "Failed to drop table {}", ex.displayText()); } } diff --git a/src/Databases/DataLake/RestCatalog.h b/src/Databases/DataLake/RestCatalog.h index 4eb33d1045ab..c2c8ac2f37c9 100644 --- a/src/Databases/DataLake/RestCatalog.h +++ b/src/Databases/DataLake/RestCatalog.h @@ -82,12 +82,20 @@ class RestCatalog : public ICatalog, public DB::WithContext std::optional getStorageType() const override; + String getDefaultBaseLocation() const override; + DB::DatabaseDataLakeCatalogType getCatalogType() const override { return DB::DatabaseDataLakeCatalogType::ICEBERG_REST; } - void createTable(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr metadata_content) const override; + bool createTable( + const String & namespace_name, + const String & table_name, + const String & new_metadata_path, + Poco::JSON::Object::Ptr metadata_content, + DB::CompressionMethod metadata_compression_method, + bool if_not_exists) const override; bool updateMetadata(const String & namespace_name, const String & table_name, const String & new_metadata_path, Poco::JSON::Object::Ptr new_snapshot) const override; @@ -102,7 +110,9 @@ class RestCatalog : public ICatalog, public DB::WithContext bool isTransactional() const override { return true; } - void dropTable(const String & namespace_name, const String & table_name) const override; + void dropTable(const String & namespace_name, const String & table_name, bool purge, bool if_exists) const override; + + void createNamespaceIfNotExists(const String & namespace_name, const String & location) const override; ICatalog::CredentialsRefreshCallback getCredentialsConfigurationCallback(const DB::StorageID & storage_id) override; @@ -121,8 +131,6 @@ class RestCatalog : public ICatalog, public DB::WithContext const std::string & namespaces_, DB::ContextPtr context_); - void createNamespaceIfNotExists(const String & namespace_name, const String & location) const; - struct Config { /// Prefix is a path of the catalog endpoint, diff --git a/src/Databases/DataLake/S3TablesCatalog.cpp b/src/Databases/DataLake/S3TablesCatalog.cpp index 07cd7e723da3..2aaca65d3b42 100644 --- a/src/Databases/DataLake/S3TablesCatalog.cpp +++ b/src/Databases/DataLake/S3TablesCatalog.cpp @@ -33,6 +33,7 @@ namespace DB::ErrorCodes { extern const int BAD_ARGUMENTS; extern const int DATALAKE_DATABASE_ERROR; + extern const int SUPPORT_IS_DISABLED; } namespace DB::Setting @@ -214,8 +215,24 @@ ICatalog::CredentialsRefreshCallback S3TablesCatalog::getCredentialsConfiguratio }; } -void S3TablesCatalog::dropTable(const String & namespace_name, const String & table_name) const +void S3TablesCatalog::dropTable(const String & namespace_name, const String & table_name, bool delete_data, bool if_exists) const { + /// The API only offers a purging delete, so keeping the data is not expressible here; refuse rather + /// than delete data the `DROP TABLE` asked to keep. + /// https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-tables-delete.html + if (!delete_data) + { + /// A table that is not there is nothing to refuse, so `IF EXISTS` still means a no-op. + if (if_exists && !existsTable(namespace_name, table_name)) + return; + + throw DB::Exception( + DB::ErrorCodes::SUPPORT_IS_DISABLED, + "S3 Tables cannot drop table {}.{} without deleting its data, and `data_lake_delete_data_on_drop` is disabled. " + "Enable `data_lake_delete_data_on_drop` to drop the table together with its data", + namespace_name, table_name); + } + const std::string endpoint = (base_url / config.prefix / "namespaces" / namespace_name / "tables" / table_name).string() + "?purgeRequested=True"; diff --git a/src/Databases/DataLake/S3TablesCatalog.h b/src/Databases/DataLake/S3TablesCatalog.h index aff432c1b679..251253f72bc7 100644 --- a/src/Databases/DataLake/S3TablesCatalog.h +++ b/src/Databases/DataLake/S3TablesCatalog.h @@ -41,7 +41,7 @@ class S3TablesCatalog final : public RestCatalog DB::ContextPtr context_, TableMetadata & result) const override; - void dropTable(const String & namespace_name, const String & table_name) const override; + void dropTable(const String & namespace_name, const String & table_name, bool delete_data, bool if_exists) const override; ICatalog::CredentialsRefreshCallback getCredentialsConfigurationCallback(const DB::StorageID & storage_id) override; diff --git a/src/Databases/DataLake/tests/gtest_construct_table_location.cpp b/src/Databases/DataLake/tests/gtest_construct_table_location.cpp new file mode 100644 index 000000000000..5547bde1bc4e --- /dev/null +++ b/src/Databases/DataLake/tests/gtest_construct_table_location.cpp @@ -0,0 +1,126 @@ +#include +#include + +#include + +#include +#include + +namespace DataLake::Test +{ + +class ConstructTableLocationTest : public ::testing::Test +{ +}; + +TEST_F(ConstructTableLocationTest, S3HttpsEndpoint) +{ + EXPECT_EQ( + constructTableLocation("s3", "http://minio:9000/warehouse-rest", "ns", "tbl"), + "s3://warehouse-rest/ns/tbl"); + EXPECT_EQ( + constructTableLocation("s3", "http://minio:9000/warehouse/data", "ns", "tbl"), + "s3://warehouse/data/ns/tbl"); +} + +TEST_F(ConstructTableLocationTest, S3RejectsEndpointWithoutBucket) +{ + EXPECT_THROW( + constructTableLocation("s3", "http://minio:9000/", "ns", "tbl"), + DB::Exception); +} + +/// A virtual-hosted host cannot be split into bucket and service unambiguously, so `CREATE TABLE` rejects it +/// whatever the endpoint looks like; `default_base_location` has to be set instead. +TEST_F(ConstructTableLocationTest, S3VirtualHostedIsRejected) +{ + for (const auto * endpoint : { + "https://warehouse-rest.minio.example.com", + "https://warehouse-rest.minio.example.com/prefix", + "https://s3.us-east-1.amazonaws.com", + "https://10.0.0.5:9000", + }) + EXPECT_THROW( + constructTableLocation("s3", endpoint, "ns", "tbl", DB::S3UriStyle::VIRTUAL_HOSTED), + DB::Exception); +} + +/// The constructed Azure URI must round-trip through `setLocation`, which means it has to carry the +/// `@` authority. +TEST_F(ConstructTableLocationTest, AzureHttpsEndpoint) +{ + const String location = constructTableLocation( + "abfss", + "https://account.dfs.core.windows.net/mycontainer", + "ns", + "tbl"); + EXPECT_EQ(location, "abfss://mycontainer@account.dfs.core.windows.net/ns/tbl"); + + TableMetadata metadata; + metadata.withLocation(); + metadata.setLocation(location); + EXPECT_EQ(metadata.getLocation(), location); + EXPECT_EQ(metadata.getStorageType(), StorageType::Azure); + + EXPECT_EQ( + constructTableLocation( + "abfss", + "https://account.dfs.core.windows.net/mycontainer/warehouse/data", + "ns", + "tbl"), + "abfss://mycontainer@account.dfs.core.windows.net/warehouse/data/ns/tbl"); + EXPECT_EQ( + constructTableLocation( + "abfss", + "https://account.dfs.core.windows.net/mycontainer/", + "ns", + "tbl"), + "abfss://mycontainer@account.dfs.core.windows.net/ns/tbl"); +} + +TEST_F(ConstructTableLocationTest, AzureAbfssEndpoint) +{ + EXPECT_EQ( + constructTableLocation( + "abfss", + "abfss://mycontainer@account.dfs.core.windows.net/", + "ns", + "tbl"), + "abfss://mycontainer@account.dfs.core.windows.net/ns/tbl"); + EXPECT_EQ( + constructTableLocation( + "abfss", + "abfss://mycontainer@account.dfs.core.windows.net/warehouse/data", + "ns", + "tbl"), + "abfss://mycontainer@account.dfs.core.windows.net/warehouse/data/ns/tbl"); +} + +TEST_F(ConstructTableLocationTest, AzureRejectsEndpointWithoutContainer) +{ + EXPECT_THROW( + constructTableLocation("abfss", "https://account.dfs.core.windows.net/", "ns", "tbl"), + DB::Exception); + EXPECT_THROW( + constructTableLocation("abfss", "abfss://account.dfs.core.windows.net/", "ns", "tbl"), + DB::Exception); +} + +TEST_F(ConstructTableLocationTest, HdfsPreservesAuthority) +{ + EXPECT_EQ( + constructTableLocation("hdfs", "hdfs://namenode:9000/warehouse", "ns", "tbl"), + "hdfs://namenode:9000/warehouse/ns/tbl"); + EXPECT_EQ( + constructTableLocation("hdfs", "hdfs://namenode:9000", "ns", "tbl"), + "hdfs://namenode:9000/ns/tbl"); +} + +TEST_F(ConstructTableLocationTest, FileWithoutAuthority) +{ + EXPECT_EQ( + constructTableLocation("file", "file:///var/iceberg/warehouse", "ns", "tbl"), + "file:///var/iceberg/warehouse/ns/tbl"); +} + +} diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 1e21692c2b03..9bf815f58bfa 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -189,7 +189,7 @@ StoragePtr DatabaseAtomic::detachTable(ContextPtr /* context */, const String & return detached_table; } -void DatabaseAtomic::dropTable(ContextPtr local_context, const String & table_name, bool sync) +void DatabaseAtomic::dropTable(ContextPtr local_context, const String & table_name, bool sync, bool /*if_exists*/) { auto component_guard = Coordination::setCurrentComponent("DatabaseAtomic::dropTable"); waitDatabaseStarted(); diff --git a/src/Databases/DatabaseAtomic.h b/src/Databases/DatabaseAtomic.h index f1626a66087f..a6d4374249d7 100644 --- a/src/Databases/DatabaseAtomic.h +++ b/src/Databases/DatabaseAtomic.h @@ -48,7 +48,7 @@ class DatabaseAtomic : public DatabaseOrdinary bool exchange, bool dictionary) override; - void dropTable(ContextPtr context, const String & table_name, bool sync) override; + void dropTable(ContextPtr context, const String & table_name, bool sync, bool if_exists) override; void dropTableImpl(ContextPtr context, const String & table_name, bool sync); void attachTable(ContextPtr context, const String & name, const StoragePtr & table, const String & relative_table_path) override; diff --git a/src/Databases/DatabaseBackup.cpp b/src/Databases/DatabaseBackup.cpp index a60fb82f166a..77585b6586d7 100644 --- a/src/Databases/DatabaseBackup.cpp +++ b/src/Databases/DatabaseBackup.cpp @@ -187,6 +187,7 @@ void DatabaseBackup::detachTablePermanently(ContextPtr, const String &) void DatabaseBackup::dropTable( ContextPtr, const String &, + bool, bool) { throw Exception(ErrorCodes::UNSUPPORTED_METHOD, "DROP TABLE is not supported for Backup database"); diff --git a/src/Databases/DatabaseBackup.h b/src/Databases/DatabaseBackup.h index 180de68f4037..5abf0aa306e7 100644 --- a/src/Databases/DatabaseBackup.h +++ b/src/Databases/DatabaseBackup.h @@ -45,7 +45,8 @@ class DatabaseBackup final : public DatabaseOrdinary void dropTable( ContextPtr context, const String & table_name, - bool sync) override; + bool sync, + bool if_exists) override; void renameTable( ContextPtr context, diff --git a/src/Databases/DatabaseMemory.cpp b/src/Databases/DatabaseMemory.cpp index 8dd9741c9aeb..bd10a02604a8 100644 --- a/src/Databases/DatabaseMemory.cpp +++ b/src/Databases/DatabaseMemory.cpp @@ -59,7 +59,8 @@ void DatabaseMemory::createTable( void DatabaseMemory::dropTable( ContextPtr /*context*/, const String & table_name, - bool /*sync*/) + bool /*sync*/, + bool /*if_exists*/) { StoragePtr table; { diff --git a/src/Databases/DatabaseMemory.h b/src/Databases/DatabaseMemory.h index 2e7d313c864b..219cf1bd09f3 100644 --- a/src/Databases/DatabaseMemory.h +++ b/src/Databases/DatabaseMemory.h @@ -32,7 +32,8 @@ class DatabaseMemory final : public DatabaseWithOwnTablesBase void dropTable( ContextPtr context, const String & table_name, - bool sync) override; + bool sync, + bool if_exists) override; ASTPtr getCreateTableQueryImpl(const String & name, ContextPtr context, bool throw_on_error) const override; diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 89480f6bbece..731a4971a380 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -353,7 +353,7 @@ void DatabaseOnDisk::detachTablePermanently(ContextPtr query_context, const Stri } } -void DatabaseOnDisk::dropTable(ContextPtr local_context, const String & table_name, bool /*sync*/) +void DatabaseOnDisk::dropTable(ContextPtr local_context, const String & table_name, bool /*sync*/, bool /*if_exists*/) { auto component_guard = Coordination::setCurrentComponent("DatabaseOnDisk::dropTable"); waitDatabaseStarted(); diff --git a/src/Databases/DatabaseOnDisk.h b/src/Databases/DatabaseOnDisk.h index 4260c0d82730..bd4070a816a7 100644 --- a/src/Databases/DatabaseOnDisk.h +++ b/src/Databases/DatabaseOnDisk.h @@ -48,7 +48,8 @@ class DatabaseOnDisk : public DatabaseWithOwnTablesBase void dropTable( ContextPtr context, const String & table_name, - bool sync) override; + bool sync, + bool if_exists) override; void renameTable( ContextPtr context, diff --git a/src/Databases/DatabaseOverlay.cpp b/src/Databases/DatabaseOverlay.cpp index b8d861a079c1..cb515b5481cd 100644 --- a/src/Databases/DatabaseOverlay.cpp +++ b/src/Databases/DatabaseOverlay.cpp @@ -72,13 +72,13 @@ void DatabaseOverlay::createTable(ContextPtr context_, const String & table_name getEngineName()); } -void DatabaseOverlay::dropTable(ContextPtr context_, const String & table_name, bool sync) +void DatabaseOverlay::dropTable(ContextPtr context_, const String & table_name, bool sync, bool if_exists) { for (auto & db : databases) { if (db->isTableExist(table_name, context_)) { - db->dropTable(context_, table_name, sync); + db->dropTable(context_, table_name, sync, if_exists); return; } } diff --git a/src/Databases/DatabaseOverlay.h b/src/Databases/DatabaseOverlay.h index 72cb8d71b322..74a710ab1d14 100644 --- a/src/Databases/DatabaseOverlay.h +++ b/src/Databases/DatabaseOverlay.h @@ -29,7 +29,7 @@ class DatabaseOverlay : public IDatabase, protected WithContext void createTable(ContextPtr context, const String & table_name, const StoragePtr & table, const ASTPtr & query) override; - void dropTable(ContextPtr context, const String & table_name, bool sync) override; + void dropTable(ContextPtr context, const String & table_name, bool sync, bool if_exists) override; void attachTable(ContextPtr context, const String & table_name, const StoragePtr & table, const String & relative_table_path) override; diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index 27af8b4126be..94d4f393e9fa 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -2198,7 +2198,7 @@ void DatabaseReplicated::shutdown() DatabaseAtomic::shutdown(); } -void DatabaseReplicated::dropTable(ContextPtr local_context, const String & table_name, bool sync) +void DatabaseReplicated::dropTable(ContextPtr local_context, const String & table_name, bool sync, bool /*if_exists*/) { auto component_guard = Coordination::setCurrentComponent("DatabaseReplicated::dropTable"); waitDatabaseStarted(); diff --git a/src/Databases/DatabaseReplicated.h b/src/Databases/DatabaseReplicated.h index a2934f9a6586..7a5b30b4ed9a 100644 --- a/src/Databases/DatabaseReplicated.h +++ b/src/Databases/DatabaseReplicated.h @@ -79,7 +79,7 @@ class DatabaseReplicated : public DatabaseAtomic String getEngineName() const override { return "Replicated"; } /// If current query is initial, then the following methods add metadata updating ZooKeeper operations to current ZooKeeperMetadataTransaction. - void dropTable(ContextPtr, const String & table_name, bool sync) override; + void dropTable(ContextPtr, const String & table_name, bool sync, bool if_exists) override; void renameTable(ContextPtr context, const String & table_name, IDatabase & to_database, const String & to_table_name, bool exchange, bool dictionary) override; void detachTablePermanently(ContextPtr context, const String & table_name) override; diff --git a/src/Databases/IDatabase.cpp b/src/Databases/IDatabase.cpp index 2e62449b6176..7ae3437df500 100644 --- a/src/Databases/IDatabase.cpp +++ b/src/Databases/IDatabase.cpp @@ -144,7 +144,8 @@ void IDatabase::createTable( void IDatabase::dropTable( /// NOLINT ContextPtr /*context*/, const String & /*name*/, - [[maybe_unused]] bool sync) + [[maybe_unused]] bool sync, + [[maybe_unused]] bool if_exists) { throw Exception(ErrorCodes::NOT_IMPLEMENTED, "There is no DROP TABLE query for Database{}", getEngineName()); } diff --git a/src/Databases/IDatabase.h b/src/Databases/IDatabase.h index b92e15ac35d3..4d8ca640dc00 100644 --- a/src/Databases/IDatabase.h +++ b/src/Databases/IDatabase.h @@ -189,6 +189,10 @@ class IDatabase : public std::enable_shared_from_this virtual bool isDatalakeCatalog() const { return false; } + /// Reject an explicitly-specified table engine that is incompatible with this database, before the + /// table is created. + virtual void validateCreateTableEngine(const String & /*engine_name*/) const {} + /// True for databases such as `MySQL`/`PostgreSQL` whose table list lives on a remote service. /// This is distinct from `isExternal`, which classifies whether the engine supports ClickHouse internal table types. virtual bool isRemoteDatabase() const { return false; } @@ -323,7 +327,8 @@ class IDatabase : public std::enable_shared_from_this virtual void dropTable( /// NOLINT ContextPtr /*context*/, const String & /*name*/, - [[maybe_unused]] bool sync = false); + [[maybe_unused]] bool sync = false, + [[maybe_unused]] bool if_exists = false); /// Add a table to the database, but do not add it to the metadata. The database may not support this method. /// diff --git a/src/Databases/MySQL/DatabaseMySQL.cpp b/src/Databases/MySQL/DatabaseMySQL.cpp index a9494fa2abd3..67ff8eb04f15 100644 --- a/src/Databases/MySQL/DatabaseMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMySQL.cpp @@ -533,7 +533,7 @@ void DatabaseMySQL::detachTablePermanently(ContextPtr, const String & table_name table_iter->second.second->is_detached = true; } -void DatabaseMySQL::dropTable(ContextPtr local_context, const String & table_name, bool /*sync*/) +void DatabaseMySQL::dropTable(ContextPtr local_context, const String & table_name, bool /*sync*/, bool /*if_exists*/) { if (!persistent) throw Exception(ErrorCodes::NOT_IMPLEMENTED, "DROP TABLE is not supported for non-persistent MySQL database"); diff --git a/src/Databases/MySQL/DatabaseMySQL.h b/src/Databases/MySQL/DatabaseMySQL.h index 5ac2e10474d4..7fe80687b58a 100644 --- a/src/Databases/MySQL/DatabaseMySQL.h +++ b/src/Databases/MySQL/DatabaseMySQL.h @@ -79,7 +79,7 @@ class DatabaseMySQL final : public DatabaseWithAltersOnDiskBase, WithContext void detachTablePermanently(ContextPtr context, const String & table_name) override; - void dropTable(ContextPtr context, const String & table_name, bool sync) override; + void dropTable(ContextPtr context, const String & table_name, bool sync, bool if_exists) override; void attachTable(ContextPtr context, const String & table_name, const StoragePtr & storage, const String & relative_table_path) override; diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index b17234d69570..89061ead199a 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -391,7 +391,7 @@ void DatabaseMaterializedPostgreSQL::attachTable(ContextPtr context_, const Stri catch (...) { /// This is a failed attach table. Remove already created nested table. - DatabaseAtomic::dropTable(current_context, table_name, true); + DatabaseAtomic::dropTable(current_context, table_name, true, /* if_exists */ false); throw; } } @@ -439,7 +439,7 @@ void DatabaseMaterializedPostgreSQL::detachTablePermanently(ContextPtr, const St { auto current_context = Context::createCopy(getContext()->getGlobalContext()); current_context->makeQueryContext(); - DatabaseAtomic::dropTable(current_context, table_name, true); + DatabaseAtomic::dropTable(current_context, table_name, true, /* if_exists */ false); } catch (Exception & e) { @@ -480,10 +480,10 @@ void DatabaseMaterializedPostgreSQL::stopReplication() } -void DatabaseMaterializedPostgreSQL::dropTable(ContextPtr local_context, const String & table_name, bool sync) +void DatabaseMaterializedPostgreSQL::dropTable(ContextPtr local_context, const String & table_name, bool sync, bool if_exists) { /// Modify context into nested_context and pass query to Atomic database. - DatabaseAtomic::dropTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, sync); + DatabaseAtomic::dropTable(StorageMaterializedPostgreSQL::makeNestedTableContext(local_context), table_name, sync, if_exists); } diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h index d1963b2ec8a6..239914dc584c 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.h @@ -59,7 +59,7 @@ class DatabaseMaterializedPostgreSQL : public DatabaseAtomic StoragePtr detachTable(ContextPtr context, const String & table_name) override; - void dropTable(ContextPtr local_context, const String & name, bool sync) override; + void dropTable(ContextPtr local_context, const String & name, bool sync, bool if_exists) override; void drop(ContextPtr local_context) override; diff --git a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp index e30586e34f5e..6d85ee6f7475 100644 --- a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp @@ -296,7 +296,7 @@ void DatabasePostgreSQL::createTable(ContextPtr local_context, const String & ta } -void DatabasePostgreSQL::dropTable(ContextPtr, const String & table_name, bool /* sync */) +void DatabasePostgreSQL::dropTable(ContextPtr, const String & table_name, bool /* sync */, bool /* if_exists */) { if (!persistent) throw Exception(ErrorCodes::NOT_IMPLEMENTED, "DROP TABLE is not supported for non-persistent MySQL database"); diff --git a/src/Databases/PostgreSQL/DatabasePostgreSQL.h b/src/Databases/PostgreSQL/DatabasePostgreSQL.h index 59d49e37940a..fb218017278f 100644 --- a/src/Databases/PostgreSQL/DatabasePostgreSQL.h +++ b/src/Databases/PostgreSQL/DatabasePostgreSQL.h @@ -54,7 +54,7 @@ class DatabasePostgreSQL final : public DatabaseWithAltersOnDiskBase, WithContex StoragePtr tryGetTable(const String & name, ContextPtr context) const override; void createTable(ContextPtr, const String & table_name, const StoragePtr & storage, const ASTPtr & create_query) override; - void dropTable(ContextPtr, const String & table_name, bool sync) override; + void dropTable(ContextPtr, const String & table_name, bool sync, bool if_exists) override; void attachTable(ContextPtr context, const String & table_name, const StoragePtr & storage, const String & relative_table_path) override; StoragePtr detachTable(ContextPtr context, const String & table_name) override; diff --git a/src/Interpreters/DDLWorker.cpp b/src/Interpreters/DDLWorker.cpp index 81355067db4a..f5ca0071d204 100644 --- a/src/Interpreters/DDLWorker.cpp +++ b/src/Interpreters/DDLWorker.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -572,6 +573,8 @@ bool DDLWorker::tryExecuteQuery(DDLTaskBase & task, const ZooKeeperPtr & zookeep try { + checkQueryDatabasesSupportOnClusterDDL(task.query, context); + auto query_context = task.makeQueryContext(context, zookeeper); chassert(!query_context->getCurrentTransaction()); diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index 0c3eb06684fe..01df31077d01 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -421,6 +421,9 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter) if (table && table->as()) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Mutations with ON CLUSTER are not allowed for KeeperMap tables"); + if (table_id) + checkDatabaseSupportsOnClusterDDL(DatabaseCatalog::instance().tryGetDatabase(table_id.database_name)); + DDLQueryOnClusterParams params; params.access_to_check = getRequiredAccess(); return executeDDLQueryOnCluster(query_ptr, getContext(), params); diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index dcd3f3ea2674..73a80be5a535 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -82,6 +83,7 @@ #include #include +#include #include #include #include @@ -1766,6 +1768,28 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) if (!UserDefinedSQLFunctionFactory::instance().empty()) UserDefinedSQLFunctionVisitor::visit(query_ptr, getContext()); + /// Snapshot whether the user wrote an explicit `ENGINE` before `setEngine` fills in a default below. + const bool engine_user_specified = create.storage && create.storage->engine; + + /// Capture explicit unsupported storage clauses before `setEngine` merges the source's clauses for + /// `CREATE TABLE ... AS` and makes explicit and inherited indistinguishable; rejected below. + /// Engine `SETTINGS` are real data-lake storage settings (e.g. `iceberg_format_version`), so they are + /// unsupported only on the engine-less path, where they would be silently dropped. + const char * datalake_unsupported_storage_clause = nullptr; + if (create.storage) + { + if (create.storage->primary_key) + datalake_unsupported_storage_clause = "PRIMARY KEY"; + else if (create.storage->sample_by) + datalake_unsupported_storage_clause = "SAMPLE BY"; + else if (create.storage->ttl_table) + datalake_unsupported_storage_clause = "TTL"; + else if (create.storage->unique_key) + datalake_unsupported_storage_clause = "UNIQUE KEY"; + else if (create.storage->settings && !engine_user_specified) + datalake_unsupported_storage_clause = "engine SETTINGS"; + } + /// Set and retrieve list of columns, indices and constraints. Set table engine if needed. Rewrite query in canonical way. TableProperties properties = getTablePropertiesAndNormalizeCreateQuery(create, mode); @@ -1838,6 +1862,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) if (!create.cluster.empty()) { + checkDatabaseSupportsOnClusterDDL(database); chassert(!ddl_guard); return executeQueryOnCluster(create); } @@ -1845,6 +1870,76 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) if (need_add_to_database && !database) throw Exception(ErrorCodes::UNKNOWN_DATABASE, "Database {} does not exist", backQuoteIfNeed(database_name)); + if (database && database->isDatalakeCatalog()) + { + if (create.is_ordinary_view || create.is_materialized_view + || create.is_dictionary || create.attach || create.is_clone_as + || create.replace_table || create.replace_view) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, + "DataLakeCatalog supports only plain CREATE TABLE; " + "views, dictionaries, ATTACH, CLONE AS, and REPLACE TABLE are not allowed"); + + if (engine_user_specified) + { + if (!create.storage->engine->name.starts_with("Iceberg")) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "DataLakeCatalog only supports Iceberg-family table engines; got '{}'", + create.storage->engine->name); + + database->validateCreateTableEngine(create.storage->engine->name); + } + + /// For `CREATE TABLE ... AS` the storage is later rebuilt to keep only PARTITION BY / ORDER BY, so + /// these explicit clauses (captured before `setEngine`) can only be rejected here. + if (datalake_unsupported_storage_clause) + { + if (engine_user_specified) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "DataLakeCatalog CREATE TABLE with an explicit table engine supports only " + "PARTITION BY, ORDER BY, and engine SETTINGS; " + "PRIMARY KEY, SAMPLE BY, TTL, and UNIQUE KEY are not supported " + "(got {})", datalake_unsupported_storage_clause); + + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "DataLakeCatalog CREATE TABLE supports only PARTITION BY and ORDER BY; " + "PRIMARY KEY, SAMPLE BY, TTL, UNIQUE KEY, and engine SETTINGS are not supported " + "(got {})", datalake_unsupported_storage_clause); + } + + /// Only column names and types (plus `PARTITION BY` / `ORDER BY`) reach the initial Iceberg + /// metadata, and the table is re-instantiated from the catalog on every access, so any other + /// column property would be silently lost. `properties` also covers columns inherited via + /// `CREATE TABLE ... AS`; `DatabaseDataLake::createTable` re-validates the engine-less path. + for (const auto & column : properties.columns) + { + if (column.default_desc.expression || column.default_desc.kind != ColumnDefaultKind::Default) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Column '{}': {} is not yet supported by DataLakeCatalog table creation", + column.name, toString(column.default_desc.kind)); + + /// Implicit auto-statistics on a `CREATE TABLE ... AS` source do not count as an explicit + /// column property (same distinction as `formatColumns`). + if (!column.comment.empty() || column.codec || column.ttl + || !column.settings.empty() || column.statistics.hasExplicitStatistics()) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Column '{}': COMMENT, CODEC, TTL, STATISTICS, SETTINGS, and PRIMARY KEY " + "are not supported by DataLakeCatalog table creation", + column.name); + } + + if (!properties.indices.empty() || !properties.constraints.empty() || !properties.projections.empty() + || (create.columns_list && (create.columns_list->primary_key || create.columns_list->primary_key_from_columns))) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "DataLakeCatalog CREATE TABLE does not support PRIMARY KEY, indices, constraints, or projections"); + + /// The comment is not persisted in Iceberg metadata or catalog properties, + /// so reject it instead of silently dropping it. + if (create.comment) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Table COMMENT is not supported by DataLakeCatalog table creation " + "(note: CREATE TABLE ... AS inherits the comment from the source table)"); + } + if (create.isTemporary() && create.replace_table) { chassert(!ddl_guard); @@ -1859,7 +1954,30 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create) } /// Actually creates table - bool created = doCreateTable(create, properties, ddl_guard, mode); + bool created = false; + try + { + created = doCreateTable(create, properties, ddl_guard, mode, engine_user_specified); + } + catch (const Exception & e) + { + /// A `DataLakeCatalog` is shared, so the existence check inside `doCreateTable` can be stale by + /// the time the create reaches the catalog. Both create paths report that lost race with + /// `TableAlreadyExistsInCatalogException` and leave nothing behind, so `IF NOT EXISTS` means what + /// it means for the local check: the query created nothing, and `AS SELECT` must not fill the + /// table the winner of the race created. + if (!create.if_not_exists || !dynamic_cast(&e) + || !database || !database->isDatalakeCatalog()) + throw; + /// The query succeeds and no table appears, so record in the log why. + LOG_INFO( + getLogger("InterpreterCreateQuery"), + "CREATE TABLE IF NOT EXISTS {}.{} created nothing: {}", + backQuoteIfNeed(create.getDatabase()), + backQuoteIfNeed(create.getTable()), + e.message()); + created = false; + } ddl_guard.reset(); if (!created) /// Table already exists @@ -1937,7 +2055,7 @@ catch (...) bool InterpreterCreateQuery::doCreateTable(ASTCreateQuery & create, const InterpreterCreateQuery::TableProperties & properties, - DDLGuardPtr & ddl_guard, LoadingStrictnessLevel mode) + DDLGuardPtr & ddl_guard, LoadingStrictnessLevel mode, bool engine_user_specified) { if (create.isTemporary()) { @@ -2034,6 +2152,56 @@ bool InterpreterCreateQuery::doCreateTable(ASTCreateQuery & create, database->checkTableNameLength(create.getTable()); } + auto & create_query = query_ptr->as(); + if (database->isDatalakeCatalog() && !engine_user_specified) + { + if (!as_table_saved.empty()) + { + String as_database_name = getContext()->resolveDatabase(as_database_saved); + StoragePtr as_storage = DatabaseCatalog::instance().getTable({as_database_name, as_table_saved}, getContext()); + auto as_storage_metadata = as_storage->getInMemoryMetadataPtr(getContext(), false); + + /// `setEngine` merged the source's engine, `SETTINGS` and clauses into `create_query.storage`; + /// none apply to a `DataLakeCatalog` table, so rebuild it keeping only the partition and + /// sorting keys - explicit ones take precedence, the source's are the fallback. + ASTPtr partition_by; + ASTPtr order_by; + if (create_query.storage) + { + if (create_query.storage->partition_by) + partition_by = create_query.storage->partition_by->clone(); + if (create_query.storage->order_by) + order_by = create_query.storage->order_by->clone(); + } + + if (!partition_by && as_storage_metadata->isPartitionKeyDefined() && as_storage_metadata->hasPartitionKey()) + partition_by = as_storage_metadata->getPartitionKeyAST()->clone(); + if (!order_by && as_storage_metadata->isSortingKeyDefined() && as_storage_metadata->hasSortingKey()) + order_by = as_storage_metadata->getSortingKeyAST()->clone(); + + auto storage_ast = make_intrusive(); + create_query.set(create_query.storage, storage_ast); + if (partition_by) + create_query.storage->set(create_query.storage->partition_by, partition_by); + if (order_by) + create_query.storage->set(create_query.storage->order_by, order_by); + } + + /// Ensure the columns are in the query AST (mainly for `CREATE TABLE ... AS source`). `formatColumns` + /// keeps every column modifier, so `DatabaseDataLake` can reject the unsupported ones. + if (!create_query.columns_list + || !create_query.columns_list->columns + || create_query.columns_list->columns->children.empty()) + { + auto columns_declare_list = make_intrusive(); + columns_declare_list->set(columns_declare_list->columns, formatColumns(properties.columns)); + create_query.set(create_query.columns_list, columns_declare_list); + } + + database->createTable(getContext(), create.getTable(), nullptr, query_ptr); + return true; + } + data_path = database->getTableDataPath(create); // When creating a table, when checking if the data path exists, it should use the local disk to check, not the database disk. Because the database disk stores metadata files only. auto full_data_path = fs::path{getContext()->getPath()} / data_path; @@ -2146,7 +2314,6 @@ bool InterpreterCreateQuery::doCreateTable(ASTCreateQuery & create, is_restore_from_backup); /// If schema was inferred while storage creation, add columns description to create query. - auto & create_query = query_ptr->as(); addColumnsDescriptionToCreateQueryIfNecessary(create_query, res); /// Add any inferred engine args if needed. For example, data format for engines File/S3/URL/etc if (auto * engine_args = getEngineArgsFromCreateQuery(create_query)) @@ -2327,7 +2494,7 @@ BlockIO InterpreterCreateQuery::doCreateOrReplaceTable(ASTCreateQuery & create, { /// Create temporary table (random name will be generated) DDLGuardPtr ddl_guard; - [[maybe_unused]] bool done = InterpreterCreateQuery(query_ptr, create_context).doCreateTable(create, properties, ddl_guard, mode); + [[maybe_unused]] bool done = InterpreterCreateQuery(query_ptr, create_context).doCreateTable(create, properties, ddl_guard, mode, /*engine_user_specified=*/false); ddl_guard.reset(); chassert(done); created = true; diff --git a/src/Interpreters/InterpreterCreateQuery.h b/src/Interpreters/InterpreterCreateQuery.h index cb50a1cd9aeb..bed1e44de5b1 100644 --- a/src/Interpreters/InterpreterCreateQuery.h +++ b/src/Interpreters/InterpreterCreateQuery.h @@ -108,7 +108,7 @@ class InterpreterCreateQuery : public IInterpreter, WithMutableContext AccessRightsElements getRequiredAccess() const; /// Create IStorage and add it to database. If table already exists and IF NOT EXISTS specified, do nothing and return false. - bool doCreateTable(ASTCreateQuery & create, const TableProperties & properties, DDLGuardPtr & ddl_guard, LoadingStrictnessLevel mode); + bool doCreateTable(ASTCreateQuery & create, const TableProperties & properties, DDLGuardPtr & ddl_guard, LoadingStrictnessLevel mode, bool engine_user_specified); BlockIO doCreateOrReplaceTable(ASTCreateQuery & create, const InterpreterCreateQuery::TableProperties & properties, LoadingStrictnessLevel mode); BlockIO doCreateOrReplaceTemporaryTable(ASTCreateQuery & create, const InterpreterCreateQuery::TableProperties & properties, LoadingStrictnessLevel mode); #if CLICKHOUSE_CLOUD diff --git a/src/Interpreters/InterpreterDropQuery.cpp b/src/Interpreters/InterpreterDropQuery.cpp index 81c275ae7269..ffcf50fef94c 100644 --- a/src/Interpreters/InterpreterDropQuery.cpp +++ b/src/Interpreters/InterpreterDropQuery.cpp @@ -98,6 +98,9 @@ BlockIO InterpreterDropQuery::execute() BlockIO InterpreterDropQuery::executeSingleDropQuery(const ASTPtr & drop_query_ptr) { auto & drop = drop_query_ptr->as(); + if (!drop.cluster.empty() && drop.table) + checkDatabaseSupportsOnClusterDDL( + DatabaseCatalog::instance().tryGetDatabase(getContext()->resolveDatabase(drop.getDatabase()))); if (!drop.cluster.empty() && drop.table && !drop.if_empty && !maybeRemoveOnCluster(current_query_ptr, getContext())) { DDLQueryOnClusterParams params; @@ -347,6 +350,10 @@ BlockIO InterpreterDropQuery::executeToTableImpl(const ContextPtr & context_, AS bool check_loading_deps = !check_ref_deps && getContext()->getSettingsRef()[Setting::check_table_dependencies]; DatabaseCatalog::instance().checkTableCanBeRemovedOrRenamed(table_id, check_ref_deps, check_loading_deps, is_drop_or_detach_database); + /// `drop` can run in a background thread long after this query, so let the storage capture + /// what it needs from the query context now. + table->prepareForDrop(context_); + table->flushAndShutdown(true); TableExclusiveLockHolder table_lock; @@ -355,7 +362,7 @@ BlockIO InterpreterDropQuery::executeToTableImpl(const ContextPtr & context_, AS DatabaseCatalog::instance().removeDependencies(table_id, check_ref_deps, check_loading_deps, is_drop_or_detach_database); NamedCollectionFactory::instance().removeDependencies(table_id); - database->dropTable(context_, table_id.table_name, query.sync); + database->dropTable(context_, table_id.table_name, query.sync, query.if_exists); /// We have to clear mmapio cache when dropping table from Ordinary database /// to avoid reading old data if new table with the same name is created diff --git a/src/Interpreters/executeDDLQueryOnCluster.cpp b/src/Interpreters/executeDDLQueryOnCluster.cpp index a1f76f89cfdc..1817371d4a1f 100644 --- a/src/Interpreters/executeDDLQueryOnCluster.cpp +++ b/src/Interpreters/executeDDLQueryOnCluster.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -16,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -49,6 +51,35 @@ extern const int LOGICAL_ERROR; } +void checkQueryDatabasesSupportOnClusterDDL(const ASTPtr & query_ptr, ContextPtr context) +{ + /// `RENAME` / `EXCHANGE TABLE` is `ASTRenameQuery`, which does not derive from `ASTQueryWithTableAndOutput`; + /// queries with no target table (`CREATE DATABASE`, `SYSTEM`, ...) contribute none. + std::vector target_databases; + const auto add_target_database = [&](String name) + { + target_databases.push_back(name.empty() ? context->getCurrentDatabase() : std::move(name)); + }; + + if (const auto * with_table = dynamic_cast(query_ptr.get()); + with_table && with_table->table) + { + add_target_database(with_table->getDatabase()); + } + else if (const auto * rename = dynamic_cast(query_ptr.get()); rename && !rename->database) + { + for (const auto & elem : rename->getElements()) + { + add_target_database(elem.from.getDatabase()); + add_target_database(elem.to.getDatabase()); + } + } + + for (const auto & database_name : target_databases) + checkDatabaseSupportsOnClusterDDL(DatabaseCatalog::instance().tryGetDatabase(database_name)); +} + + bool isSupportedAlterTypeForOnClusterDDLQuery(int type) { chassert(type != ASTAlterCommand::NO_TYPE); @@ -85,6 +116,9 @@ BlockIO executeDDLQueryOnCluster(const ASTPtr & query_ptr_, ContextPtr context, throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Distributed execution is not supported for such DDL queries"); } + /// Initiator-side guard; workers re-check in `DDLWorker::tryExecuteQuery`. + checkQueryDatabasesSupportOnClusterDDL(query_ptr, context); + if (!context->getSettingsRef()[Setting::allow_distributed_ddl]) throw Exception(ErrorCodes::QUERY_IS_PROHIBITED, "Distributed DDL queries are prohibited for the user"); @@ -224,6 +258,14 @@ BlockIO getDDLOnClusterStatus(const String & node_path, const String & replicas_ return io; } +void checkDatabaseSupportsOnClusterDDL(const DatabasePtr & database) +{ + if (database && database->isDatalakeCatalog()) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, + "ON CLUSTER is not supported for DataLakeCatalog databases: " + "the catalog is shared, run the query without ON CLUSTER"); +} + bool maybeRemoveOnCluster(const ASTPtr & query_ptr, ContextPtr context) { const auto * query = dynamic_cast(query_ptr.get()); diff --git a/src/Interpreters/executeDDLQueryOnCluster.h b/src/Interpreters/executeDDLQueryOnCluster.h index 69e0c38834e6..822cdd84c29d 100644 --- a/src/Interpreters/executeDDLQueryOnCluster.h +++ b/src/Interpreters/executeDDLQueryOnCluster.h @@ -19,10 +19,18 @@ namespace DB struct DDLLogEntry; class Cluster; using ClusterPtr = std::shared_ptr; +class IDatabase; +using DatabasePtr = std::shared_ptr; /// Returns true if provided ALTER type can be executed ON CLUSTER bool isSupportedAlterTypeForOnClusterDDLQuery(int type); +/// Throws if DDL against this database's tables does not support `ON CLUSTER`. +void checkDatabaseSupportsOnClusterDDL(const DatabasePtr & database); + +/// Same, for every database the query targets. +void checkQueryDatabasesSupportOnClusterDDL(const ASTPtr & query_ptr, ContextPtr context); + struct DDLQueryOnClusterParams { /// A cluster to execute a distributed query. diff --git a/src/Storages/IStorage.h b/src/Storages/IStorage.h index a77f425b9f75..bd4527f39065 100644 --- a/src/Storages/IStorage.h +++ b/src/Storages/IStorage.h @@ -520,6 +520,12 @@ It is currently only implemented in StorageObjectStorage. */ virtual void drop() {} + /** Called by `DROP TABLE` while the query is still running. `drop` itself can run much later in a + * background thread, where only the global context is available, so a storage that needs + * query-level settings while dropping (for example `data_lake_delete_data_on_drop`) captures them here. + */ + virtual void prepareForDrop(ContextPtr /* query_context */) {} + virtual void dropInnerTableIfAny(bool /* sync */, ContextPtr /* context */) {} /// Return true if the storage supports TRUNCATE operation. diff --git a/src/Storages/ObjectStorage/DataLakes/DataLakeConfiguration.h b/src/Storages/ObjectStorage/DataLakes/DataLakeConfiguration.h index 92c8bf4cf60a..6095b4dc4ba5 100644 --- a/src/Storages/ObjectStorage/DataLakes/DataLakeConfiguration.h +++ b/src/Storages/ObjectStorage/DataLakes/DataLakeConfiguration.h @@ -343,10 +343,10 @@ class DataLakeConfiguration : public BaseStorageConfiguration, public std::enabl return current_metadata->getColumnMapperForCurrentSchema(storage_metadata_snapshot, context); } - void drop(ContextPtr local_context) override + void drop(bool delete_data) override { if (current_metadata) - current_metadata->drop(local_context); + current_metadata->drop(delete_data); } SinkToStoragePtr write( @@ -853,7 +853,7 @@ class StorageIcebergConfiguration : public StorageObjectStorageConfiguration, pu bool supportsPrewhere() const override { return getImpl().supportsPrewhere(); } - void drop(ContextPtr context) override { getImpl().drop(context); } + void drop(bool delete_data) override { getImpl().drop(delete_data); } protected: void createDynamicConfiguration(ASTs & args, ContextPtr context) diff --git a/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h b/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h index 8784705e85b2..d64e8529f0dd 100644 --- a/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/IDataLakeMetadata.h @@ -287,7 +287,9 @@ class IDataLakeMetadata : boost::noncopyable throwNotImplemented("truncate"); } - virtual void drop(ContextPtr) { } + /// `delete_data` is what `StorageObjectStorage::drop` resolved from `data_lake_delete_data_on_drop`. + /// There is no context here: the drop runs in the background. + virtual void drop(bool /* delete_data */) { } virtual ObjectStorageType getObjectStorageType() const { return ObjectStorageType::None; } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp index 477c22316d9e..6cd49cfba9c6 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Compaction.cpp @@ -437,7 +437,7 @@ static void writeMetadataFiles( auto new_snapshot = metadata_generator.generateNextMetadata( plan.generator, - generated_metadata_info.path, + Iceberg::IcebergPathFromMetadata{}, history_record.parent_id, append->added_files, total_records_count, diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp index 24930b88462a..d9e339ebc2ee 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.cpp @@ -121,6 +121,7 @@ extern const int NOT_IMPLEMENTED; extern const int ICEBERG_SPECIFICATION_VIOLATION; extern const int S3_ERROR; extern const int TABLE_ALREADY_EXISTS; +extern const int FILE_ALREADY_EXISTS; extern const int SUPPORT_IS_DISABLED; extern const int METADATA_MISMATCH; extern const int UNFINISHED; @@ -145,7 +146,6 @@ extern const SettingsBool allow_experimental_iceberg_compaction; extern const SettingsBool allow_experimental_geo_types_in_iceberg; extern const SettingsBool allow_iceberg_remove_orphan_files; extern const SettingsBool allow_experimental_expire_snapshots; -extern const SettingsBool iceberg_delete_data_on_drop; } static constexpr size_t MAX_TRANSACTION_RETRIES = 100; @@ -879,6 +879,11 @@ void IcebergMetadata::createInitial( if (!configuration_ptr) throw Exception(ErrorCodes::LOGICAL_ERROR, "Trying to create Iceberg table, but storage configuration is expired"); + String namespace_name; + String table_name; + if (catalog) + std::tie(namespace_name, table_name) = DataLake::parseTableName(table_id_.getTableName()); + std::vector metadata_files; try { @@ -890,11 +895,22 @@ void IcebergMetadata::createInitial( } if (!metadata_files.empty()) { - if (if_not_exists) - return; - else + /// Without a catalog `IF NOT EXISTS` attaches to the metadata already there. With a catalog + /// nothing gets registered, so success would report an invisible table; the thrown + /// `TableAlreadyExistsInCatalogException` becomes the `IF NOT EXISTS` no-op instead. + if (!catalog) + { + if (if_not_exists) + return; throw Exception( ErrorCodes::TABLE_ALREADY_EXISTS, "Iceberg table with path {} already exists", configuration_ptr->getPathForRead().path); + } + throw DataLake::TableAlreadyExistsInCatalogException( + "The catalog has no table {}.{} registered, but Iceberg metadata files are already present at {}, " + "so creating the table there would clash with them. This is usually left behind by a previous " + "`DROP TABLE` without `data_lake_delete_data_on_drop`, which keeps the data and metadata in " + "place: remove the leftover files, or create the table at a different location", + namespace_name, table_name, configuration_ptr->getPathForRead().path); } String location_path = configuration_ptr->getRawPath().path; @@ -912,7 +928,35 @@ void IcebergMetadata::createInitial( if (!compression_suffix.empty()) compression_suffix = "." + compression_suffix; - auto filename = fmt::format("{}metadata/v1{}.metadata.json", configuration_ptr->getRawPath().path, compression_suffix); + auto table_uuid = metadata_content_object->getValue(Iceberg::f_table_uuid); + auto metadata_file_name = (catalog && catalog->isTransactional()) + ? fmt::format("v1-{}{}.metadata.json", table_uuid, compression_suffix) + : fmt::format("v1{}.metadata.json", compression_suffix); + auto filename = fmt::format("{}metadata/{}", configuration_ptr->getRawPath().path, metadata_file_name); + + if (catalog) + { + /// The namespace default location must be the namespace base, not this table's directory, or + /// later tables created in the same namespace without an explicit location would land under it. + /// The engine clause names an arbitrary path, so the base is only derivable when that path ends + /// with `/
`; otherwise there is no base to offer and the location stays empty. + String namespace_location = location_path; + while (namespace_location.ends_with('/')) + namespace_location.pop_back(); + + String namespace_path = namespace_name; + std::replace(namespace_path.begin(), namespace_path.end(), '.', '/'); + if (namespace_location.ends_with("/" + namespace_path + "/" + table_name)) + namespace_location.resize(namespace_location.size() - table_name.size() - 1); + else + namespace_location.clear(); + + /// Register the namespace before any file is written (but after all local validation, so a + /// rejected `CREATE` leaves no trace in the catalog): a catalog that shares its storage view + /// with the data (e.g. SeaweedFS) refuses to create a namespace over the plain directory + /// those files would leave behind. + catalog->createNamespaceIfNotExists(namespace_name, namespace_location); + } try { @@ -920,27 +964,76 @@ void IcebergMetadata::createInitial( } catch (const Exception & e) { - /// The write uses `If-None-Match: *`, so S3 returns PreconditionFailed when the metadata file - /// already exists (e.g. leftover data after `DROP TABLE` with `iceberg_delete_data_on_drop` off, - /// or a concurrent creation). When `IF NOT EXISTS` was specified, this is expected. - if (if_not_exists && e.code() == ErrorCodes::S3_ERROR - && e.message().find("PreconditionFailed") != String::npos) - return; + /// The write uses `If-None-Match: *`, so S3 answers `PreconditionFailed` when the metadata + /// file is already there: leftovers from an earlier drop, or a concurrent creation. + const bool precondition_failed + = (e.code() == ErrorCodes::S3_ERROR && e.message().contains("PreconditionFailed")) + || e.code() == ErrorCodes::FILE_ALREADY_EXISTS; + if (if_not_exists && precondition_failed) + { + /// As in the `metadata_files` probe above: with a catalog nothing was registered. + if (!catalog) + return; + throw DataLake::TableAlreadyExistsInCatalogException( + "The catalog has no table {}.{} registered, but Iceberg metadata files are already present at {}, " + "so creating the table there would clash with them. This is usually left behind by a previous " + "`DROP TABLE` without `data_lake_delete_data_on_drop`, which keeps the data and metadata in " + "place: remove the leftover files, or create the table at a different location", + namespace_name, table_name, configuration_ptr->getPathForRead().path); + } throw; } - if (configuration_ptr->getDataLakeSettings()[DataLakeStorageSetting::iceberg_use_version_hint].value) + String filename_version_hint; + try { - auto filename_version_hint = configuration_ptr->getRawPath().path + "metadata/version-hint.text"; - writeMessageToFile("1", filename_version_hint, object_storage, local_context, "*", ""); + if (configuration_ptr->getDataLakeSettings()[DataLakeStorageSetting::iceberg_use_version_hint].value) + { + auto version_hint_path = configuration_ptr->getRawPath().path + "metadata/version-hint.text"; + writeMessageToFile("1", version_hint_path, object_storage, local_context, "*", ""); + filename_version_hint = version_hint_path; + } + } + catch (...) + { + /// Nothing is registered in the catalog yet, so removing the files we just wrote loses nothing, + /// and a leftover `.metadata.json` would make the next `CREATE` report an existing table. The + /// rollback fails closed: a failed removal propagates in place of the original exception, which + /// is logged here. + tryLogCurrentException(__PRETTY_FUNCTION__, "Removing the files of the Iceberg table that failed to be created"); + object_storage->removeObjectIfExists(StoredObject(filename)); + if (!filename_version_hint.empty()) + object_storage->removeObjectIfExists(StoredObject(filename_version_hint)); + throw; } if (catalog) { auto catalog_filename = configuration_ptr->getTypeName() + "://" + configuration_ptr->getNamespace() + "/" - + configuration_ptr->getRawPath().path + "metadata/v1.metadata.json"; - const auto & [namespace_name, table_name] = DataLake::parseTableName(table_id_.getTableName()); - catalog->createTable(namespace_name, table_name, catalog_filename, metadata_content_object); + + configuration_ptr->getRawPath().path + "metadata/" + metadata_file_name; + + /// The registration sits outside any rollback: a failed `createTable` is ambiguous, because the + /// HTTP layer retries connection failures and the request may have succeeded with only its response + /// lost. Removing the metadata file would then leave the catalog pointing at an object that is gone, + /// with no earlier version to fall back to; an orphaned file is recoverable, so it stays. + if (!catalog->createTable(namespace_name, table_name, catalog_filename, metadata_content_object, compression_method, if_not_exists)) + { + /// Unlike an exception, this answer is definitive: the catalog did not create the table, so the + /// files we wrote are ours to remove - a transactional catalog's file name carries our own table + /// UUID, and otherwise the `If-None-Match: *` write already proved that we created them. + LOG_INFO( + getLogger("IcebergMetadata"), + "Table {}.{} was registered in the catalog by another client, removing the initial metadata file {} " + "written by this `CREATE`", + namespace_name, + table_name, + filename); + object_storage->removeObjectIfExists(StoredObject(filename)); + if (!filename_version_hint.empty()) + object_storage->removeObjectIfExists(StoredObject(filename_version_hint)); + throw DataLake::TableAlreadyExistsInCatalogException( + "Table {}.{} already exists in the catalog", namespace_name, table_name); + } } } @@ -1537,11 +1630,15 @@ SinkToStoragePtr IcebergMetadata::write( } } -void IcebergMetadata::drop(ContextPtr context) +void IcebergMetadata::drop(bool delete_data) { - if (!context->getSettingsRef()[Setting::iceberg_delete_data_on_drop].value) + if (!delete_data) return; + /// The drop runs in the background, when the query context is already gone; `delete_data` was + /// resolved from it by `StorageObjectStorage::drop`. + auto context = Context::getGlobalContextInstance(); + /// Files outside `table_path` (secondary storage, or base storage elsewhere in the bucket) are only /// discoverable through the metadata graph the base wipe below removes, so enumerate them first. Let /// a failure propagate rather than wiping the metadata re-enumeration on retry depends on (fail closed). diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h index b28d476438da..b7f378a52997 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergMetadata.h @@ -218,7 +218,7 @@ class IcebergMetadata : public IDataLakeMetadata StorageMetadataPtr storage_metadata, ContextPtr local_context) const override; - void drop(ContextPtr context) override; + void drop(bool delete_data) override; Poco::JSON::Object::Ptr getMetadataJSON(ContextPtr local_context) const; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h index 26cb3a564d0f..9d8878f394fc 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergPath.h @@ -89,6 +89,8 @@ class IcebergPathResolver IcebergPathFromMetadata reverseResolve(const String & storage_path) const { + if (!table_location.empty() && storage_path.starts_with(table_location)) + return IcebergPathFromMetadata::deserialize(storage_path); if (storage_path.size() > table_root.size() && storage_path.starts_with(table_root)) return IcebergPathFromMetadata::deserialize(table_location + storage_path.substr(table_root.size())); diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp index 5ddc5ff3744b..576c0cd2641a 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp @@ -918,7 +918,7 @@ void generateManifestList( writer.write(entry_datum); } - if (use_previous_snapshots) + if (use_previous_snapshots && new_snapshot->has(Iceberg::f_parent_snapshot_id)) { auto parent_snapshot_id = new_snapshot->getValue(Iceberg::f_parent_snapshot_id); auto snapshots = metadata->getArray(Iceberg::f_snapshots); @@ -1039,6 +1039,7 @@ IcebergStorageSink::IcebergStorageSink( compression_method, persistent_table_components.table_uuid); metadata_compression_method = compression_method; + previous_metadata_file_path = metadata_path; filename_generator = FileNamesGenerator( persistent_table_components.path_resolver.getTableLocation(), (catalog != nullptr && catalog->isTransactional()), metadata_compression_method, write_format); @@ -1264,7 +1265,7 @@ bool IcebergStorageSink::initializeMetadata() total_data_files += static_cast(writer.getDataFiles().size()); auto [new_snapshot, manifest_list_path] = MetadataGenerator(metadata).generateNextMetadata( filename_generator, - metadata_info.path, + previous_metadata_file_path.empty() ? Iceberg::IcebergPathFromMetadata{} : resolver.reverseResolve(previous_metadata_file_path), parent_snapshot, total_data_files, total_rows, @@ -1313,6 +1314,7 @@ bool IcebergStorageSink::initializeMetadata() LOG_DEBUG(log, "Rereading metadata file {} with version {}", metadata_path, last_version); metadata_compression_method = compression_method; + previous_metadata_file_path = metadata_path; filename_generator.setVersion(last_version + 1); metadata = getMetadataJSONObject( diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h index 988aece81874..84ab625257bb 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.h @@ -158,6 +158,7 @@ class IcebergStorageSink final : public SinkToStorage bool initializeMetadata(); FileNamesGenerator filename_generator; + String previous_metadata_file_path; std::optional partitioner; Poco::JSON::Object::Ptr partititon_spec; Int64 partition_spec_id; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.cpp index 55ee1c99baf3..23f7ef7f2363 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.cpp @@ -343,7 +343,7 @@ Poco::JSON::Object::Ptr MetadataGenerator::getParentSnapshot(Int64 parent_snapsh MetadataGenerator::NextMetadataResult MetadataGenerator::generateNextMetadata( FileNamesGenerator & generator, - const Iceberg::IcebergPathFromMetadata & metadata_file_path, + const Iceberg::IcebergPathFromMetadata & previous_metadata_file_path, Int64 parent_snapshot_id, Int64 added_files, Int64 added_records, @@ -367,7 +367,8 @@ MetadataGenerator::NextMetadataResult MetadataGenerator::generateNextMetadata( auto manifest_list_path = generator.generateManifestListName(snapshot_id, format_version); new_snapshot->set(Iceberg::f_metadata_snapshot_id, snapshot_id); - new_snapshot->set(Iceberg::f_parent_snapshot_id, parent_snapshot_id); + if (parent_snapshot_id != -1) + new_snapshot->set(Iceberg::f_parent_snapshot_id, parent_snapshot_id); auto now = std::chrono::system_clock::now(); auto ms = duration_cast(now.time_since_epoch()); @@ -453,9 +454,10 @@ MetadataGenerator::NextMetadataResult MetadataGenerator::generateNextMetadata( else metadata_object->getObject(Iceberg::f_refs)->getObject(Iceberg::f_main)->set(Iceberg::f_metadata_snapshot_id, snapshot_id); + if (!previous_metadata_file_path.empty()) { Poco::JSON::Object::Ptr new_metadata_item = new Poco::JSON::Object; - new_metadata_item->set(Iceberg::f_metadata_file, metadata_file_path.serialize()); + new_metadata_item->set(Iceberg::f_metadata_file, previous_metadata_file_path.serialize()); new_metadata_item->set(Iceberg::f_timestamp_ms, timestamp); getOrCreateArray(metadata_object, Iceberg::f_metadata_log)->add(new_metadata_item); } diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.h b/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.h index a20c1cfdc827..bf21fbcefe58 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.h +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/MetadataGenerator.h @@ -31,7 +31,7 @@ class MetadataGenerator NextMetadataResult generateNextMetadata( FileNamesGenerator & generator, - const Iceberg::IcebergPathFromMetadata & metadata_file_path, + const Iceberg::IcebergPathFromMetadata & previous_metadata_file_path, Int64 parent_snapshot_id, Int64 added_files, Int64 added_records, diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp index 2933f50f9296..5e80c8df477a 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Mutations.cpp @@ -437,12 +437,13 @@ static bool writeMetadataFiles( std::optional & chunk_partitioner, Iceberg::FileContentType content_type, SharedHeader sample_block, - bool write_metadata_json_file) + bool write_metadata_json_file, + const Iceberg::IcebergPathFromMetadata & previous_metadata_file_path) { auto metadata_info = filename_generator.generateMetadataPathWithInfo(); auto storage_metadata_name = path_resolver.resolve(metadata_info.path); Int64 parent_snapshot = -1; - if (metadata->has(Iceberg::f_current_snapshot_id)) + if (metadata->has(Iceberg::f_current_snapshot_id) && !metadata->isNull(Iceberg::f_current_snapshot_id)) parent_snapshot = metadata->getValue(Iceberg::f_current_snapshot_id); Int64 total_rows = 0; @@ -461,7 +462,7 @@ static bool writeMetadataFiles( { auto result = MetadataGenerator(metadata).generateNextMetadata( filename_generator, - metadata_info.path, + previous_metadata_file_path, parent_snapshot, /* added_files */ 0, /* added_records */ 0, @@ -476,7 +477,7 @@ static bool writeMetadataFiles( { auto result = MetadataGenerator(metadata).generateNextMetadata( filename_generator, - metadata_info.path, + previous_metadata_file_path, parent_snapshot, /* added_files */ total_files, /* added_records */ total_rows, @@ -672,6 +673,7 @@ void mutate( filename_generator.setCompressionMethod(compression_method); auto metadata = getMetadataJSONObject(metadata_path, object_storage, persistent_table_components.metadata_cache, context, log, compression_method, persistent_table_components.table_uuid); + auto previous_metadata_path = persistent_table_components.path_resolver.reverseResolve(metadata_path); /// Iceberg v3 writers must not add new position-delete files; row-level deletes require /// deletion vectors. Fail closed before any object writes until ClickHouse can write DVs. @@ -715,7 +717,7 @@ void mutate( current_iceberg_snapshot.metadata_file_path = metadata_path; current_iceberg_snapshot.metadata_version = last_version; current_iceberg_snapshot.schema_id = static_cast(current_schema_id); - if (metadata->has(Iceberg::f_current_snapshot_id)) + if (metadata->has(Iceberg::f_current_snapshot_id) && !metadata->isNull(Iceberg::f_current_snapshot_id)) { Int64 snapshot_id_val = metadata->getValue(Iceberg::f_current_snapshot_id); if (snapshot_id_val >= 0) @@ -761,7 +763,8 @@ void mutate( chunk_partitioner, Iceberg::FileContentType::POSITION_DELETE, std::make_shared(getPositionDeleteFileSampleBlock()), - !mutation_files->data_file); + !mutation_files->data_file, + previous_metadata_path); if (!result_delete_files_metadata) continue; @@ -784,7 +787,8 @@ void mutate( chunk_partitioner, Iceberg::FileContentType::DATA, sample_block, - true); + true, + previous_metadata_path); if (!result_data_files_metadata) { continue; diff --git a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp index 48f088cbe96e..3112fb0af552 100644 --- a/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp +++ b/src/Storages/ObjectStorage/DataLakes/Iceberg/Utils.cpp @@ -376,7 +376,19 @@ bool writeMetadataFileAndVersionHint( } else { - break; + /// Remove the metadata file written above, otherwise version-hint resolution could later + /// pick this uncommitted file as the latest version. Deliberately not wrapped in a + /// try/catch: a failed removal must fail the whole operation instead of returning a + /// retriable `false` while the uncommitted `vN-.metadata.json` is still there. + LOG_INFO( + getLogger("IcebergMetadataFileWriter"), + "Removing the uncommitted Iceberg metadata file {}: the version hint is already at version {}, " + "at or past version {} this commit tried to write, so the write did not commit", + storage_metadata_path, + old_version, + metadata_file_info.version); + object_storage->removeObjectIfExists(StoredObject(storage_metadata_path)); + return false; } ++i; } @@ -793,6 +805,9 @@ static Poco::JSON::Object::Ptr getPartitionField( { if (!param.has_value()) throw Exception(ErrorCodes::BAD_ARGUMENTS, "TRUNCATE function for iceberg partitioning requires one integer parameter"); + /// The Iceberg spec requires the truncate width to be a positive integer. + if (*param <= 0) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "TRUNCATE function for iceberg partitioning requires a positive width, got {}", *param); result->set(Iceberg::f_transform, fmt::format("truncate[{}]", *param)); return result; } @@ -800,6 +815,9 @@ static Poco::JSON::Object::Ptr getPartitionField( { if (!param.has_value()) throw Exception(ErrorCodes::BAD_ARGUMENTS, "BUCKET function for iceberg partitioning requires one integer parameter"); + /// The Iceberg spec requires the number of buckets to be a positive integer. + if (*param <= 0) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "BUCKET function for iceberg partitioning requires a positive number of buckets, got {}", *param); result->set(Iceberg::f_transform, fmt::format("bucket[{}]", *param)); return result; } diff --git a/src/Storages/ObjectStorage/StorageObjectStorage.cpp b/src/Storages/ObjectStorage/StorageObjectStorage.cpp index abd0cb896222..c2d383909299 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorage.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorage.cpp @@ -55,6 +55,7 @@ namespace Setting extern const SettingsInt64 delta_lake_snapshot_start_version; extern const SettingsInt64 delta_lake_snapshot_end_version; extern const SettingsUInt64 max_streams_for_files_processing_in_cluster_functions; + extern const SettingsBool data_lake_delete_data_on_drop; } namespace ErrorCodes @@ -849,15 +850,40 @@ void StorageObjectStorage::truncate( object_storage->removeObjectsIfExist(objects); } +void StorageObjectStorage::prepareForDrop(ContextPtr query_context) +{ + /// `drop` runs in the background, when the query context is gone, so a per-query or per-session + /// `data_lake_delete_data_on_drop` has to be read while the `DROP TABLE` query is still running. + delete_data_on_drop = query_context->getSettingsRef()[Setting::data_lake_delete_data_on_drop]; +} + void StorageObjectStorage::drop() { + /// No query context here, because `drop` runs in the background. `prepareForDrop` captured the value + /// if this drop came from a `DROP TABLE` query; without a capture we keep the data, because deleting + /// it is irreversible while an orphaned data directory can still be removed later. + const std::optional captured_delete_data = delete_data_on_drop.load(); + const bool delete_data = captured_delete_data.value_or(false); + + if (!captured_delete_data + && Context::getGlobalContextInstance()->getSettingsRef()[Setting::data_lake_delete_data_on_drop]) + { + LOG_WARNING( + log, + "Keeping the data of table {} although `data_lake_delete_data_on_drop` is enabled server-wide: the value for this drop " + "could not be captured, which happens when the table was never loaded, and data is never deleted on a fallback path. " + "Access the table before dropping it, so that the settings of the `DROP TABLE` query reach the table.", + storage_id.getNameForLogs()); + } + if (catalog) { const auto [namespace_name, table_name] = DataLake::parseTableName(storage_id.getTableName()); - catalog->dropTable(namespace_name, table_name); + /// This runs in the background, after the query has finished, so a missing table cannot be + /// reported as a no-op to the user anyway: keep reporting it as an error in the log. + catalog->dropTable(namespace_name, table_name, delete_data, /* if_exists */ false); } - /// We cannot use query context here, because drop is executed in the background. - configuration->drop(Context::getGlobalContextInstance()); + configuration->drop(delete_data); } std::unique_ptr StorageObjectStorage::createReadBufferIterator( diff --git a/src/Storages/ObjectStorage/StorageObjectStorage.h b/src/Storages/ObjectStorage/StorageObjectStorage.h index 5f1db8f2a527..cefaecd4d839 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorage.h +++ b/src/Storages/ObjectStorage/StorageObjectStorage.h @@ -18,7 +18,9 @@ #include #include +#include #include +#include #include namespace DB @@ -110,6 +112,8 @@ class StorageObjectStorage : public IStorage, public IBackgroundOperation void drop() override; + void prepareForDrop(ContextPtr query_context) override; + bool supportsPartitionBy() const override { return true; } bool supportsSubcolumns() const override { return true; } @@ -258,6 +262,11 @@ class StorageObjectStorage : public IStorage, public IBackgroundOperation std::shared_ptr catalog; StorageID storage_id; BackgroundJobsAssignee background_operations_assignee; + + /// `data_lake_delete_data_on_drop` as it was set for the `DROP TABLE` query, captured by + /// `prepareForDrop` because `drop` runs without a query context. Stays empty when the drop does not + /// come from a `DROP TABLE` query, and `drop` then keeps the data rather than guessing. + std::atomic> delete_data_on_drop; }; } diff --git a/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp b/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp index 9eefd709aba1..7eee9e8796f3 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp +++ b/src/Storages/ObjectStorage/StorageObjectStorageCluster.cpp @@ -777,6 +777,15 @@ void StorageObjectStorageCluster::drop() IStorageCluster::drop(); } +/// `drop` forwards to `pure_storage`, so the capture `drop` reads has to land on the same object. +/// Without this the query-level `data_lake_delete_data_on_drop` never reaches the storage and the +/// data of a dropped table is kept. +void StorageObjectStorageCluster::prepareForDrop(ContextPtr query_context) +{ + if (pure_storage) + pure_storage->prepareForDrop(query_context); +} + void StorageObjectStorageCluster::dropInnerTableIfAny(bool sync, ContextPtr context) { if (getClusterName(context).empty()) diff --git a/src/Storages/ObjectStorage/StorageObjectStorageCluster.h b/src/Storages/ObjectStorage/StorageObjectStorageCluster.h index 6894bb76d2e1..fd9e92671a71 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageCluster.h +++ b/src/Storages/ObjectStorage/StorageObjectStorageCluster.h @@ -74,6 +74,8 @@ class StorageObjectStorageCluster : public IStorageCluster void drop() override; + void prepareForDrop(ContextPtr query_context) override; + void dropInnerTableIfAny(bool sync, ContextPtr context) override; void truncate( diff --git a/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h b/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h index c1af64950396..543683246cd8 100644 --- a/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h +++ b/src/Storages/ObjectStorage/StorageObjectStorageConfiguration.h @@ -338,7 +338,9 @@ class StorageObjectStorageConfiguration return true; } - virtual void drop(ContextPtr) {} + /// `delete_data` is what `StorageObjectStorage::drop` resolved from `data_lake_delete_data_on_drop`. + /// There is no context here: the drop runs in the background. + virtual void drop(bool /* delete_data */) {} virtual bool isBackgroundExecutable() const { diff --git a/src/Storages/StorageProxy.h b/src/Storages/StorageProxy.h index e3ad0474a062..c78ae3f67f68 100644 --- a/src/Storages/StorageProxy.h +++ b/src/Storages/StorageProxy.h @@ -84,6 +84,8 @@ class StorageProxy : public IStorage return getNested()->write(query, metadata_snapshot, context, async_insert); } + /// `prepareForDrop` is not forwarded here; only the proxies that hold a nested storage + /// (`StorageTableProxy`, `StorageTableFunctionProxy`) forward it, and only once it is resolved. void drop() override { getNested()->drop(); } void truncate( diff --git a/src/Storages/StorageTableFunction.h b/src/Storages/StorageTableFunction.h index 884908cf0ccf..ef7044ff9af9 100644 --- a/src/Storages/StorageTableFunction.h +++ b/src/Storages/StorageTableFunction.h @@ -85,6 +85,13 @@ class StorageTableFunctionProxy final : public StorageProxy nested->drop(); } + void prepareForDrop(ContextPtr query_context) override + { + std::lock_guard lock{nested_mutex}; + if (nested) + nested->prepareForDrop(query_context); + } + void read( QueryPlan & query_plan, const Names & column_names, diff --git a/src/Storages/StorageTableProxy.h b/src/Storages/StorageTableProxy.h index e935de70e221..8c481b7c621c 100644 --- a/src/Storages/StorageTableProxy.h +++ b/src/Storages/StorageTableProxy.h @@ -79,6 +79,15 @@ class StorageTableProxy final : public StorageProxy nested->flushAndPrepareForShutdown(); } + void prepareForDrop(ContextPtr query_context) override + { + std::lock_guard lock{nested_mutex}; + /// Forwarded only to an already-resolved nested storage. A storage that receives no capture must + /// not infer a destructive action from a server-level default (see `StorageObjectStorage::drop`). + if (nested) + nested->prepareForDrop(query_context); + } + void drop() override { std::lock_guard lock{nested_mutex}; diff --git a/tests/integration/test_database_glue/test.py b/tests/integration/test_database_glue/test.py index 665aee5cd205..c2b8bb2342ea 100644 --- a/tests/integration/test_database_glue/test.py +++ b/tests/integration/test_database_glue/test.py @@ -724,6 +724,102 @@ def test_create(started_cluster): assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{root_namespace}.{table_name}`") == "AAPL\n" +def test_native_create_gzip_metadata(started_cluster): + # The engine-less `CREATE TABLE` path does not go through `IcebergMetadata::createInitial`: + # `GlueCatalog::createTable` writes and registers the first metadata file itself, and must honour + # `iceberg_metadata_compression_method` just like the explicit engine path (`test_create_gzip_metadata`). + node = started_cluster.instances["node1"] + + test_ref = f"test_native_create_gzip_{uuid.uuid4()}" + table_name = f"{test_ref}_table" + root_namespace = f"{test_ref}_namespace" + + create_clickhouse_glue_database(started_cluster, node, CATALOG_NAME) + + node.query( + f"CREATE TABLE {CATALOG_NAME}.`{root_namespace}.{table_name}` (x String)", + settings={ + "allow_experimental_database_glue_catalog": 1, + "allow_database_glue_catalog": 1, + "write_full_path_in_iceberg_metadata": 1, + "iceberg_metadata_compression_method": "gzip", + }, + ) + + glue_client = boto3.client( + "glue", region_name="us-east-1", endpoint_url=get_glue_local_url(started_cluster) + ) + table_info = glue_client.get_table(DatabaseName=root_namespace, Name=table_name)["Table"] + metadata_location = table_info["Parameters"]["metadata_location"] + # The file is named with the compression token, exactly as `IcebergMetadata::createInitial` names it. + assert metadata_location.endswith(".gzip.metadata.json"), metadata_location + + # The registered file must exist and its contents must really be gzip, not just carry the name. + assert metadata_location.startswith("s3://"), metadata_location + bucket, _, key = metadata_location[len("s3://") :].partition("/") + metadata_bytes = started_cluster.minio_client.get_object(bucket, key).read() + assert metadata_bytes[:2] == b"\x1f\x8b", metadata_bytes[:16] + + create_clickhouse_glue_database(started_cluster, node, CATALOG_NAME) + assert node.query(f"SELECT count() FROM {CATALOG_NAME}.`{root_namespace}.{table_name}`") == "0\n" + node.query( + f"INSERT INTO {CATALOG_NAME}.`{root_namespace}.{table_name}` VALUES ('AAPL');", + settings={ + "allow_insert_into_iceberg": 1, + "write_full_path_in_iceberg_metadata": 1, + "iceberg_metadata_compression_method": "gzip", + }, + ) + assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{root_namespace}.{table_name}`") == "AAPL\n" + + +def test_create_table_engine_backend_mismatch_rejected(started_cluster): + # Glue has a fixed S3 backend and reopens every table with it, so an explicit Iceberg engine + # pinning a different backend must be rejected up front instead of yielding an unreadable table. + node = started_cluster.instances["node1"] + + test_ref = f"test_engine_backend_mismatch_{uuid.uuid4()}" + root_namespace = f"{test_ref}_namespace" + + create_clickhouse_glue_database(started_cluster, node, CATALOG_NAME) + + for engine in [ + "IcebergAzure('http://acc.blob.core.windows.net/cont/tbl/', 'acc', 'key')", + "IcebergLocal('/var/lib/clickhouse/user_files/tbl/')", + "IcebergHDFS('hdfs://namenode:9000/tbl/')", + ]: + error = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`{root_namespace}.mismatch` (x String) ENGINE = {engine}", + settings={ + "allow_experimental_database_glue_catalog": 1, + "allow_database_glue_catalog": 1, + }, + ) + assert ( + "would be reopened with the catalog's storage backend and become unreadable" in error + ), error + assert "stores tables on S3" in error, error + + # Positive control: the matching S3 engine is accepted. + create_clickhouse_glue_table( + started_cluster, node, root_namespace, f"{test_ref}_s3", "(x String)" + ) + assert ( + node.query(f"SELECT count() FROM {CATALOG_NAME}.`{root_namespace}.{test_ref}_s3`") == "0\n" + ) + + error = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`{root_namespace}.{test_ref}_generic` (x String) " + f"ENGINE = Iceberg('http://minio1:9001/warehouse-glue/{test_ref}_generic/', " + f"'{minio_access_key}', '{minio_secret_key}')", + settings={ + "allow_experimental_database_glue_catalog": 1, + "write_full_path_in_iceberg_metadata": 1, + }, + ) + assert "generic 'Iceberg' engine is not supported" in error, error + + def test_schema_evolution(started_cluster): node = started_cluster.instances["node1"] @@ -807,6 +903,14 @@ def test_drop_table(started_cluster): assert len(catalog.list_tables(root_namespace)) == 1 assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{root_namespace}.{table_name}`") == "" + # ClickHouse's Glue drop only removes the catalog entry and does not delete the data files, so a purge + # drop is rejected and the table is left intact. + error = node.query_and_get_error( + f"DROP TABLE {CATALOG_NAME}.`{root_namespace}.{table_name}` SETTINGS data_lake_delete_data_on_drop = 1" + ) + assert "not supported for the Glue catalog" in error + assert len(catalog.list_tables(root_namespace)) == 1 + drop_clickhouse_glue_table(node, root_namespace, table_name) assert len(catalog.list_tables(root_namespace)) == 0 diff --git a/tests/integration/test_database_iceberg/test.py b/tests/integration/test_database_iceberg/test.py index 5ec77a1a8fbd..9579c2bba6ff 100644 --- a/tests/integration/test_database_iceberg/test.py +++ b/tests/integration/test_database_iceberg/test.py @@ -11,6 +11,7 @@ import pytest import requests import pytz +from minio import Minio from pyiceberg.catalog import load_catalog from pyiceberg.partitioning import PartitionField, PartitionSpec, UNPARTITIONED_PARTITION_SPEC from pyiceberg.schema import Schema @@ -195,6 +196,7 @@ def started_cluster(): user_configs=[], stay_alive=True, with_iceberg_catalog=True, + with_zookeeper=True, ) cluster.add_instance( @@ -1006,6 +1008,67 @@ def test_cluster_select(started_cluster): assert node2.query(f"SELECT * FROM {CATALOG_NAME}.`{root_namespace}.{table_name}`", settings={"parallel_replicas_for_cluster_engines": 1, "enable_parallel_replicas": 2, "cluster_for_parallel_replicas": "cluster_simple"}) == 'pablo\n' +def test_on_cluster_ddl_rejected_for_datalake_catalog(started_cluster): + # ON CLUSTER DDL against a shared DataLakeCatalog must be rejected on the worker as well as the initiator, + # because an initiator that lacks the catalog database locally cannot detect it and enqueues the query anyway. + node1 = started_cluster.instances["node1"] + node2 = started_cluster.instances["node2"] + + test_ref = f"test_on_cluster_ban_{uuid.uuid4().hex}" + table_name = f"{test_ref}_table" + namespace = f"{test_ref}_namespace" + qualified = f"{CATALOG_NAME}.`{namespace}.{table_name}`" + engine = ( + f"ENGINE = IcebergS3('http://minio1:9001/warehouse-rest/{table_name}/', " + f"'{minio_access_key}', '{minio_secret_key}')" + ) + ddl_settings = { + "allow_experimental_database_iceberg": 1, + "allow_database_iceberg": 1, + "write_full_path_in_iceberg_metadata": 1, + "distributed_ddl_output_mode": "throw", + } + + # Initiator has the catalog locally: rejected centrally before the query is ever enqueued. + create_clickhouse_iceberg_database(started_cluster, node1, CATALOG_NAME) + create_clickhouse_iceberg_database(started_cluster, node2, CATALOG_NAME) + err = node1.query_and_get_error( + f"CREATE TABLE {qualified} ON CLUSTER cluster_simple (x String) {engine}", + settings=ddl_settings, + ) + assert "ON CLUSTER is not supported for DataLakeCatalog" in err, err + + # Initiator lacks the catalog, but a worker (node1) has it, so only the worker guard can fire. The plain + # ON CLUSTER control below confirms the query reaches the workers, ruling out an unrelated failure. + node2.query(f"DROP DATABASE IF EXISTS {CATALOG_NAME}") + try: + control_table = f"{test_ref}_control" + node2.query( + f"CREATE TABLE default.{control_table} ON CLUSTER cluster_simple (x Int32) ENGINE = MergeTree ORDER BY x", + settings={"distributed_ddl_output_mode": "throw"}, + ) + assert node1.query(f"EXISTS TABLE default.{control_table}") == "1\n" + node2.query( + f"DROP TABLE default.{control_table} ON CLUSTER cluster_simple SYNC", + settings={"distributed_ddl_output_mode": "throw"}, + ) + + node2.query_and_get_error( + f"CREATE TABLE {qualified} ON CLUSTER cluster_simple (x String) {engine}", + settings=ddl_settings, + ) + + # The shared catalog must be untouched: without the worker guard node1 would have created the table here. + catalog = load_catalog_impl(started_cluster) + existing_namespaces = {".".join(ns) for ns in catalog.list_namespaces()} + if namespace in existing_namespaces: + tables = {ident[-1] for ident in catalog.list_tables(namespace)} + assert table_name not in tables, f"table must not have been created in the shared catalog: {tables}" + finally: + # Restore the catalog database on node2 for the tests that follow. + create_clickhouse_iceberg_database(started_cluster, node2, CATALOG_NAME) + + def test_used_storages_in_query_log(started_cluster): node1 = started_cluster.instances["node1"] node2 = started_cluster.instances["node2"] @@ -1169,6 +1232,691 @@ def test_system_tables_with_nullptr_table(started_cluster): node.query(f"DROP DATABASE IF EXISTS {CATALOG_NAME}") + +def test_create_table_as(started_cluster): + node = started_cluster.instances["node1"] + + namespace = "test_ctas_ns" + src_table = "src_ctas" + catalog = load_catalog_impl(started_cluster) + + create_clickhouse_iceberg_database( + started_cluster, + node, + CATALOG_NAME, + additional_settings={"default_base_location": "s3://warehouse-rest/data"}, + ) + + node.query(f"DROP TABLE IF EXISTS default.{src_table}") + for table in ["from_as", "override"]: + node.query( + f"DROP TABLE IF EXISTS {CATALOG_NAME}.`{namespace}.{table}` SETTINGS allow_database_iceberg=1" + ) + + # CTAS must work from a source with a functional partition key. + node.query( + f""" + CREATE TABLE default.{src_table} + ( + id Int64, + name String, + dt Date + ) + ENGINE = MergeTree + PARTITION BY toYearNumSinceEpoch(dt) + ORDER BY (id, name) + """ + ) + + node.query( + f""" + CREATE TABLE {CATALOG_NAME}.`{namespace}.from_as` + AS default.{src_table} SETTINGS allow_database_iceberg=1; + """ + ) + + node.query( + f""" + CREATE TABLE {CATALOG_NAME}.`{namespace}.override` + AS default.{src_table} + PARTITION BY id + ORDER BY name + SETTINGS allow_database_iceberg=1; + """ + ) + + tables = catalog.list_tables(namespace) + table_names = [t[1] for t in tables] + assert "from_as" in table_names + assert "override" in table_names + + tbl = catalog.load_table(f"{namespace}.from_as") + col_names = [f.name for f in tbl.schema().fields] + assert col_names == ["id", "name", "dt"] + + tbl = catalog.load_table(f"{namespace}.override") + assert len(tbl.spec().fields) == 1 + assert tbl.spec().fields[0].name == "id" + assert str(tbl.spec().fields[0].transform) == "identity" + + col_names = [f.name for f in tbl.schema().fields] + assert col_names == ["id", "name", "dt"] + + # Unsupported storage clauses written explicitly on CREATE ... AS must be rejected, not silently dropped + # when the storage is rebuilt to keep only PARTITION BY / ORDER BY. + for clause in [ + "PRIMARY KEY id", + "SAMPLE BY id", + "TTL toDate('2099-01-01')", + "UNIQUE KEY id", + "SETTINGS index_granularity = 8192", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`{namespace}.ctas_unsupp` " + f"AS default.{src_table} {clause}", + settings={"allow_database_iceberg": 1}, + ) + assert "supports only PARTITION BY and ORDER BY" in err + + for table in ["from_as", "override"]: + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.{table}` SETTINGS allow_database_iceberg=1" + ) + node.query(f"DROP TABLE default.{src_table}") + + +def test_create_table_as_rejects_column_modifiers(started_cluster): + node = started_cluster.instances["node1"] + + namespace = "test_ctas_colmod_ns" + + create_clickhouse_iceberg_database( + started_cluster, + node, + CATALOG_NAME, + additional_settings={"default_base_location": "s3://warehouse-rest/data"}, + ) + + # Each source table carries a column modifier that Iceberg tables do not support. CREATE TABLE ... AS + # must reject it instead of silently creating the Iceberg table with weaker semantics than the source. + cases = [ + ("comment", "id Int64, name String COMMENT 'the name'"), + ("codec", "id Int64, name String CODEC(ZSTD)"), + ("ttl", "id Int64, dt Date, val Int64 TTL dt + INTERVAL 1 DAY"), + ] + for src_suffix, cols in cases: + src_table = f"src_colmod_{src_suffix}" + node.query(f"DROP TABLE IF EXISTS default.{src_table}") + node.query( + f"CREATE TABLE default.{src_table} ({cols}) ENGINE = MergeTree ORDER BY id" + ) + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`{namespace}.dst_{src_suffix}` " + f"AS default.{src_table} SETTINGS allow_database_iceberg = 1" + ) + assert "COMMENT, CODEC, TTL, STATISTICS, SETTINGS, and PRIMARY KEY are not supported" in err + node.query(f"DROP TABLE default.{src_table}") + + +def test_create_table_explicit_columns(started_cluster): + node = started_cluster.instances["node1"] + + namespace = "test_ctex_ns" + catalog = load_catalog_impl(started_cluster) + + create_clickhouse_iceberg_database( + started_cluster, + node, + CATALOG_NAME, + additional_settings={"default_base_location": "s3://warehouse-rest/data"}, + ) + + node.query( + f"DROP TABLE IF EXISTS {CATALOG_NAME}.`{namespace}.explicit` SETTINGS allow_database_iceberg=1" + ) + + node.query( + f""" + CREATE TABLE {CATALOG_NAME}.`{namespace}.explicit` + ( + id Int64, + name String, + value Float64 + ) + PARTITION BY id + ORDER BY name + SETTINGS allow_database_iceberg=1; + """ + ) + + tables = catalog.list_tables(namespace) + table_names = [t[1] for t in tables] + assert "explicit" in table_names + + tbl = catalog.load_table(f"{namespace}.explicit") + col_names = [f.name for f in tbl.schema().fields] + assert col_names == ["id", "name", "value"] + + iceberg_types = {f.name: str(f.field_type) for f in tbl.schema().fields} + assert iceberg_types["id"] == "long" + assert iceberg_types["name"] == "string" + assert iceberg_types["value"] == "double" + + node.query( + f"INSERT INTO {CATALOG_NAME}.`{namespace}.explicit` VALUES (1, 'a', 1.5);", + settings={ + "allow_insert_into_iceberg": 1, + "write_full_path_in_iceberg_metadata": 1, + }, + ) + assert ( + node.query( + f"SELECT id, name, value FROM {CATALOG_NAME}.`{namespace}.explicit`" + ) + == "1\ta\t1.5\n" + ) + + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.explicit` SETTINGS allow_database_iceberg=1" + ) + + +def test_create_table_nested_namespace(started_cluster): + node = started_cluster.instances["node1"] + + namespace = "test_nested_ns.a.b" + catalog = load_catalog_impl(started_cluster) + + create_clickhouse_iceberg_database( + started_cluster, + node, + CATALOG_NAME, + additional_settings={"default_base_location": "s3://warehouse-rest/data"}, + ) + + node.query( + f"DROP TABLE IF EXISTS {CATALOG_NAME}.`{namespace}.nested` SETTINGS allow_database_iceberg=1" + ) + node.query( + f""" + CREATE TABLE {CATALOG_NAME}.`{namespace}.nested` + ( + id Int64 + ) + SETTINGS allow_database_iceberg=1; + """ + ) + + tables = catalog.list_tables(namespace) + table_names = [t[-1] for t in tables] + assert "nested" in table_names + + write_settings = {"allow_insert_into_iceberg": 1, "write_full_path_in_iceberg_metadata": 1} + + # INSERT exercises `RestCatalog::updateMetadata` for the nested namespace. + node.query( + f"INSERT INTO {CATALOG_NAME}.`{namespace}.nested` VALUES (1);", + settings=write_settings, + ) + assert node.query( + f"SELECT id FROM {CATALOG_NAME}.`{namespace}.nested`", + settings={"allow_database_iceberg": 1}, + ).strip() == "1" + + # ALTER exercises `RestCatalog::updateSchema` for the nested namespace. + node.query( + f"ALTER TABLE {CATALOG_NAME}.`{namespace}.nested` ADD COLUMN z Nullable(String);", + settings=write_settings, + ) + assert "z" in node.query( + f"DESCRIBE TABLE {CATALOG_NAME}.`{namespace}.nested`", + settings=write_settings, + ) + + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.nested` SETTINGS allow_database_iceberg=1" + ) + + +def test_create_non_table_rejected(started_cluster): + node = started_cluster.instances["node1"] + + create_clickhouse_iceberg_database(started_cluster, node, CATALOG_NAME) + + # Each DDL flips a distinct flag in the interpreter's "plain CREATE TABLE only" guard. + for ddl in [ + f"CREATE VIEW {CATALOG_NAME}.`ns.v` AS SELECT 1", + f"CREATE MATERIALIZED VIEW {CATALOG_NAME}.`ns.mv` ENGINE = Memory AS SELECT 1", + f"ATTACH TABLE {CATALOG_NAME}.`ns.attached` (x Int32) ENGINE = Memory", + f"CREATE OR REPLACE TABLE {CATALOG_NAME}.`ns.replaced` (x Int32)", + ]: + err = node.query_and_get_error(ddl, settings={"allow_database_iceberg": 1}) + assert "supports only plain CREATE TABLE" in err + + node.query("DROP TABLE IF EXISTS default.src_clone") + node.query( + "CREATE TABLE default.src_clone (x Int32) ENGINE = MergeTree ORDER BY x" + ) + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.cloned` CLONE AS default.src_clone", + settings={"allow_database_iceberg": 1}, + ) + assert "supports only plain CREATE TABLE" in err + node.query("DROP TABLE default.src_clone") + + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.mem` (x Int32) ENGINE = Memory", + settings={"allow_database_iceberg": 1}, + ) + assert "only supports Iceberg-family table engines" in err + + +def test_create_table_unsupported_clauses(started_cluster): + node = started_cluster.instances["node1"] + + create_clickhouse_iceberg_database(started_cluster, node, CATALOG_NAME) + + base_ddl = f"CREATE TABLE {CATALOG_NAME}.`ns.unsupp` (id Int64, name String)" + for clause in [ + "PRIMARY KEY id ORDER BY id", + "ORDER BY id SAMPLE BY id", + "ORDER BY id TTL toDate('2099-01-01')", + "ORDER BY id SETTINGS index_granularity = 8192", + ]: + err = node.query_and_get_error( + f"{base_ddl} {clause}", + settings={"allow_database_iceberg": 1}, + ) + assert "supports only PARTITION BY and ORDER BY" in err + + # The table COMMENT is not persisted anywhere, so it is rejected by a separate check. + err = node.query_and_get_error( + f"{base_ddl} ORDER BY id COMMENT 'tbl comment'", + settings={"allow_database_iceberg": 1}, + ) + assert "Table COMMENT is not supported" in err + + for table_element in [ + "INDEX idx_name name TYPE bloom_filter GRANULARITY 1", + "PROJECTION p (SELECT id ORDER BY name)", + "CONSTRAINT c CHECK id > 0", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.unsupp_elem` (id Int64, name String, {table_element}) ORDER BY id", + settings={"allow_database_iceberg": 1}, + ) + assert "does not support PRIMARY KEY, indices" in err + + # Column-level PRIMARY KEY is normalized into the storage-level clause, covered above. + for col_clause in [ + "(id Int64 COMMENT 'pk', name String)", + "(id Int64, name String CODEC(ZSTD))", + "(id Int64, dt Date TTL dt + INTERVAL 1 DAY)", + "(id Int64, name String SETTINGS (max_compress_block_size = 1))", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.unsupp_col` {col_clause} ORDER BY id", + settings={"allow_database_iceberg": 1}, + ) + assert "COMMENT, CODEC, TTL, STATISTICS, SETTINGS, and PRIMARY KEY are not supported" in err + + for col_clause in [ + "(id Int64, d Int64 DEFAULT 1)", + "(id Int64, d Int64 MATERIALIZED id + 1)", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.unsupp_def` {col_clause} ORDER BY id", + settings={"allow_database_iceberg": 1}, + ) + assert "is not yet supported" in err + + +def test_create_table_with_engine_unsupported_clauses(started_cluster): + node = started_cluster.instances["node1"] + + create_clickhouse_iceberg_database(started_cluster, node, CATALOG_NAME) + + engine = ( + f"ENGINE = IcebergS3('http://minio1:9001/warehouse-rest/engine_unsupp/', " + f"'{minio_access_key}', '{minio_secret_key}')" + ) + + # The explicit-ENGINE path persists only column names/types, `PARTITION BY` and `ORDER BY` into the + # initial metadata, so unsupported storage clauses must be rejected there too. Engine `SETTINGS` stay + # allowed, unlike on the engine-less path: they are real data-lake settings used during creation. + for clause in [ + "PRIMARY KEY id", + "ORDER BY id SAMPLE BY id", + "TTL toDate('2099-01-01')", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.engine_unsupp` (id Int64, name String) {engine} {clause}", + settings={"allow_database_iceberg": 1}, + ) + assert "PRIMARY KEY, SAMPLE BY, TTL, and UNIQUE KEY are not supported" in err + + for table_element in [ + "INDEX idx_name name TYPE bloom_filter GRANULARITY 1", + "PROJECTION p (SELECT id ORDER BY name)", + "CONSTRAINT c CHECK id > 0", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.engine_unsupp` (id Int64, name String, {table_element}) {engine}", + settings={"allow_database_iceberg": 1}, + ) + assert "does not support PRIMARY KEY, indices" in err + + for col_clause in [ + "(id Int64 COMMENT 'pk', name String)", + "(id Int64, name String CODEC(ZSTD))", + "(id Int64, dt Date TTL dt + INTERVAL 1 DAY)", + "(id Int64, name String SETTINGS (max_compress_block_size = 1))", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.engine_unsupp` {col_clause} {engine}", + settings={"allow_database_iceberg": 1}, + ) + assert "COMMENT, CODEC, TTL, STATISTICS, SETTINGS, and PRIMARY KEY are not supported" in err + + for col_clause in [ + "(id Int64, d Int64 DEFAULT 1)", + "(id Int64, d Int64 MATERIALIZED id + 1)", + ]: + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.engine_unsupp` {col_clause} {engine}", + settings={"allow_database_iceberg": 1}, + ) + assert "is not yet supported" in err + + # Modifiers inherited via CREATE TABLE ... AS are rejected on the explicit-ENGINE path too. + node.query("DROP TABLE IF EXISTS default.src_engine_colmod") + node.query( + "CREATE TABLE default.src_engine_colmod (id Int64, name String COMMENT 'the name') " + "ENGINE = MergeTree ORDER BY id" + ) + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`ns.engine_unsupp` AS default.src_engine_colmod {engine}", + settings={"allow_database_iceberg": 1}, + ) + assert "COMMENT, CODEC, TTL, STATISTICS, SETTINGS, and PRIMARY KEY are not supported" in err + node.query("DROP TABLE default.src_engine_colmod") + + # Positive control: PARTITION BY, ORDER BY, and engine SETTINGS remain supported with an explicit engine. + namespace = f"test_engine_supp_{uuid.uuid4().hex}" + node.query( + f"CREATE TABLE {CATALOG_NAME}.`{namespace}.engine_supp` (id Int64, name String) " + f"ENGINE = IcebergS3('http://minio1:9001/warehouse-rest/engine_supp/', " + f"'{minio_access_key}', '{minio_secret_key}') " + f"PARTITION BY id ORDER BY name SETTINGS iceberg_format_version = 2", + settings={ + "allow_database_iceberg": 1, + "write_full_path_in_iceberg_metadata": 1, + }, + ) + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.engine_supp`", + settings={"allow_database_iceberg": 1}, + ) + + +def test_create_table_invalid_partition_transforms(started_cluster): + node = started_cluster.instances["node1"] + + namespace = "test_invalid_part_ns" + catalog = load_catalog_impl(started_cluster) + + create_clickhouse_iceberg_database( + started_cluster, + node, + CATALOG_NAME, + additional_settings={"default_base_location": "s3://warehouse-rest/data"}, + ) + + # A valid positive transform is accepted (and creates the namespace). + node.query( + f"DROP TABLE IF EXISTS {CATALOG_NAME}.`{namespace}.good` SETTINGS allow_database_iceberg=1" + ) + node.query( + f""" + CREATE TABLE {CATALOG_NAME}.`{namespace}.good` + ( + id Int64 + ) + PARTITION BY icebergBucket(8, id) + SETTINGS allow_database_iceberg=1; + """ + ) + assert "good" in [t[1] for t in catalog.list_tables(namespace)] + + # Invalid transform parameters must be rejected before any catalog metadata is written, or `CREATE TABLE` + # would register an unreadable table whose partition spec serializes as `bucket[0]`, `bucket[-1]` + # or `truncate[0]`. + for i, transform in enumerate( + ["icebergBucket(0, id)", "icebergBucket(-1, id)", "icebergTruncate(0, id)"] + ): + tbl = f"bad_{i}" + err = node.query_and_get_error( + f"CREATE TABLE {CATALOG_NAME}.`{namespace}.{tbl}` (id Int64) " + f"PARTITION BY {transform} SETTINGS allow_database_iceberg=1" + ) + assert "requires a positive" in err, err + assert tbl not in [t[1] for t in catalog.list_tables(namespace)] + + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.good` SETTINGS allow_database_iceberg=1" + ) + + +def test_create_table_namespace_location(started_cluster): + node = started_cluster.instances["node1"] + + namespace = f"test_ns_location_{uuid.uuid4().hex[:8]}" + catalog = load_catalog_impl(started_cluster) + + create_clickhouse_iceberg_database( + started_cluster, + node, + CATALOG_NAME, + additional_settings={"default_base_location": "s3://warehouse-rest/data"}, + ) + + node.query( + f"DROP TABLE IF EXISTS {CATALOG_NAME}.`{namespace}.first` SETTINGS allow_database_iceberg=1" + ) + node.query( + f""" + CREATE TABLE {CATALOG_NAME}.`{namespace}.first` + ( + id Int64 + ) + SETTINGS allow_database_iceberg=1; + """ + ) + + table_location = catalog.load_table(f"{namespace}.first").location().rstrip("/") + ns_location = catalog.load_namespace_properties(namespace).get("location") + + # The namespace default location must point at the namespace base, not at the first table's directory, + # or later tables created without an explicit location would land under that first table. + assert ns_location is not None, "namespace is missing its location property" + ns_location = ns_location.rstrip("/") + assert ns_location != table_location, ( + f"namespace location {ns_location} must not equal the first table location {table_location}" + ) + assert table_location.startswith(ns_location + "/"), (ns_location, table_location) + assert table_location[len(ns_location):].strip("/") == "first", (ns_location, table_location) + + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.first` SETTINGS allow_database_iceberg=1" + ) + + +def test_create_table_with_engine_namespace_location(started_cluster): + node = started_cluster.instances["node1"] + + namespace = f"test_ns_engine_location_{uuid.uuid4().hex[:8]}" + table_dir = f"engine_ns_location_{uuid.uuid4().hex[:8]}" + catalog = load_catalog_impl(started_cluster) + + create_clickhouse_iceberg_database(started_cluster, node, CATALOG_NAME) + + node.query( + f"CREATE TABLE {CATALOG_NAME}.`{namespace}.first` (id Int64) " + f"ENGINE = IcebergS3('http://minio1:9001/warehouse-rest/{table_dir}/', " + f"'{minio_access_key}', '{minio_secret_key}')", + settings={ + "allow_database_iceberg": 1, + "write_full_path_in_iceberg_metadata": 1, + }, + ) + + table_location = catalog.load_table(f"{namespace}.first").location().rstrip("/") + assert table_location.endswith(table_dir), table_location + + # The engine path here is `//`, which does not follow `//
`, + # so there is no namespace base to derive. The table's own directory must not become the namespace + # default location, or later tables in this namespace would land inside it. + ns_location = catalog.load_namespace_properties(namespace).get("location") + if ns_location is not None: + ns_location = ns_location.rstrip("/") + assert ns_location != table_location, (ns_location, table_location) + assert not ns_location.startswith(table_location + "/"), (ns_location, table_location) + + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.first`", + settings={"allow_database_iceberg": 1}, + ) + + # When the engine path does follow `//
`, the namespace base is unambiguous and + # is registered as the namespace default location, so later tables land next to this one, not inside it. + base_dir = f"engine_ns_base_{uuid.uuid4().hex[:8]}" + nested_namespace = f"test_ns_engine_derived_{uuid.uuid4().hex[:8]}" + node.query( + f"CREATE TABLE {CATALOG_NAME}.`{nested_namespace}.second` (id Int64) " + f"ENGINE = IcebergS3('http://minio1:9001/warehouse-rest/{base_dir}/{nested_namespace}/second/', " + f"'{minio_access_key}', '{minio_secret_key}')", + settings={ + "allow_database_iceberg": 1, + "write_full_path_in_iceberg_metadata": 1, + }, + ) + + ns_location = catalog.load_namespace_properties(nested_namespace).get("location") + assert ns_location is not None, "namespace is missing its location property" + assert ns_location.rstrip("/") == f"s3://warehouse-rest/{base_dir}/{nested_namespace}", ns_location + + node.query( + f"DROP TABLE {CATALOG_NAME}.`{nested_namespace}.second`", + settings={"allow_database_iceberg": 1}, + ) + + +def test_drop_table_purge(started_cluster): + node = started_cluster.instances["node1"] + + namespace = "test_drop_purge_ns" + catalog = load_catalog_impl(started_cluster) + minio_client = Minio( + f"{started_cluster.minio_ip}:{started_cluster.minio_port}", + access_key=minio_access_key, + secret_key=minio_secret_key, + secure=False, + ) + + create_clickhouse_iceberg_database( + started_cluster, + node, + CATALOG_NAME, + additional_settings={"default_base_location": "s3://warehouse-rest/data"}, + ) + + for table in ["to_keep", "to_purge"]: + node.query( + f"DROP TABLE IF EXISTS {CATALOG_NAME}.`{namespace}.{table}` SETTINGS allow_database_iceberg=1" + ) + node.query( + f""" + CREATE TABLE {CATALOG_NAME}.`{namespace}.{table}` + ( + id Int64 + ) + SETTINGS allow_database_iceberg=1; + """ + ) + + table_names = [t[1] for t in catalog.list_tables(namespace)] + assert "to_keep" in table_names + assert "to_purge" in table_names + + def table_prefix(table): + location = catalog.load_table(f"{namespace}.{table}").location() + assert location.startswith("s3://warehouse-rest/") + prefix = location[len("s3://warehouse-rest/"):].rstrip("/") + "/" + assert list( + minio_client.list_objects("warehouse-rest", prefix=prefix, recursive=True) + ), f"Expected metadata under {prefix} before drop" + return prefix + + keep_prefix = table_prefix("to_keep") + purge_prefix = table_prefix("to_purge") + + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.to_keep` SETTINGS allow_database_iceberg=1" + ) + node.query( + f"DROP TABLE {CATALOG_NAME}.`{namespace}.to_purge` SETTINGS allow_database_iceberg=1, data_lake_delete_data_on_drop=1" + ) + + table_names = [t[1] for t in catalog.list_tables(namespace)] + assert "to_keep" not in table_names + assert "to_purge" not in table_names + + # A drop without purge unregisters the table from the catalog but must keep its data. + assert list( + minio_client.list_objects("warehouse-rest", prefix=keep_prefix, recursive=True) + ), f"Expected objects under {keep_prefix} to survive a drop without purge" + + remaining = [ + o.object_name + for o in minio_client.list_objects("warehouse-rest", prefix=purge_prefix, recursive=True) + ] + assert not remaining, f"Expected purge to remove objects under {purge_prefix}, found: {remaining}" + + + +def test_create_if_not_exists_with_engine_over_leftover_metadata(started_cluster): + """ + A drop without `data_lake_delete_data_on_drop` keeps the metadata, so a later + `CREATE TABLE IF NOT EXISTS ... ENGINE = IcebergS3(...)` over the same path registers nothing and + must report that nothing was created - otherwise the insert of `... AS SELECT` gets no table. + """ + node = started_cluster.instances["node1"] + namespace = f"test_ns_leftover_{uuid.uuid4().hex[:8]}" + engine = ( + f"IcebergS3('http://minio1:9001/warehouse-rest/{namespace}/t/', " + f"'{minio_access_key}', '{minio_secret_key}')" + ) + settings = { + "allow_database_iceberg": 1, + "allow_insert_into_iceberg": 1, + "write_full_path_in_iceberg_metadata": 1, + } + + create_clickhouse_iceberg_database(started_cluster, node, CATALOG_NAME) + node.query( + f"CREATE TABLE {CATALOG_NAME}.`{namespace}.t` (id Int64) ENGINE = {engine}", + settings=settings, + ) + node.query(f"DROP TABLE {CATALOG_NAME}.`{namespace}.t`", settings=settings) + + node.query( + f"CREATE TABLE IF NOT EXISTS {CATALOG_NAME}.`{namespace}.t` (id Int64) " + f"ENGINE = {engine} AS SELECT 1 AS id", + settings=settings, + ) + assert node.query(f"EXISTS TABLE {CATALOG_NAME}.`{namespace}.t`", settings=settings) == "0\n" + + def test_delete_on_lazy_initialized_table(started_cluster): """ Regression test for https://github.com/ClickHouse/ClickHouse/issues/96806. diff --git a/tests/integration/test_storage_iceberg_no_spark/test_drop_delete_data_on_drop.py b/tests/integration/test_storage_iceberg_no_spark/test_drop_delete_data_on_drop.py new file mode 100644 index 000000000000..5487851202fb --- /dev/null +++ b/tests/integration/test_storage_iceberg_no_spark/test_drop_delete_data_on_drop.py @@ -0,0 +1,66 @@ +import pytest + +from helpers.iceberg_utils import create_iceberg_table, get_uuid_str + + +def _table_path(table_name): + return f"var/lib/clickhouse/user_files/iceberg_data/default/{table_name}" + + +def _files_left(cluster, storage_type, table_name): + if storage_type == "local": + return int( + cluster.instances["node1"] + .exec_in_container( + [ + "bash", + "-c", + f"find /{_table_path(table_name)} -type f 2>/dev/null | wc -l", + ] + ) + .strip() + ) + + # The S3 table path is relative to the bucket root, unlike the absolute local one. + return len( + list( + cluster.minio_client.list_objects( + cluster.minio_bucket, f"{_table_path(table_name)}/", recursive=True + ) + ) + ) + + +@pytest.mark.parametrize("delete_data_on_drop", [0, 1]) +@pytest.mark.parametrize("storage_type", ["local", "s3"]) +def test_drop_honours_query_level_delete_data_on_drop( + started_cluster_iceberg_no_spark, storage_type, delete_data_on_drop +): + # `StorageObjectStorage::drop` runs in a background thread, where the query context is already gone. + # A query-level `data_lake_delete_data_on_drop` reaches it only because `IStorage::prepareForDrop` + # captures it while the `DROP TABLE` query is still running. + instance = started_cluster_iceberg_no_spark.instances["node1"] + table_name = f"test_delete_data_on_drop_{storage_type}_{delete_data_on_drop}_{get_uuid_str()}" + + create_iceberg_table( + storage_type, + instance, + table_name, + started_cluster_iceberg_no_spark, + "(x String, y Int64)", + ) + instance.query(f"INSERT INTO {table_name} VALUES ('123', 1)") + assert instance.query(f"SELECT * FROM {table_name} ORDER BY ALL") == "123\t1\n" + assert _files_left(started_cluster_iceberg_no_spark, storage_type, table_name) > 0 + + instance.query( + f"DROP TABLE {table_name} SYNC", + settings={"data_lake_delete_data_on_drop": delete_data_on_drop}, + ) + + if delete_data_on_drop: + assert ( + _files_left(started_cluster_iceberg_no_spark, storage_type, table_name) == 0 + ) + else: + assert _files_left(started_cluster_iceberg_no_spark, storage_type, table_name) > 0