Skip to content

26.1 Antalya port - fixes for s3Cluster distributed calls#1395

Open
ianton-ru wants to merge 8 commits intoantalya-26.1from
frontport/antalya-26.1/fix_remote_calls
Open

26.1 Antalya port - fixes for s3Cluster distributed calls#1395
ianton-ru wants to merge 8 commits intoantalya-26.1from
frontport/antalya-26.1/fix_remote_calls

Conversation

@ianton-ru
Copy link

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Frontports for Antalya 26.1

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • Tiered Storage (2h)

@github-actions
Copy link

github-actions bot commented Feb 11, 2026

Workflow [PR], commit [3829e1a]

@ianton-ru ianton-ru added antalya port-antalya PRs to be ported to all new Antalya releases antalya-26.1 labels Feb 13, 2026
@ianton-ru
Copy link
Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7df85aef41

ℹ️ 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".

Comment on lines +233 to +234
auto join_node = query_node.getJoinTree();
query_tree_distributed = join_node->as<JoinNode>()->getLeftTableExpression()->clone();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Limit JOIN rewrite to the top-level join tree

When object_storage_cluster_join_mode='local', this branch assumes info.has_join means the root QueryNode join tree is a JoinNode, but getQueryTreeInfo() currently marks has_join if it finds any JOIN anywhere in query_tree (including scalar/IN subqueries). In queries where the top-level FROM is just s3Cluster(...) and only a nested subquery contains a JOIN, query_node.getJoinTree() is not a JoinNode, so as<JoinNode>() throws and the query fails. Please restrict JOIN detection/rewrite to the top-level join tree (or validate node type before casting).

Useful? React with 👍 / 👎.

@ianton-ru
Copy link
Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3829e1a458

ℹ️ 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".

auto modified_query_tree = query_tree->clone();

SearcherVisitor left_table_expression_searcher({QueryTreeNodeType::TABLE, QueryTreeNodeType::TABLE_FUNCTION}, context);
left_table_expression_searcher.visit(modified_query_tree);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Search source table only in main join tree

The source-table search is run on the entire modified_query_tree and stops at the first TABLE/TABLE_FUNCTION node, so queries with scalar/CTE subqueries can match an unrelated table before the main FROM source. When object_storage_cluster_join_mode='local', the later projection and WHERE rewrite then operates on the wrong node, which can generate invalid remote subqueries or strip the wrong predicates. Limit this search to the main join tree (for example query_node.getJoinTree()) instead of the full query tree.

Useful? React with 👍 / 👎.

Comment on lines +248 to +250
query_node.resolveProjectionColumns(columns);
auto column_nodes_to_select = std::make_shared<ListNode>();
column_nodes_to_select->getNodes().reserve(columns.size());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve non-empty projection in rewritten remote query

The rewrite always replaces the SELECT list with collected source columns, but that set can be empty for valid JOIN shapes that do not reference source columns directly (e.g. SELECT count() with CROSS JOIN, or JOIN ... ON 1). In that case the serialized remote query becomes an empty projection (SELECT FROM ...) and fails to parse on remote shards. Add a fallback projection (such as a constant) or skip projection replacement when columns is empty.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

antalya antalya-26.1 port-antalya PRs to be ported to all new Antalya releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants