Optimize string conversion comparison

[punithns97]

Closes: #108955

Changelog category (leave one):

• Performance Improvement

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

Type-based pruning of expressions involving toString

[clickhouse-gh]

Workflow [PR], commit [9f45d6e]

Summary: ❌

job_name	test_name	status	info	comment
Build (arm_tidy)		FAIL
	Build ClickHouse	FAIL	cidb
Fast test (arm_darwin)		DROPPED
Build (amd_debug)		DROPPED
Build (amd_asan_ubsan)		DROPPED
Build (amd_tsan)		DROPPED
Build (amd_msan)		DROPPED
Build (amd_binary)		DROPPED
Build (arm_debug)		DROPPED
Build (arm_asan_ubsan)		DROPPED
Build (arm_tsan)		DROPPED

---

AI Review

Summary

This PR adds OptimizeStringConversionComparisonPass to prune impossible string-constant comparisons over toString / CAST(..., 'String') and to rewrite eligible tuple substring searches into per-element predicates. The optimization is useful, but the current implementation changes semantics in two cases: it folds invalid LIKE / ILIKE patterns into constants instead of preserving the runtime exception, and it rewrites tuple searches for quoted-and-escaped types beyond plain String, producing false negatives for Dynamic, Variant, and JSON values.

PR Metadata

Changelog category is correct: this is a Performance Improvement.

Changelog entry is required and too narrow for the actual change: it mentions only pruning and omits the tuple-destructuring/text-index optimization.

Suggested replacement: Optimize toString string-constant comparisons by pruning impossible matches and rewriting eligible tuple substring searches into per-element predicates.

Findings

❌ Blockers

• [src/Analyzer/Passes/OptimizeStringConversionComparisonPass.cpp:214] extractLikeRequiredChars treats a trailing backslash as a literal byte, but ClickHouse rejects LIKE / ILIKE patterns that end with an escape character. As a result, queries such as toString(number) LIKE '%\\' or NOT LIKE '%\\' can be constant-folded to 0 / 1 instead of raising CANNOT_PARSE_ESCAPE_SEQUENCE. Suggested fix: validate the pattern with the existing LIKE parser before pruning, or make this helper refuse invalid trailing escapes.
• [src/Analyzer/Passes/OptimizeStringConversionComparisonPass.cpp:512] The tuple-destructuring guard only recognizes String / FixedString elements as quoted-and-escaped, but Dynamic, Variant, and JSON also use serializeTextQuoted inside tuples while scalar toString uses raw serializeText. For a tuple element storing a<newline>b, toString((x, 1)) LIKE '%nb%' matches the escaped tuple rendering and is rewritten to a per-element LIKE that returns false on the raw value. Suggested fix: extend the guard to every type whose quoted serialization can introduce escapes, or conservatively skip destructuring for those types.

Tests

• ⚠️ Add a stateless regression for invalid patterns such as toString(number) LIKE '%\\' and NOT LIKE '%\\' to prove the new pass preserves the existing exception path.
• ⚠️ Add a tuple-destructuring regression for at least one quoted-and-escaped non-String carrier (Dynamic, Variant, or JSON) containing a newline or quote so the rewrite is proven equivalent there too.

Final Verdict

❌ Changes requested.

[clickhouse-gh]

LIKE patterns ending with a backslash are supposed to raise CANNOT_PARSE_ESCAPE_SEQUENCE, but extractLikeRequiredChars treats the trailing \ as a literal required byte. That means toString(number) LIKE '%\\' can be pruned to 0 (and NOT LIKE to 1) instead of preserving the existing exception. Can we validate or reject invalid patterns before using this helper for pruning?

[clickhouse-gh]

The escape-sequence guard here only recognizes String / FixedString, but tuple elements of type Dynamic, Variant, and JSON also go through serializeTextQuoted inside tuples while scalar toString uses raw serializeText. For a Dynamic string value a\nb, toString((x, 1)) LIKE '%nb%' matches the escaped tuple rendering, while the rewritten toString(x) LIKE '%nb%' runs on the raw newline and returns false. This rewrite needs to exclude every type whose quoted serialization can inject escapes, not just plain strings.

[clickhouse-gh]

📊 Cloud Performance Report

✅ AI verdict: no_change — no significant changes across 37 queries analysed

This PR adds a single analyzer pass (OptimizeStringConversionComparisonPass) that only rewrites expressions of the form op(toString(x), 'constant') where op is equals/like/ilike/position over restricted-alphabet types like numbers, dates and tuples of them. None of the flagged clickbench queries (Q8 aggregation, Q18, Q34) use that idiom, so the pass cannot plausibly change their execution time. The flagged +10.8% (Q8) and +10.4% (Q18) are off the changed code path — Q8's source runs were also extremely noisy — so both were downgraded to not_sure, and Q34 was already correctly held within master's variance band.

clickbench

⚠️ 3 inconclusive

Flagged queries (3 of 43)

	Query	Verdict	Baseline median (ms)	PR median (ms)	Change	q-value	Hint
⚠️	8	not_sure	325	360	+10.8%	<0.0001	This PR only adds an analyzer pass that fires on toString(numeric/date) LIKE/=/position patterns; Q8's AdvEngineID aggregation never uses them, so the +10.8% is off-path with very noisy source runs.
⚠️	18	not_sure	1251	1381	+10.4%	<0.0001	The new pass only rewrites toString(restricted-type) string comparisons, which this query does not use; the +10.4% is unrelated to the diff and reads as run-to-run variance.
⚠️	34	not_sure	1233	1304	+5.8%	<0.0001	Deterministically kept within master's variance band; this query uses no toString(...) string comparisons, so the +5.8% is not attributable to the diff.

_{Change = percent below ×2; the ratio of medians (×N faster/slower) beyond, where percent understates the scale. q-value = BH-FDR adjusted p; smaller is stronger evidence. MIRAI flags a query when q < fdr_q (default 0.10) — the value the verdict is based on.}

tpch_adapted_1_official

🟢 No significant changes

Debug info

• StressHouse run: 61d76eee-2578-4402-bc46-174502b02f94
• MIRAI run: c9f0603f-fe7c-4079-a057-d08c6cb60c93
• PR check IDs:
  • clickbench_379798_1786233034
  • clickbench_379804_1786233034
  • clickbench_379815_1786233034
  • tpch_adapted_1_official_379826_1786233034
  • tpch_adapted_1_official_379839_1786233034
  • tpch_adapted_1_official_379869_1786233034