[WIP] Antalya 26.1; Remote initiator improvements 2#1608
[WIP] Antalya 26.1; Remote initiator improvements 2#1608ianton-ru wants to merge 3 commits intoantalya-26.1from
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a03617679
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const auto & settings = context->getSettingsRef(); | ||
|
|
||
| is_cluster_function = !settings[Setting::object_storage_cluster].value.empty() && typename Base::Configuration().isClusterSupported(); | ||
| is_cluster_function = typename Base::Configuration().isClusterSupported(); |
There was a problem hiding this comment.
Preserve simple-mode fallback when no cluster is configured
Assigning is_cluster_function from Base::Configuration().isClusterSupported() unconditionally forces s3(...)/iceberg(...) fallback functions into cluster mode even when object_storage_cluster is empty. In the CREATE TABLE ... AS <table_function> path, arguments are parsed before validateUseToCreateTable() is called, so this change now makes validateUseToCreateTable() throw for previously valid non-cluster statements like CREATE TABLE t AS s3(...). Please keep cluster mode gated by an actual configured cluster name (or a narrower remote-initiator condition) to avoid regressing non-cluster table-function creation.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed463afe38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (make_cluster_function) | ||
| { |
There was a problem hiding this comment.
Set cluster_name_in_settings on non-cluster rewrite
When make_cluster_function is false, this branch now skips setting cluster_name_in_settings, but updateQueryToSendIfNeeded() still uses that flag to decide whether the first argument is a cluster name (extractDynamicStorageType(..., !cluster_name_in_settings)). For engine-to-function rewrites in the new remote-initiator-without-local-cluster flow, this misclassifies the first real argument as a cluster name, so named-collection-based data lake queries can lose storage_type detection and fall back to S3 semantics on the remote side.
Useful? React with 👍 / 👎.
| is_cluster_function = (!settings[Setting::object_storage_cluster].value.empty() || !settings[Setting::object_storage_remote_initiator_cluster].value.empty()) | ||
| && typename Base::Configuration().isClusterSupported(); |
There was a problem hiding this comment.
Gate cluster fallback on remote initiator being enabled
This condition now flips to cluster-mode parsing whenever object_storage_remote_initiator_cluster is set, even if object_storage_remote_initiator is disabled. That changes plain s3/iceberg/... table-function behavior globally for sessions that carry only the cluster-name setting (for example from a profile), and can trigger cluster-only restrictions such as validateUseToCreateTable() rejecting create-table usage unexpectedly.
Useful? React with 👍 / 👎.
ed463af to
c60f71f
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
c60f71f to
b1efb77
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1efb77d2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const auto & settings = context->getSettingsRef(); | ||
| ASTPtr query_to_send = query_info.query; | ||
|
|
||
| if (!isClusterSupported() || cluster_name_from_settings.empty()) |
There was a problem hiding this comment.
Exclude non-cluster storages from remote-initiator rewrite
The new if (!isClusterSupported() || cluster_name_from_settings.empty()) branch now routes object_storage_remote_initiator=1 queries through updateQueryToSendIfNeeded(..., make_cluster_function=false) even when the storage explicitly reports isClusterSupported()==false. For datalake configs like StorageLocalPaimonConfiguration (used by PaimonLocal), StorageObjectStorageCluster::updateQueryForDistributedEngineIfNeeded cannot map the engine name (PaimonLocal) to a table function and throws a logical error before the remote call is built. Previously these queries fell back to pure local read; now they fail whenever remote initiator is enabled.
Useful? React with 👍 / 👎.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Use
object_storage_remote_initiatorwithoutobject_storage_clusteron initial node.Documentation entry for user-facing changes
Solved #1607
Query with
object_storage_remote_initiatorandobject_storage_remote_initiator_clustercan be executed, whenobject_storage_clustersetting is defined only on remote node.converted to
Remote node
random_node_from_swarm_external_nameexecuteslocally of as cluster table function depend on
object_storage_clusterdefined only on this node.CI/CD Options
Exclude tests:
Regression jobs to run: