Stabilize 03772_temporary_files_codec test by PedroTadim · Pull Request #101995 · ClickHouse/ClickHouse · GitHub
Skip to content

Stabilize 03772_temporary_files_codec test#101995

Merged
PedroTadim merged 6 commits into
masterfrom
fix-test
Apr 8, 2026
Merged

Stabilize 03772_temporary_files_codec test#101995
PedroTadim merged 6 commits into
masterfrom
fix-test

Conversation

@PedroTadim

@PedroTadim PedroTadim commented Apr 7, 2026

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

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

...

Failure here https://s3.amazonaws.com/clickhouse-test-reports/json.html?REF=master&sha=d2f57548e7eed2deb6a65aa93c50bbe8209873ae&name_0=MasterCI&name_1=Stateless+tests+%28amd_msan%2C+WasmEdge%2C+parallel%2C+1%2F2%29

Claude code fix
"The test forces external sort/group_by via session SET, but the randomizer can inject max_bytes_before_external_sort = 0 and max_bytes_ratio_before_external_sort = 0 via CLI. The inline SETTINGS clause in each query has highest priority — so pinning the thresholds there is the cleanest fix"

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Version info

  • Merged into: 26.4.1.680

@clickhouse-gh

clickhouse-gh Bot commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

@clickhouse-gh clickhouse-gh Bot added the pr-ci label Apr 7, 2026
@PedroTadim PedroTadim changed the title Stabilize 02859_replicated_db_name_zookeeper test Stabilize 02859_replicated_db_name_zookeeper and 03772_temporary_files_codec tests Apr 7, 2026
Comment thread tests/queries/0_stateless/03772_temporary_files_codec.sql Outdated
@alexey-milovidov

Copy link
Copy Markdown
Member

The test 02859_replicated_db_name_zookeeper is fixed in #101952

@alexey-milovidov alexey-milovidov self-assigned this Apr 8, 2026
@PedroTadim PedroTadim changed the title Stabilize 02859_replicated_db_name_zookeeper and 03772_temporary_files_codec tests Stabilize 03772_temporary_files_codec test Apr 8, 2026
@PedroTadim PedroTadim enabled auto-merge April 8, 2026 09:57
@PedroTadim PedroTadim added this pull request to the merge queue Apr 8, 2026
Merged via the queue into master with commit da7be06 Apr 8, 2026
161 of 164 checks passed
@PedroTadim PedroTadim deleted the fix-test branch April 8, 2026 20:57
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 8, 2026
@azat

azat commented May 14, 2026

Copy link
Copy Markdown
Member

"The test forces external sort/group_by via session SET, but the randomizer can inject max_bytes_before_external_sort = 0 and max_bytes_ratio_before_external_sort = 0 via CLI. The inline SETTINGS clause in each query has highest priority — so pinning the thresholds there is the cleanest fix"

This is wrong, settings that set with SET query has a priority over the randomized settings, so I don't see how this fixes the test

@azat

azat commented May 14, 2026

Copy link
Copy Markdown
Member

nihalzp pushed a commit to nihalzp/ClickHouse that referenced this pull request May 17, 2026
The five SELECT/GROUP BY queries (sort × 2, agg × 3) each carried
per-query `SETTINGS` clauses re-pinning
`max_bytes_before_external_sort`,
`max_bytes_ratio_before_external_sort`,
`max_bytes_before_external_group_by`, and
`max_bytes_ratio_before_external_group_by` to the exact same values
that are already pinned at session level by the `SET` statements at
the top of the file.

Session `SET` already overrides the test runner's randomization, so
the per-query `SETTINGS` were redundant noise. Removing them keeps
the session-level pins (the authoritative ones) and leaves only
query-specific settings (`log_comment`, `temporary_files_codec`) on
each query.

Verified by running the test 30 times with randomization
(`-- Tags: long` temporarily removed to allow more iterations):
30/30 passed. `long` tag restored before commit.

Follow-up to ClickHouse#101995 and
ClickHouse#102195, addressing
@azat's request to clean up the test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-ci 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.

4 participants