Skip to content

[Before forward-ports] Antalya 26.3: patch for SettingsChangesHistory#1653

Merged
zvonand merged 3 commits into
antalya-26.3from
fix/antalya-26.3/fix-settingschangeshistory
Apr 19, 2026
Merged

[Before forward-ports] Antalya 26.3: patch for SettingsChangesHistory#1653
zvonand merged 3 commits into
antalya-26.3from
fix/antalya-26.3/fix-settingschangeshistory

Conversation

@zvonand
Copy link
Copy Markdown
Collaborator

@zvonand zvonand commented Apr 15, 2026

When porting to future Antalya versions:

  • generate .tsv file containing settings values in latest Antalya release (more details in comments in tests/queries/0_stateless/02995_new_settings_history.sh), change the version number in test accordingly;
  • add new version section for the next release in src/Core/SettingsChangesHistory.cpp;

When cherry-pinking, do cherry-pick -n avoid picking:

  • ...tsv file: it needs to be re-generated
  • SettingsChangesHistory.cpp: instead, take Antalya-related sections from previous Antalya release's SettingsChangesHistory.cpp and manually add them to current file. comment all lines with Antalya-specific settings.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

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)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 15, 2026

Workflow [PR], commit [9633607]

@ianton-ru
Copy link
Copy Markdown

test_backward_compatibility/test_convert_ordinary.py::test_convert_ordinary_to_atomic broken, fix in #1646 - 24b139f

@ianton-ru
Copy link
Copy Markdown

Regression tests failed because test unmerged features, ignore it.

normalize_function_names 1
number_of_mutations_to_delay 0
number_of_mutations_to_throw 0
object_storage_cluster
Copy link
Copy Markdown

@ianton-ru ianton-ru Apr 16, 2026

Choose a reason for hiding this comment

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

Does this line correct before feature with object_storage_cluster merged? (Same for all Antalya-only features)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I guess so. A setting in CH is compared against this file, not the other way around. So it is fine to have some unincluded extra.

@zvonand zvonand added the port-antalya PRs to be ported to all new Antalya releases label Apr 19, 2026
@zvonand zvonand merged commit 1ef14ef into antalya-26.3 Apr 19, 2026
361 of 435 checks passed
@CarlosFelipeOR
Copy link
Copy Markdown
Collaborator

QA Verification — ✅ Approved

PR #1653 (Antalya 26.3 SettingsChangesHistory patch) is safe and did not introduce any regression. This is a forward-port of PR #1465 (merged into antalya-26.1 on 2026-03-02, ~7 weeks of production exposure) — the C++ logic for ClickHouseVersion suffix parsing is byte-for-byte identical to #1465. The only new content in this PR is additional commented-out settings placeholders in SettingsChangesHistory.cpp (no runtime behavior change) and a new 1527-row baseline TSV file for the Antalya canary test. All failing tests in the MasterCI merge run 24640946938 and the PR-CI run are either pre-existing on antalya-26.3, not forward-ported features, or known flaky — none are caused by this PR.

Canary test ✅

02995_new_settings_history (the stateless test directly modified by this PR) passed on 7 build types (amd_asan, amd_coverage, amd_debug, amd_debug s3 storage, amd_tsan, amd_tsan s3 storage, amd_ubsan) and was correctly skipped on arm_binary. No fails.

Summary

Category Tests / Jobs Status
Same pre-existing regression failures as PR #1651's MasterCI (identical counts on settings 126 fails, version 4 fails, etc.) disk_level_encryption, version, settings, session_timezone, aggregate_functions_{2,3}, tiered_storage_*, s3_*_1 ✅ Not caused by PR
Features not yet forward-ported to antalya-26.3 swarms, parquet, iceberg_{1,2}, s3_export_partition ✅ Expected
Pre-existing 26.3+ failures (also fail on upstream 26.4) rbac_1 (check supported timezones), ldap_external_user_directory (empty/missing server) ✅ Not caused by PR
One-off ExpectTimeoutError (infra) rbac_3 / check alter user add auth methods with two auth methods allowed ✅ Not caused by PR
Known flaky stateless/integration (each affecting 63–144 unrelated PRs over the last 30–60d) 00157_cache_dictionary, test_convert_ordinary_to_atomic, 02415_all_new_functions_must_be_documented, 03443_shared_storage_snapshots, test_cpu_time_fairness[fixed_longer_prd], 03590_simdjson_fallback, 01103_check_cpu_instructions_at_startup, 03706_statistics_preserve_checksums_on_mutations, test_parallel_quorum_actually_parallel, 04043_system_asynchronous_inserts_user_filter, test_kafka_formats_with_broken_message[generate_old_create_table_query], 03634_autopr_output_bytes_estimation, 03927_autopr_input_bytes_estimation_prewhere_filter ✅ Not caused by PR
Caused by this PR 0 🟢

Key evidence

Identical regression-suite pattern as #1651 — comparing the MasterCI runs of #1651 (25120257717) and #1653 (24640946938) on antalya-26.3, the settings job has exactly 126 fails / ~2950 OKs in both, and version has exactly 4 fails / 0 OKs in both. The same pre-existing 26.3 issue, not introduced by either PR.

Forward-port precedent — PR #1465 introduced the same ClickHouseVersion suffix-handling logic in antalya-26.1 on 2026-03-02 with no reported issues since. The C++ diff in this PR is identical; new content is structural-only (commented-out placeholders) with no runtime effect.

@CarlosFelipeOR CarlosFelipeOR added the verified Approved for release label May 14, 2026
@CarlosFelipeOR
Copy link
Copy Markdown
Collaborator

AI audit note: This review comment was generated by AI (gpt-5.3-codex).

Audit update for PR #1653 (SettingsChangesHistory Antalya patch + version parsing update):

Confirmed defects:

  • Medium: Malformed compatibility versions are now accepted instead of rejected
    Impact: Typos in compatibility values can silently apply an unintended compatibility profile instead of failing fast, causing hard-to-diagnose settings behavior.
    Anchor: src/Common/ClickHouseVersion.cpp / ClickHouseVersion::ClickHouseVersion
    Trigger: A non-numeric middle component such as 26..1, 26.1., or 26.x.1 in a parsed version string.
    Why defect: The constructor now treats the first non-numeric token (including empty tokens) as suffix and stops parsing, while previous behavior rejected such strings with BAD_ARGUMENTS.
    Fix direction (short): Restrict suffix handling to an explicit terminal suffix pattern; keep empty/intermediate non-numeric components invalid.
    Regression test direction (short): Add parser tests asserting exceptions for malformed inputs and success for valid suffixed forms like 26.1.3.20001.altinityantalya.

  • Medium: New stateless check can miss required current-version history updates (false negatives)
    Impact: A setting changed in current Antalya can bypass detection if its name exists in any historical Antalya entry, allowing missing SettingsChangesHistory updates to pass CI.
    Anchor: tests/queries/0_stateless/02995_new_settings_history.sh / filters using position(version, 'altinityantalya') > 0 and Antalya checks without version bound
    Trigger: A setting that was already present in an older Antalya history entry changes again in a newer Antalya release without a new history entry.
    Why defect: The NOT IN exclusion now matches names from all Antalya-tagged versions (or all Session versions in new Antalya blocks), so presence anywhere suppresses detection for current-release deltas.
    Fix direction (short): Bound Antalya checks to the intended version window (e.g., > previous_antalya_version) rather than “any Antalya ever”.
    Regression test direction (short): Add a synthetic test case where a setting changes in two different Antalya versions and ensure the second change is still required.

Coverage summary:

  • Scope reviewed: src/Common/ClickHouseVersion.cpp, src/Common/ClickHouseVersion.h, src/Core/SettingsChangesHistory.cpp, tests/queries/0_stateless/02995_new_settings_history.sh, and added baseline tests/queries/0_stateless/02995_settings_26_1_6_20001_antalya.tsv.
  • Categories failed: input-validation invariant for version parsing; test oracle strictness for Antalya delta detection.
  • Categories passed: call-graph/transition review of compatibility application paths (SettingsImpl::applyCompatibilitySetting, MergeTreeSettings::applyCompatibilitySetting), map ordering/duplicate-key behavior, and static side-effect review of baseline TSV addition.
  • Assumptions/limits: static audit only; CI report deep-parser tool could not be executed in this environment because node is unavailable.

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

Labels

antalya antalya-26.3 port-antalya PRs to be ported to all new Antalya releases verified Approved for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants