Workflow [PR], commit [d21e3ad]

Summary: ✅

• Performance Comparison: Performance dashboard

AI Review

Summary

The PR fixes countmin selectivity for Float32 columns compared with Float64 literals by allowing statistics on narrowed floating literals and by hashing numeric values through a temporary column. The in-range Float32 case is covered, but two selectivity-contract issues remain: non-representable Float64 literals still do not become exact ALWAYS_FALSE / ALWAYS_TRUE, and the new temporary-column hash path can still estimate an out-of-range small numeric constant as a different stored value.

Findings

⚠️ Majors

• [src/Storages/Statistics/ConditionSelectivityEstimator.cpp:382] [dismissed by author -- https://github.com/ClickHouse/ClickHouse/pull/105047#discussion_r3249263138] equals / notEquals on a Float32 column with a non-representable Float64 literal still falls back to unknown selectivity instead of the exact result. The new round-trip branch is after tryConvertFieldToType(const_value, *column_type, ...); for values such as 0.1 or out-of-range Float64, that conversion returns Null, so the function returns false before setting ALWAYS_FALSE / ALWAYS_TRUE. This keeps impossible predicates (or always-true notEquals) from being represented accurately for PREWHERE ordering. Handle the lossy/null conversion before returning: for equals, set ALWAYS_FALSE; for notEquals, set ALWAYS_TRUE; then add a focused regression with Float32 and a non-representable literal.
• [src/Storages/Statistics/StatisticsCountMinSketch.cpp:50] estimateEqual can still hash a value that was not actually converted to the statistics type. convertFieldToType(val, *data_type, data_type.get()) returns the original Field immediately because the source hint equals the target type, so a predicate such as UInt8 STATISTICS(countmin) with u = 256 reaches the temporary UInt8 column as UInt64(256), gets truncated to 0, and estimates matches for 0 instead of returning zero for an impossible predicate. Range-check without the self type hint, or pass the actual constant type, before inserting into the temporary column.

Tests

⚠️ Add a stateless regression for Float32 countmin statistics with f = 0.1 and f != 0.1, proving the estimator emits exact false/true selectivity rather than the unknown default.

⚠️ Add a focused countmin regression for a small numeric type such as UInt8 with an out-of-range equality literal, proving estimateEqual returns zero instead of hashing the truncated stored representation.

Final Verdict

Status: ⚠️ Request changes

Minimum required actions: fix the lossy Float64 -> Float32 equality path before the early return, fix StatisticsCountMinSketch::estimateEqual so unrepresentable constants do not get materialized as truncated values, and add the focused regressions above.

This was fixed by #105146. Let's update the branch.

LLVM Coverage Report

Changed lines: Changed C/C++ lines covered by tests: 23/29 (79.31%) | Lost baseline coverage: none · Uncovered code

Full report · Diff report