Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions base/poco/Net/include/Poco/Net/HTTPRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,16 @@ namespace Net
static const std::string UPGRADE;
static const std::string EXPECT;

void setSuppressKeepAliveHeader(bool suppress);
/// Suppresses the hop-by-hop `Keep-Alive: timeout=..., max=...` request header that
/// HTTPClientSession::sendRequest would otherwise add. The header is deprecated by RFC 7230
/// and some peers reject it outright: Amazon S3 Tables answers `S3TablesUnsupportedHeader`
/// to any PUT carrying it. Keep-alive itself and connection reuse are unaffected -- they are
/// driven by the `Connection` header and by ClickHouse's own connection pool.

bool getSuppressKeepAliveHeader() const;
/// Returns true if the `Keep-Alive` request header is suppressed for this request.

protected:
void getCredentials(const std::string & header, std::string & scheme, std::string & authInfo) const;
/// Returns the authentication scheme and additional authentication
Expand All @@ -179,6 +189,7 @@ namespace Net

std::string _method;
std::string _uri;
bool _suppress_keep_alive_header = false;

HTTPRequest(const HTTPRequest &);
HTTPRequest & operator=(const HTTPRequest &);
Expand All @@ -200,6 +211,18 @@ namespace Net
}


inline void HTTPRequest::setSuppressKeepAliveHeader(bool suppress)
{
_suppress_keep_alive_header = suppress;
}


inline bool HTTPRequest::getSuppressKeepAliveHeader() const
{
return _suppress_keep_alive_header;
}


}
} // namespace Poco::Net

Expand Down
3 changes: 2 additions & 1 deletion base/poco/Net/src/HTTPClientSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ std::ostream& HTTPClientSession::sendRequest(HTTPRequest& request, uint64_t * co
reconnect(connect_time);
if (!request.has(HTTPMessage::CONNECTION))
request.setKeepAlive(keepAlive);
if (keepAlive && !request.has(HTTPMessage::CONNECTION_KEEP_ALIVE) && _keepAliveTimeout.totalSeconds() > 0)
if (keepAlive && !request.getSuppressKeepAliveHeader() && !request.has(HTTPMessage::CONNECTION_KEEP_ALIVE)
&& _keepAliveTimeout.totalSeconds() > 0)
request.setKeepAliveTimeout(_keepAliveTimeout.totalSeconds(), _keepAliveMaxRequests);
if (!request.has(HTTPRequest::HOST) && !_host.empty())
request.setHost(_host, _port);
Expand Down
2 changes: 1 addition & 1 deletion src/Databases/DataLake/GlueCatalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ 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 /*delete_data*/) const
{
if (!isNamespaceAllowed(namespace_name))
throw DB::Exception(DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED,
Expand Down
2 changes: 1 addition & 1 deletion src/Databases/DataLake/GlueCatalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,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 delete_data) const override;

/// Returns a callback that re-vends fresh AWS credentials from the configured
/// credentials provider chain. Invoked by `ReadBufferFromS3` when an S3 call
Expand Down
2 changes: 1 addition & 1 deletion src/Databases/DataLake/ICatalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ 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::dropTable(const String & /*namespace_name*/, const String & /*table_name*/, bool /*delete_data*/) const
{
throw DB::Exception(DB::ErrorCodes::NOT_IMPLEMENTED, "dropTable is not implemented");
}
Expand Down
2 changes: 1 addition & 1 deletion src/Databases/DataLake/ICatalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class ICatalog
Poco::JSON::Object::Ptr metadata = nullptr) const;

/// Drop table from catalog.
virtual void dropTable(const String & namespace_name, const String & table_name) const;
virtual void dropTable(const String & namespace_name, const String & table_name, bool delete_data) 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".
Expand Down
2 changes: 1 addition & 1 deletion src/Databases/DataLake/RestCatalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1785,7 +1785,7 @@ 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 /*delete_data*/) const
{
if (!allowed_namespaces.isNamespaceAllowed(namespace_name, /*nested*/ false))
throw DB::Exception(DB::ErrorCodes::CATALOG_NAMESPACE_DISABLED,
Expand Down
2 changes: 1 addition & 1 deletion src/Databases/DataLake/RestCatalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ 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 delete_data) const override;

ICatalog::CredentialsRefreshCallback getCredentialsConfigurationCallback(const DB::StorageID & storage_id) override;

Expand Down
38 changes: 33 additions & 5 deletions src/Databases/DataLake/S3TablesCatalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -157,13 +158,17 @@ bool S3TablesCatalog::tryGetTableMetadata(
if (!RestCatalog::tryGetTableMetadata(namespace_name, table_name, context_, result))
return false;

if (!result.requiresCredentials())
return true;
/// For S3 Tables the catalog and the underlying data live in AWS S3 under the same
/// AWS principal, so endpoint/metadata-location normalization and fallback IAM
/// credential injection must always run - even when the engine was created with
/// `vended_credentials=0` (which leaves `requiresCredentials()` == false). Otherwise
/// reads and inserts can lose the catalog endpoint/credentials and fail with auth
/// or routing errors.

bool need_credentials = true;
if (const auto storage_credentials = result.getStorageCredentials())
if (result.hasStorageCredentials())
{
auto creds = std::dynamic_pointer_cast<S3Credentials>(storage_credentials);
auto creds = std::dynamic_pointer_cast<S3Credentials>(result.getStorageCredentials());
if (creds && !creds->isEmpty())
need_credentials = false;
}
Expand All @@ -178,6 +183,7 @@ bool S3TablesCatalog::tryGetTableMetadata(
"S3 Tables: catalog IAM credentials are empty for {}.{}, "
"check AWS credentials configuration",
namespace_name, table_name);
result.withStorageCredentials();
result.setStorageCredentials(std::make_shared<S3Credentials>(
aws_creds.GetAWSAccessKeyId(), aws_creds.GetAWSSecretKey(), aws_creds.GetSessionToken()));
}
Expand All @@ -191,6 +197,20 @@ bool S3TablesCatalog::tryGetTableMetadata(
result.setEndpoint(endpoint);
}

if (auto props = result.getDataLakeSpecificProperties();
props && !props->iceberg_metadata_file_location.empty())
{
const String & loc = props->iceberg_metadata_file_location;
auto scheme_end = loc.find("://");
if (scheme_end != String::npos)
{
auto path_start = loc.find('/', scheme_end + 3);
if (path_start != String::npos)
props->iceberg_metadata_file_location = loc.substr(path_start + 1);
}
result.setDataLakeSpecificProperties(std::move(props));
}

return true;
}

Expand All @@ -214,8 +234,16 @@ 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) const
{
/// https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-tables-delete.html
if (!delete_data)
throw DB::Exception(
DB::ErrorCodes::SUPPORT_IS_DISABLED,
"S3 Tables cannot drop table {}.{} without deleting its data, and `iceberg_delete_data_on_drop` is disabled. "
"Enable `iceberg_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";
Expand Down
2 changes: 1 addition & 1 deletion src/Databases/DataLake/S3TablesCatalog.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) const override;

ICatalog::CredentialsRefreshCallback getCredentialsConfigurationCallback(const DB::StorageID & storage_id) override;

Expand Down
1 change: 1 addition & 0 deletions src/IO/S3/PocoHTTPClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ void PocoHTTPClient::makeRequestInternalImpl(

Poco::Net::HTTPRequest poco_request(Poco::Net::HTTPRequest::HTTP_1_1);

poco_request.setSuppressKeepAliveHeader(true);
/** According to RFC-2616, Request-URI is allowed to be encoded.
* However, there is no clear agreement on which exact symbols must be encoded.
* Effectively, `Poco::URI` chooses smaller subset of characters to encode,
Expand Down
8 changes: 5 additions & 3 deletions src/Storages/ObjectStorage/StorageObjectStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,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 iceberg_delete_data_on_drop;
}

namespace ErrorCodes
Expand Down Expand Up @@ -833,13 +834,14 @@ void StorageObjectStorage::truncate(

void StorageObjectStorage::drop()
{
/// We cannot use query context here, because drop is executed in the background.
auto drop_context = Context::getGlobalContextInstance();
if (catalog)
{
const auto [namespace_name, table_name] = DataLake::parseTableName(storage_id.getTableName());
catalog->dropTable(namespace_name, table_name);
catalog->dropTable(namespace_name, table_name, drop_context->getSettingsRef()[Setting::iceberg_delete_data_on_drop]);
}
/// We cannot use query context here, because drop is executed in the background.
configuration->drop(Context::getGlobalContextInstance());
configuration->drop(drop_context);
}

std::unique_ptr<ReadBufferIterator> StorageObjectStorage::createReadBufferIterator(
Expand Down
Loading