Skip to content

Conversation

@bharatnc
Copy link
Contributor

@bharatnc bharatnc commented Mar 19, 2025

Reverts #77680.

Broken tests 03393_additional_table_filter_with_param 1, 2

@clickhouse-gh
Copy link

clickhouse-gh bot commented Mar 19, 2025

Workflow [PR], commit [3b09d9d]

@wxybear
Copy link
Contributor

wxybear commented Mar 19, 2025

Hi, is this broken test run failure running on the master CI? I am the one who submitted this PR, and I did not reproduce the problem.

@devcrafter
Copy link
Member

devcrafter commented Mar 19, 2025

@wxybear To reproduce it locally, you need to add to the test the following settings:

set enable_parallel_replicas=1, cluster_for_parallel_replicas='test_cluster_one_shard_three_replicas_localhost', parallel_replicas_for_non_replicated_merge_tree = 1;

and add / uncomment in programs/server/config.xml

<listen_host>0.0.0.0</listen_host> 

In the example below, parametrized query w/o parametrized additional_table_filters succeeded but after adding it, the query failed on “remote” node. This need to be fixed.

Example:
https://pastila.nl/?02797c1f/b795fc49b0e1d92f08c61280e36d678a#NKFzC7aRlmpepvjfA0Oytw==

@wxybear
Copy link
Contributor

wxybear commented Mar 19, 2025

@devcrafter Got it. The timing of additional_table_filter rewriting is indeed different from that of ordinary paramter. I will evaluate how to implement it, you can revert it first

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Mar 19, 2025
@devcrafter devcrafter enabled auto-merge March 19, 2025 11:07
@devcrafter devcrafter added this pull request to the merge queue Mar 19, 2025
Merged via the queue into master with commit 027b22f Mar 19, 2025
98 of 119 checks passed
@devcrafter devcrafter deleted the revert-77680-feat_additional_table_filter_use_param branch March 19, 2025 11:57
@robot-clickhouse robot-clickhouse added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants