Skip to content

Treat LowCardinality(String) like String in empty-string comparison rewrite - #2611

Merged
zvonand merged 1 commit into
antalya-26.8from
bugfix/antalya-26.8/empty-string-low-cardinality
Oct 20, 2026
Merged

Treat LowCardinality(String) like String in empty-string comparison rewrite#2611
zvonand merged 1 commit into
antalya-26.8from
bugfix/antalya-26.8/empty-string-low-cardinality

Conversation

@ianton-ru

@ianton-ru ianton-ru commented Oct 19, 2026

Copy link
Copy Markdown

Frontport of #2565 onto antalya-26.8.

A Distributed table can declare LowCardinality(String) over a plain String column. optimize_empty_string_comparisons rewrote s != '' to notEmpty only for the underlying String, and object_storage_cluster then failed with NOT_FOUND_COLUMN_IN_BLOCK. The rewrite now treats LowCardinality(String) like String.

Related: #2565

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):

Treat LowCardinality(String) like String in empty-string comparison rewrite.

Documentation entry for user-facing changes

...

CI/CD Options

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Unit tests
  • Performance tests
  • Aarch64 tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All Regression
  • Disable CI Cache

Regression jobs to run:

  • Fast suites (mostly <1h)
  • Aggregate Functions (2h)
  • Alter (1.5h)
  • Benchmark (30m)
  • CAS (content-addressed storage; Antalya only)
  • ClickHouse Keeper (1h)
  • Iceberg (2h)
  • LDAP (1h)
  • OAuth (5m)
  • Parquet (1.5h)
  • RBAC (1.5h)
  • SSL Server (1h)
  • S3 (2h)
  • S3 Export (2h)
  • Swarms (30m)
  • Tiered Storage (2h)

Made with Cursor

@ianton-ru @cursoragent
Treat LowCardinality(String) like String in empty-string comparis…
cc782c3 
…on rewrite.

A `Distributed` table can declare `LowCardinality(String)` over a plain `String` column. `optimize_empty_string_comparisons` then rewrote only the underlying schema, and `object_storage_cluster` failed with `NOT_FOUND_COLUMN_IN_BLOCK`.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ianton-ru ianton-ru mentioned this pull request Oct 19, 2026
Merged
9 of 30 tasks
@github-actions

github-actions Bot commented Oct 19, 2026

Copy link
Copy Markdown

Workflow [PR], commit [cc782c3]

@ianton-ru ianton-ru added port-antalya PRs to be ported to all new Antalya releases antalya-26.8 Session label (releasy session config) forwardport This is a frontport of code that existed in previous Antalya versions labels Oct 19, 2026
@ianton-ru

ianton-ru commented Oct 19, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T08:43:49.272680Z cc782c3 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Failed to set up container
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ianton-ru

ianton-ru commented Oct 19, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Failed to set up container
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ianton-ru

ianton-ru commented Oct 19, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Failed to set up container
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ianton-ru

ianton-ru commented Oct 19, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 19, 2026

Copy link
Copy Markdown

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Failed to set up container
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ianton-ru

ianton-ru commented Oct 20, 2026

Copy link
Copy Markdown
Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: cc782c375b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@zvonand zvonand mentioned this pull request Oct 20, 2026
Open
56 tasks done
@ianton-ru

ianton-ru commented Oct 20, 2026

Copy link
Copy Markdown
Author

@blau-ai

@blau-ai

blau-ai commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

CI triage for this PR

Verdict: 5 red checks, 0 caused by this PR. All of them reproduce on sibling PRs that don't touch the analyzer, so they're pre-existing infra / known-flaky regression suites. Everything that actually exercises this change is green.

Evidence SHA: cc782c375bce698a47c2d35e98fab961c07eef87 (praktika PR report).

Not PR-caused

Check What happened Why it's not this PR
Parser memory check (ERROR, "Failures: 1/2") Errored at the first sub-step Resolve master binary: No master clickhouse-examples artifact was found. The actual parser-memory comparison (step 2/2) never ran. It's a baseline-resolution/infra problem — the job couldn't fetch the master clickhouse-examples binary to diff against. Reproduces identically (Failures: 1/2, same "Resolve master binary" error) on the unrelated Iceberg PR #2610.
Grype Scan clickhouse-keeper (4 high/critical) Container-image CVE scan of the published keeper image. Scans base-image packages, not C++ source. The unrelated PR #2610 shows the identical 4 high/critical keeper result.
Grype Scan clickhouse-server …-alpine (1 high/critical) Container-image CVE scan of the alpine server image. Same as above — image/base-layer CVEs, independent of an analyzer-pass change.
Regression release oauth ("1 feature (1 failed)", 9/112 scenarios) Altinity OAuth TestFlows suite. This PR touches only the empty-string-comparison analyzer pass — nothing in auth. The OAuth suite fails the same way on unrelated PR #2607 (Unity-catalog backport).
Regression release swarms ("1 module (1 failed)", 3/1535 scenarios) Altinity Swarms TestFlows suite. Fails on unrelated PRs #2607, #2616, and the already-merged #2613 — clearly pre-existing on the antalya-26.8 line, not introduced here.

What's actually green (the parts this PR affects)

  • Builds: amd_debug / amd_asan_ubsan / amd_binary / amd_release / arm_release / wasm_parser — all 0 failures.
  • Fast test: Failed: 0 (11361 passed).
  • Stateless tests: all green, including amd_debug, distributed plan, s3 storage (parallel + sequential) and amd_debug parallel — the exact area this change targets. The new test 05303_optimize_empty_string_comparisons_distributed_low_cardinality runs under the debug-parallel suite, which reported Failed: 0.
  • Integration tests (asan_ubsan, old analyzer, 8 shards): 0 failures.
  • Unit tests, Stress tests, AST fuzzer, BuzzHouse, SQLLogic, SQLStorm, Compatibility, Install packages: all green.

The fix itself is sound: isStringOrFixedString(removeLowCardinality(expr_type)) with the added #include <DataTypes/DataTypeLowCardinality.h> is the standard way to let the rewrite treat LowCardinality(String) like String, and it's covered by the new stateless test (which passed).

Suggested next step

No code change needed for CI on this PR — none of the red checks are attributable to it. If you want a clean board before merge, just re-run RegressionTestsRelease (oauth/swarms are flaky) and Parser memory check (it needs the master clickhouse-examples baseline artifact to be available); the Grype image-CVE failures are a repo-wide base-image issue tracked independently.

(Analysis only — I did not build or run ClickHouse; verdicts are from CI reports and cross-PR comparison.)

@zvonand
zvonand merged commit 5474a43 into antalya-26.8 Oct 20, 2026
281 of 292 checks passed
@zvonand zvonand added the verified Approved for release label Oct 20, 2026
@zvonand zvonand mentioned this pull request Oct 20, 2026
Closed
8 of 27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

antalya antalya-26.8 Session label (releasy session config) forwardport This is a frontport of code that existed in previous Antalya versions port-antalya PRs to be ported to all new Antalya releases verified Approved for release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ianton-ru @blau-ai @zvonand @svb-alt