Do not prune by a min/max range when a positive IN set holds NaN - #117453
Do not prune by a min/max range when a positive IN set holds NaN#117453alexey-milovidov wants to merge 1 commit into
IN set holds NaN#117453Conversation
`IN` matches `NaN` bit-exactly - `SELECT nan IN (nan)` is `1` - unlike a
comparison, which is always false for `NaN`. But min/max-based pruning works
with ranges produced by `IColumn::getExtremes`, which deliberately skips
`NaN`, so a part or granule that holds `NaN` next to finite values gets a
`NaN`-free range, the set-vs-range intersection finds no overlap, and it is
pruned even though row-wise evaluation of the same filter matches those rows.
Both arms fire at pure default settings on a plain `MergeTree` table -
automatic `Basic` statistics are materialized on `INSERT` and
`use_statistics_for_part_pruning` defaults to `1`:
CREATE TABLE s1 (k UInt64, f Float64) ENGINE = MergeTree ORDER BY k;
SYSTEM STOP MERGES s1;
INSERT INTO s1 SELECT number, if(number < 13, nan, 1.5) FROM numbers(100000);
INSERT INTO s1 SELECT number + 200000, 2.5 FROM numbers(1000);
SELECT count() FROM s1 WHERE f IN (nan); -- 0, the truth is 13
SELECT count() FROM s1 WHERE f IN (nan, 2.5); -- 1000, the truth is 1013
The fix for #113417 taught the statistics pruner to skip a floating-point
column under a negating function, on the assumption that only a negation can
make a floating-point predicate true for `NaN`. A positive `IN` is the
counterexample: it is a set-membership test, not a comparison. It now skips
such a column too - whether the set actually holds a `NaN` is not knowable
there, since it may come from a subquery.
`KeyCondition` can inspect the elements, so it declines the set atom only
when the set really contains a `NaN` and a key column is floating-point. The
atom serves every index at once, so declining it also forgoes the primary key
for that predicate, which is the same trade `has` already makes for
floating-point arrays.
Closes: #116927
Related: #113417
|
Workflow [PR], commit [4440da6] Summary: ❌
AI ReviewSummaryThe PR fixes the wrong-result case where a positive Findings
Tests
Final Verdict
LLVM Coverage ReportMeasured on commit 4440da6.
Changed lines: Changed C/C++ lines covered: 41/49 (83.67%) · Uncovered code |
clickhouse-gh
Bot
added
the
pr-bugfix
![clickhouse-gh[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
| || (node.type == ActionsDAG::ActionType::FUNCTION && node.function_base | ||
| && isNegatingFunction(node.function_base->getName())); | ||
| && (isNegatingFunction(node.function_base->getName()) | ||
| || isSetMembershipFunction(node.function_base->getName()))); |
There was a problem hiding this comment.
This broadens the NaN exclusion from the genuinely unsafe cases to every positive set-membership predicate on a floating-point column. As soon as isSetMembershipFunction matches, StatisticsPartPruner drops the column from stats_column_name_to_type_map, so use_statistics_for_part_pruning no longer prunes safe queries like f IN (1.5) or a ready subquery set without NaN.
The branch already has the precise machinery to avoid that. StatisticsPartPruner::getKeyConditionForEstimates builds KeyCondition with require_ready_sets = true, so unbuilt subqueries are already declined safely, and this PR adds setElementsContainNaN in KeyCondition::tryPrepareSetIndexForIn, so ready sets that actually contain NaN are declined there too. By dropping the column earlier, we never reach that narrower check and regress every float IN back to a full part scan. Please keep the statistics-side exclusion limited to the negation family, and add an EXPLAIN indexes = 1 case for a float IN (1.5) query with use_statistics_for_part_pruning = 1 so this stays covered.
Build profile diff (arm_release)Comparing ✅ No significant changes. Binary sizes
The official master build is compiled with Compile time of recompiled translation units8 translation units recompiled, 23 s compile time in total, 8 of them have a recent master baseline. |
1 participant
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixed
WHERE f IN (nan)silently losing theNaNrows of a floating-point column: statistics-based part pruning and theminmaxskip index use ranges that excludeNaN, so a part or granule holdingNaNwas pruned even thoughINmatchesNaN.Documentation entry for user-facing changes
INmatchesNaNbit-exactly —SELECT nan IN (nan)is1— unlike a comparison, which is always false forNaN. But min/max-based pruning works with ranges produced byIColumn::getExtremes, which deliberately skipsNaN, so a part or granule that holdsNaNnext to finite values gets aNaN-free range, the set-vs-range intersection finds no overlap, and it is pruned even though row-wise evaluation of the same filter matches those rows.Both arms fire at pure default settings on a plain
MergeTreetable — automaticBasicstatistics are materialized onINSERTanduse_statistics_for_part_pruningdefaults to1:The fix for #113417 taught the statistics pruner to skip a floating-point column under a negating function, on the assumption that only a negation can make a floating-point predicate true for
NaN. A positiveINis the counterexample: it is a set-membership test, not a comparison. It now skips such a column too — whether the set actually holds aNaNis not knowable there, since it may come from a subquery.KeyConditioncan inspect the elements, so it declines the set atom only when the set really contains aNaNand a key column is floating-point. The atom serves every index at once, so declining it also forgoes the primary key for that predicate, which is the same tradehasalready makes for floating-point arrays.Closes: #116927
Related: #113417
Workflow [PR]
Sync PR [sync-upstream/pr/117453]