| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
…tions
Both broken identities in the catalog get the NULL/empty boundary backwards, in
opposite directions, and both produce false positives whenever the generated
condition selects no row - which the fuzzer hits constantly, since a random
predicate over a small generated table is often false or NULL everywhere.
sumIf: the rewrite was `sum(if(cond, x, 0))`. With
aggregate_functions_null_for_empty=1, which this identity pins, `sumIf` returns
NULL when the condition holds for no row, while the `0` else branch feeds a real
value into `sum` for every row and returns 0. Its siblings `minIf` and `maxIf`
already use `NULL` as the else branch; `sumIf` was the odd one out. Measured on
26.9:
SELECT sumIf(c, c IS NOT NULL) FROM (SELECT NULL AS c) -- \N
SELECT sum(if(c IS NOT NULL, c, 0)) FROM (SELECT NULL AS c) -- 0 mismatch
SELECT sum(if(c IS NOT NULL, c, NULL)) FROM (SELECT NULL AS c) -- \N agrees
countIf: the rewrite was `sum(toUInt64(cond))`. This is the mirror mistake - when
`cond` is Nullable and NULL for every row (or the input is empty), the argument is
NULL throughout and `sum` returns NULL, while `countIf` returns 0. Guard the
condition with `ifNull(cond, 0)`, which also matches countIf's own treatment of a
NULL condition as false and is a no-op when the condition is already non-Nullable:
SELECT countIf(c) FROM (SELECT NULL AS c) -- 0
SELECT sum(toUInt64(c)) FROM (SELECT NULL AS c) -- \N mismatch
SELECT sum(toUInt64(ifNull(c, 0))) FROM (SELECT NULL AS c) -- 0 agrees
Both replacements were checked against the cases the fuzzer can produce: empty
input, condition false for every row, condition NULL for every row, value NULL
under a true condition, and mixed rows including negatives and zero. The other
four identities (minIf, maxIf, sumOrNull, avgOrNull) agree in all of them and are
left alone.
The two exact-string assertions that pinned the old forms are updated, plus two
new tests that lock the property rather than the string: every `if`-family rewrite
must use NULL as its else branch, and the countIf rewrite must null-guard its
condition.
This is the ClickHouse-side finding it was producing (SQLancer nightly,
2026-08-25, TLPCombinator, 1 of 87 findings):
First query : SELECT sumIf(right_1.c0, (right_1.c0 IS NOT NULL)) FROM ...
Second query: SELECT sum(if((right_1.c0 IS NOT NULL), right_1.c0, 0)) FROM ...
It misses: "[0]"
The two tests added in the previous commit were wrapped by hand and do not match what formatter-maven-plugin:2.20.0 produces, which the build's eclipseformat validate goal checks.
|
@qoega could you review this one? A formal review request is not possible here - GitHub only allows them from collaborators on this repository, and you are not one on ClickHouse/sqlancer yet. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Two of the six identities in ClickHouseCombinatorIdentities.CATALOG do not hold when the generated
condition selects no row — which the fuzzer hits constantly, since a random predicate over a small
generated table is often false or NULL everywhere. They get the NULL/empty boundary wrong in
opposite directions.
sumIf — the rewrite was sum(if(cond, x, 0)). With aggregate_functions_null_for_empty=1,
which this identity pins, sumIf returns NULL when the condition holds for no row, while the 0
else branch feeds a real value into sum for every row and returns 0. minIf and maxIf already
use NULL here; sumIf was the odd one out.
countIf — the rewrite was sum(toUInt64(cond)), the mirror mistake. When cond is Nullable
and NULL for every row (or the input is empty) the argument is NULL throughout, so sum returns
NULL while countIf returns 0. ifNull(cond, 0) fixes it, matches countIf's own treatment of a
NULL condition as false, and is a no-op when the condition is already non-Nullable.
Measured on 26.9:
Both replacements were checked against every case the fuzzer can produce: empty input, condition
false for every row, condition NULL for every row, value NULL under a true condition, and mixed rows
including negatives and zero. The other four identities (minIf, maxIf, sumOrNull, avgOrNull)
agree in all of them and are left alone.
This is the finding it was producing, from the SQLancer nightly of 2026-08-25 (TLPCombinator, 1 of
that run's 87 findings):
First query : SELECT sumIf(right_1.c0, (right_1.c0 IS NOT NULL)) FROM t0 AS left FULL OUTER JOIN t0 AS right_0 ON ... RIGHT ANTI JOIN t0 AS right_1 ON ... SETTINGS aggregate_functions_null_for_empty=1, ... Second query: SELECT sum(if((right_1.c0 IS NOT NULL), right_1.c0, 0)) FROM ... It misses: "[0]"The joins are incidental — SELECT sumIf(c, c IS NOT NULL) FROM (SELECT NULL AS c) shows it with no
join at all.
The two exact-string assertions that pinned the old forms are updated, and two new tests lock the
property rather than the string: every if-family rewrite must use NULL as its else branch, and the
countIf rewrite must null-guard its condition. ClickHouseCombinatorIdentitiesTest passes 10/10
on JDK 25.