25.8 Antalya: Fix joins with Iceberg tables #1082

ianton-ru · 2025-10-15T17:06:06Z

Changelog category (leave one):

Bug Fix (user-visible misbehavior in an official stable release)

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

Fix joins with Iceberg tables
Solved #1063 (I hope)

Documentation entry for user-facing changes

Joins works with Iceberg tables, not only with table functions
Fix for local columns in ORDER BY/GROUP BY
WHERE/PREWHERE condition for subquery
More tests

CI/CD Options

Exclude tests:

Regression jobs to run:

github-actions · 2025-10-15T17:06:59Z

Workflow [PR], commit [2513c29]

ianton-ru · 2025-10-16T07:56:24Z

Failed tests:
03572_export_replicated_merge_tree_part_to_object_storage - looks like intersection in object_storage_cluster and part export
02995_new_settings_history:

2025-10-15 14:20:45 +PLEASE ADD THE NEW SETTING TO SettingsChangesHistory.cpp: export_merge_tree_part_overwrite_file_if_exists WAS ADDED
2025-10-15 14:20:45 +PLEASE ADD THE NEW SETTING TO SettingsChangesHistory.cpp: allow_experimental_export_merge_tree_part WAS ADDED
2025-10-15 14:20:45 +PLEASE ADD THE NEW SETTING TO SettingsChangesHistory.cpp: allow_experimental_export_merge_tree_part WAS ADDED

01079_parallel_alter_detach_table_zookeeper - looks like flap

mkmkme · 2025-10-20T08:45:10Z

src/Storages/IStorageCluster.cpp

+        if (query_node.hasWhere() || query_node.hasPrewhere())
+        {
+            CollectUsedColumnsForSourceVisitor collector_where(table_function_node, context, true);
+            if (query_node.hasPrewhere())


It's quite odd that the original code didn't take prewhere into account...

If those are actually needed, we need to place a mental pin here that after backporting prewhere+row_policy we'll need to add hasRowLevelFilter here as well.

mkmkme · 2025-10-20T11:01:43Z

tests/integration/test_database_iceberg/test.py

    assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{root_namespace}.{table_encoded_name}`") == "\\N\tAAPL\t193.24\t193.31\t('bot')\n"
+
+
+def test_cluster_joins(started_cluster):


This test seems to be passing on the current Antalya branch:

test_database_iceberg/test.py::test_cluster_joins PASSED

Is it expected?

Nice catch, test doesn't cover case with local right table, thanks!

And not on cluster...

Can you add a test for the CROSS JOIN case as well?

…alya-25.8/s3cluster_global_join_fixes

…l_join_fixes

This reverts commit bff8bba.

…l_join_fixes

ianton-ru · 2025-11-13T10:47:34Z

Remove unused code

arthurpassos · 2025-11-13T16:32:45Z

@ianton-ru can you give me a bit more context on what the issue was in each specific bullet point and how it was fixed?

For example:

Joins works with Iceberg tables, not only with table functions

Why wasn't it working? What was missing? And what did you do to get it working?

Same for the others.

ianton-ru · 2025-11-14T08:09:39Z

Joins works with Iceberg tables, not only with table functions

When I search tree node to make subquery to replicas, I have searched only QueryTreeNodeType::TABLE_FUNCTION before (in #972).
Now QueryTreeNodeType::TABLE is processed too.

Fix for local columns in ORDER BY/GROUP BY

ORDER BY condition sent to replica nodes and got error about unknown identifiers when it with local columns, now condition works only on local node.

WHERE/PREWHERE condition for subquery
In first realization I removed WHERE/PREWHERE condition in subquery when is contains local columns.

SELECT ice.x, local.y
  FROM iceberg.table AS ice
  JOIN local.table AS local
  ON ice.key=local.key
WHERE
  ice.x >= 10 AND local.y >= 20

Befor subquery was like

SELECT ice.x
  FROM iceberg.table AS ice

Now removeExpressionsThatDoNotDependOnTableIdentifiers is calles, and subquery with part of WHERE condition ()

SELECT ice.x
  FROM iceberg.table AS ice
WHERE
  ixe.x >= 10

Also fixed bugs founded by Alsu:

CROSS JOIN (has separate node type in query tree)
Lost strings (forgotten convertToFullColumnIfConst in StorageObjectStorageSource::generate)
Incorrect work when table function in joined with itself

SELECT * FROM iceberg('path') JOIN iceberg('path') ...

storage reused in this case, but I tried to save some info about query (has_join) inside storage object. As result was attempt to process right table as left table with strange errors.

arthurpassos

Given my ignorance on the subject, I did my best for the 1st pass. Mostly questions so I can better understand the code and do a follow up. I beg for patience :)

arthurpassos · 2025-11-14T11:53:54Z

src/Storages/IStorageCluster.cpp

-        auto table_function_node = table_function_searcher.getNode();
-        if (!table_function_node)
-            throw Exception(ErrorCodes::LOGICAL_ERROR, "Can't find table function node");
+    SearcherVisitor table_function_searcher({QueryTreeNodeType::TABLE, QueryTreeNodeType::TABLE_FUNCTION}, context);


Nit: is this still a table_FUNCTION_visitor?

Table and table function. Try to make better name. Actually I try to get left expression of join here, something like left_table_expression_searcher.

arthurpassos · 2025-11-14T11:59:15Z

src/Storages/IStorageCluster.cpp

+    auto & query_node = query_tree->as<QueryNode &>();
+    if (query_node.hasWhere() || query_node.hasPrewhere())
+    {
        CollectUsedColumnsForSourceVisitor collector_where(table_function_node, context, true);


I understand CollectUsedColumnsForSourceVisitor is meant to collect all columns for a given "source". As I understand, the source is provided in the constructor. But what makes it local? Doesn't the query_tree object you are passing represent the entire query tree? I don't get it, please educate me

Yes, query_tree contains all, But in different places.
Columns can be in selected list, in where condition, in join condition, etc.

SELECT table1.column1,... FROM table1 JOIN table2 ON table1.column2=table2.column2 WHERE table1.column3=...

and collector traverses the tree and collect all cases in single list.
Need to all these columns to select list for left table subquery.

I understand the columns can be in different places, but let's say there are no local columns. Wouldn't collector_where.getColumns() be non-empty regardless? Btw, what makes a table local or not local assuming you can't check for its existence on the remote node?

Actually CollectUsedColumnsForSourceVisitor find here all columns from other sources (last boolean flag collect_columns_from_other_sources_ in constructor).
Columns without source skipped in any case.
So when if where something like table1.column1 =1 AND table2.column2 = 2 AND (table1.column3 + table2.column4) = 3 AND randUniform(1,6) = 4 result contains only table2.column2 and table2.column4. Actually I don't know is table2 local or not, but author of query can know. If he used object_storage_cluster_join_mode='local', need to process as if it local.

src/Storages/IStorageCluster.cpp

arthurpassos · 2025-11-14T12:27:52Z

src/Storages/IStorageCluster.cpp

+
+            if (info.has_join || info.has_cross_join)
+            {
+                if (table_function_searcher.getType().value() == QueryTreeNodeType::TABLE_FUNCTION)


Do I get it right that you are extracting ONLY the table function from the original AST? Why doesn't it have to be done for tables as well?

I mean, I understand it is being done for tables when grabbing the left side. But why is it different. Is the query tree different for table function vs regular tables?

Good point. Looks like lats two blocks work for table function too!
But now need to retest everything.

arthurpassos · 2025-11-14T12:28:48Z

src/Storages/IStorageCluster.cpp

+                    auto & table_function_ast = table_function->as<ASTFunction &>();
+                    query_tree_distributed->setAlias(table_function_ast.alias);
+                }
+                else if (info.has_join)


Do I get it right that the iceberg table is always the left one?

Yes (actually not only iceberg, s3Cluster too). In other cases code does not come into IStorageCluster.
When join with local table on the left and iceberg on the right clickhouse processes query as local join and only on final stage reads right table without attempts to send left to remote nodes as part of the subquery, so I did not touch that case.

Hmm... So in a nutshell, there are two cases: iceberg / s3cluster table on the left, in which we need to prevent local tables from reaching remote nodes. and the other case, in which the local is resolved locally before reaching this code path?

Yes. In this code left table always s3Cluster/icebergCluster or Iceberg table.

arthurpassos · 2025-11-14T12:29:50Z

src/Storages/IStorageCluster.cpp

+                }
+                else
+                {
+                    SearcherVisitor join_searcher({QueryTreeNodeType::CROSS_JOIN}, context);


Shouldn't this become a method in QueryNode? For example: QueryNode::getCrossJoinTree

All others methods in QueryNode don't search on request, but returns pre-filled fields for children vector. Now is very specific case, not sure that for this single case need to make and try to fill this position in all queries.
May be it make sense, but need to review all cross-join-related code and make refactoring.

arthurpassos · 2025-11-14T12:30:16Z

src/Storages/IStorageCluster.cpp

+                    auto cross_join_node = join_searcher.getNode();
+                    if (!cross_join_node)
+                        throw Exception(ErrorCodes::LOGICAL_ERROR, "Can't find CROSS JOIN node");
+                    query_tree_distributed = cross_join_node->as<CrossJoinNode>()->getTableExpressions()[0]->clone();


Does [0] mean left?

Yes. CROSS JOIN has a vector of expression, can have more than two sources. So element 0 is a left. Can't be empty, as I understand.

Perhaps just add a comment, but not required

Add comment

…l_join_fixes

arthurpassos

LGTM

ianton-ru added 3 commits October 15, 2025 18:16

Join for tables

38a45e4

Some fixes

43825ba

Add test for tables

4e9ac7e

Fix stateless tests

3dd9b46

mkmkme reviewed Oct 20, 2025

View reviewed changes

ianton-ru added 5 commits October 20, 2025 14:21

Fix typo

f5215f2

Add tests with local table

023e38d

ColumnConstable

bff8bba

Merge branch 'feature/antalya-25.8/column_constable' into feature/ant…

8dcdd67

…alya-25.8/s3cluster_global_join_fixes

Fix build with sanitizer

e143307

ianton-ru changed the title ~~25.8 Antalya: Fix joins with Iceberg tables~~ [WIP] 25.8 Antalya: Fix joins with Iceberg tables Oct 24, 2025

svb-alt added antalya antalya-25.8 labels Oct 24, 2025

ianton-ru added 2 commits November 5, 2025 15:51

Forgotten change

70805a8

Iceberg cross join

39882f5

ianton-ru force-pushed the feature/antalya-25.8/s3cluster_global_join_fixes branch from fba187a to 39882f5 Compare November 6, 2025 13:39

ianton-ru and others added 5 commits November 11, 2025 14:19

Fix info about joins

9ce6b71

Merge branch 'antalya-25.8' into feature/antalya-25.8/s3cluster_globa…

454bafd

…l_join_fixes

Turn off contable column

239cf4c

Revert "ColumnConstable"

154e5fe

This reverts commit bff8bba.

Merge branch 'antalya-25.8' into feature/antalya-25.8/s3cluster_globa…

e1a8cd1

…l_join_fixes

ianton-ru changed the title ~~[WIP] 25.8 Antalya: Fix joins with Iceberg tables~~ 25.8 Antalya: Fix joins with Iceberg tables Nov 13, 2025

arthurpassos reviewed Nov 14, 2025

View reviewed changes

Fixes after review

06e48e4

Add tests

6ceecc4

ianton-ru force-pushed the feature/antalya-25.8/s3cluster_global_join_fixes branch from 7255bda to 6ceecc4 Compare November 17, 2025 11:43

Merge branch 'antalya-25.8' into feature/antalya-25.8/s3cluster_globa…

2513c29

…l_join_fixes

arthurpassos approved these changes Nov 17, 2025

View reviewed changes

Enmk merged commit 7799454 into antalya-25.8 Nov 17, 2025
105 of 108 checks passed

		assert node.query(f"SELECT * FROM {CATALOG_NAME}.`{root_namespace}.{table_encoded_name}`") == "\\N\tAAPL\t193.24\t193.31\t('bot')\n"


		def test_cluster_joins(started_cluster):

25.8 Antalya: Fix joins with Iceberg tables #1082

25.8 Antalya: Fix joins with Iceberg tables #1082

Uh oh!

Conversation

ianton-ru commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Documentation entry for user-facing changes

CI/CD Options

Exclude tests:

Regression jobs to run:

Uh oh!

github-actions bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianton-ru commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ianton-ru commented Nov 13, 2025

Uh oh!

arthurpassos commented Nov 13, 2025

Uh oh!

ianton-ru commented Nov 14, 2025

Uh oh!

arthurpassos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arthurpassos left a comment

ianton-ru commented Oct 15, 2025 •

edited

Loading

github-actions bot commented Oct 15, 2025 •

edited

Loading

ianton-ru commented Oct 16, 2025 •

edited

Loading