Antalya-26.3 Frontport of #1659 - Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks by mkmkme · Pull Request #1723 · Altinity/ClickHouse

mkmkme · 2026-05-02T11:57:16Z

Antalya 26.1 - Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks

Changelog category (leave one):

New Feature

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

Added support for moving Hybrid table watermarks (#1659 by @mkmkme)

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

Regression jobs to run:

Antalya 26.1 - Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks

github-actions · 2026-05-02T11:58:19Z

Workflow [PR], commit [ee97139]

arthurpassos · 2026-05-04T16:25:55Z

+    const ASTPtr & base_predicate, const std::vector<StorageDistributed::HybridSegment> & segs)
+{
+    std::unordered_map<String, String> result;
+    std::function<void(const ASTPtr &)> walk = [&](const ASTPtr & node)


This function looks very similar to replace_hybrid_params, maybe there is a way to reduce the code duplication?

I see this function only validates it, which seems to be a copy and paste of replace_hybrid_params without the replace logic. Maybe you can clone the original ast and just call replace? This will also make the maintainance better

I tried to address it in 9c92e1d by:

Moving the checks into a separate function

Moving the iterating routine into a visitor

arthurpassos · 2026-05-04T16:31:32Z

+                const auto & param_name = name_lit->value.safeGet<String>();
+                const auto & type_name = type_lit->value.safeGet<String>();
+
+                if (!watermark_snapshot)


Can't this be checked on line 1234? Right after auto watermark_snapshot = hybrid_watermark_params.get();

It can, but in this case we won't have a watermark name to put into the exception message

arthurpassos · 2026-05-04T16:33:53Z



+/// Extract declared hybridParam types from all Hybrid predicate ASTs.
+static std::unordered_map<String, String> collectHybridParamTypes(


Does it really extract types? It seems to me it actually builds a map from the hybrid args, not only type names.

The map stores {name1: type1, ...} so I don't really see the conflict here. I mean, yeah, it does build a map for the hybrid args, but it's only the mapping from the name to the type.

arthurpassos · 2026-05-04T16:53:28Z

+            if (auto * func = node->as<ASTFunction>(); func && func->name == "hybridParam")
+            {
+                auto * arg_list = func->arguments ? func->arguments->as<ASTExpressionList>() : nullptr;
+                if (!arg_list || arg_list->children.size() != 2)
+                    throw Exception(ErrorCodes::BAD_ARGUMENTS,
+                        "hybridParam() requires exactly 2 arguments: (name, type)");
+
+                auto * name_lit = arg_list->children[0]->as<ASTLiteral>();
+                auto * type_lit = arg_list->children[1]->as<ASTLiteral>();
+                if (!name_lit || name_lit->value.getType() != Field::Types::String)
+                    throw Exception(ErrorCodes::BAD_ARGUMENTS,
+                        "hybridParam() first argument (name) must be a string literal");
+                if (!type_lit || type_lit->value.getType() != Field::Types::String)
+                    throw Exception(ErrorCodes::BAD_ARGUMENTS,
+                        "hybridParam() second argument (type) must be a string literal");
+
+                const auto & param_name = name_lit->value.safeGet<String>();
+                const auto & type_name = type_lit->value.safeGet<String>();


IMHO we should abstract away these checks. Searching for type_lit->value.getType() != Field::Types::String shows 3 occurrences.

…atermark

arthurpassos

lgtm

…atermark

alsugiliazova · 2026-05-11T14:12:54Z

Verification report: Altinity/ClickHouse PR #1723

PR: #1723
Title: Antalya-26.3 frontport of #1659 — Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks
Base: antalya-26.3 · State: OPEN, mergeable
Head SHA: ee97139559e9a1849045aa6133825e4a026b8bc9
Diff: +862 / -13 across 8 files · Labels: antalya, port-antalya, hybrid, antalya-26.3, forwardport.
CI run analyzed: latest run on PR head.
Sources: GitHub check-runs API + `gh-data`.checks + `gh-data`.clickhouse_regression_results.

Conclusion

CI red on head, but every failure is either a pre-existing flake or a regression-suite scenario already broken at baseline on antalya-26.3. No PR-caused regression found. Safe to merge.

Caveat — partial frontport (same as #1640 / #1646 / #1687 / #1694). Companion antalya-26.1 frontports are still missing on antalya-26.3; chronic regression failures here are branch-level missing-dependency symptoms. Note that this run does not include the Iceberg suites in failure (the iceberg jobs ran clean for this set), so the typical --iceberg_partition_timezone missing-dep signal is absent — but the same other chronic baselines (swarms, parquet, s3_export_partition) fire as expected.

CI on head `ee971395` — failures

PR test workflow (3 failed checks, 42 success)

Check	Test FAIL	Class
`Stateless tests (amd_debug, sequential)`	`00157_cache_dictionary`	Pre-existing flake — 106 / 25 PRs / 90d
`Stateless tests (arm_asan, azure, sequential, 2/2)`	`03443_shared_storage_snapshots`	Pre-existing flake — 30 / 18
`Stress test (arm_asan)`	`Unknown error` (job-level)	Pre-existing instability — 5 occurrences / 5 PRs / 30d on `antalya-26.3`

Regression workflow (8 failed checks)

Check	Top failing tests on PR-1723 builds (30d)	Baseline (`antalya-26.3`, 30d)	Class
`Swarms` (Release + Aarch64)	`node failure / initiator out of disk space` (×8), `swarm joins / join clause` (×8)	78% / 49%	Pre-existing broken
`S3Export (partition)` (Release + Aarch64)	`sanity / mismatched columns`, `partition export tight pool lock inside task`, `no partition by` (×8 each)	50–60%	Pre-existing broken
`S3Export (part)` (Release + Aarch64)	`/s3` suite-level fails	flaky	Pre-existing flaky
`Parquet` (Release + Aarch64)	postgresql/mysql round-trip compression-type variants (×8 each)	~36%	Pre-existing flaky

Regression DB on /PRs/1723/ builds (30d): 751 Fail / 23,591 OK ≈ 3.1%. Every top failure matches the all-PR baseline fail rate on antalya-26.3.

Related to PR diff?

PR is the 26.3 frontport of upstream #1659 — ALTER TABLE MODIFY SETTING for Hybrid table watermarks (8 files in Hybrid storage / settings plumbing).

Failing test	Diff overlap	Related?
`00157_cache_dictionary`, `03443_shared_storage_snapshots`	none	No
Stress arm_asan `Unknown error`	none — recurring on 5 unrelated PRs on `antalya-26.3`	No
`swarms / `, `parquet / `, `s3_export_partition / `, `s3_export_part / `	none (Hybrid watermarks ≠ swarm clusters / parquet round-trip / export-partition)	No

No failing test intersects the Hybrid-watermark code path or fails above the all-PR baseline.

Recommendations

Safe to merge. Rerunning the 3 flaky PR-test jobs may clear them, but it is not blocking.
Re-verify after the companion 26.1 → 26.3 frontports land (Antalya 26.1; Remote initiator improvements 2 #1608/Antalya 25.8: Remote initiator improvements 2 #1638, Fix attempt to create table from table function #1701, Cherry-pick: Fix: system.databases always shows data lake catalog databases #1690) — the chronic regression failures should drop noticeably once those land.
The Stress arm_asan Unknown error recurring across 5 PRs on antalya-26.3 since late April is worth one shared investigation/tracking issue if not already filed.
Same chronic-baseline cleanup recommendation as VERIFICATION_PR_1640.md for swarms / parquet / s3_export_partition scenarios.

Local checkout

cd /Users/alsugilyazova/workspace/altinity-clickhouse/ClickHouse
gh pr checkout 1723 --repo Altinity/ClickHouse
# HEAD: ee97139559e9a1849045aa6133825e4a026b8bc9

alsugiliazova · 2026-05-11T14:13:12Z

PR #1723 audit — Hybrid watermarks `ALTER TABLE ... MODIFY SETTING`

PR: Antalya-26.3 Frontport of #1659 - Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks

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

Audit update for PR #1723 (Hybrid hybrid_watermark_* engine settings, runtime snapshot via MultiVersion, query-time substitution in StorageDistributed::read, ALTER validation and loadHybridWatermarkParams):

Confirmed defects

No confirmed defects in reviewed scope.

Merge pull request #1659 from Altinity/mkmkme/hybrid/alter-watermark

e8e1bbf

Antalya 26.1 - Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks

mkmkme added antalya hybrid antalya-26.3 forwardport This is a frontport of code that existed in previous Antalya versions labels May 2, 2026

arthurpassos reviewed May 4, 2026

View reviewed changes

Merge branch 'antalya-26.3' into frontport/antalya-26.3/hybrid-move-w…

488e6af

…atermark

svb-alt added the port-antalya PRs to be ported to all new Antalya releases label May 5, 2026

StorageDistributed: reduce code duplication

9c92e1d

arthurpassos approved these changes May 7, 2026

View reviewed changes

Merge branch 'antalya-26.3' into frontport/antalya-26.3/hybrid-move-w…

ee97139

…atermark

alsugiliazova added the verified Approved for release label May 11, 2026

This was referenced May 11, 2026

Antalya 26.3: DataLakeCatalog: avoid full catalog read for UNKNOWN_TABLE typo hints #1675

Merged

Antalya 26.3: Enabled experimental datalake catalogues by-default #1743

Merged

zvonand merged commit 4403244 into antalya-26.3 May 12, 2026
290 of 313 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Antalya-26.3 Frontport of #1659 - Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks#1723

Antalya-26.3 Frontport of #1659 - Add ALTER TABLE MODIFY SETTING support for Hybrid watermarks#1723
zvonand merged 4 commits into
antalya-26.3from
frontport/antalya-26.3/hybrid-move-watermark

mkmkme commented May 2, 2026

Uh oh!

github-actions Bot commented May 2, 2026 •

edited

Loading

Uh oh!

arthurpassos May 4, 2026

Uh oh!

arthurpassos May 4, 2026

Uh oh!

mkmkme May 6, 2026

Uh oh!

arthurpassos May 4, 2026

Uh oh!

mkmkme May 5, 2026

Uh oh!

arthurpassos May 4, 2026

Uh oh!

mkmkme May 5, 2026

Uh oh!

arthurpassos May 4, 2026

Uh oh!

arthurpassos left a comment

Uh oh!

alsugiliazova commented May 11, 2026

Uh oh!

alsugiliazova commented May 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants



		/// Extract declared hybridParam types from all Hybrid predicate ASTs.
		static std::unordered_map<String, String> collectHybridParamTypes(

Conversation

mkmkme commented May 2, 2026

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 May 2, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arthurpassos left a comment

Choose a reason for hiding this comment

Uh oh!

alsugiliazova commented May 11, 2026

Verification report: Altinity/ClickHouse PR #1723

Conclusion

CI on head ee971395 — failures

PR test workflow (3 failed checks, 42 success)

Regression workflow (8 failed checks)

Related to PR diff?

Recommendations

Local checkout

Uh oh!

alsugiliazova commented May 11, 2026

PR #1723 audit — Hybrid watermarks ALTER TABLE ... MODIFY SETTING

Confirmed defects

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions Bot commented May 2, 2026 •

edited

Loading

CI on head `ee971395` — failures

PR #1723 audit — Hybrid watermarks `ALTER TABLE ... MODIFY SETTING`