Skip to content

Commit 5f380e8

Browse files
authored
fix(sql): avoid uint ndv overflow in scan stats (#19632)
* fix(sql): avoid uint ndv overflow in scan stats (#19555) * chore: ignore local toolchain artifacts
1 parent 8419b41 commit 5f380e8

3 files changed

Lines changed: 65 additions & 8 deletions

File tree

.gitignore

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,14 @@
88
.bash_history
99
.mysql_history
1010

11+
1112
# Rust
1213
target/
14+
.cargo-home/
15+
.cargo-target/
16+
.toolchain-bin/
17+
.worktrees/
18+
1319

1420
# OS related
1521
### MacOS ###

src/query/sql/src/planner/plans/scan.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,19 @@ impl Scan {
175175
}
176176
}
177177

178+
fn reduce_ndv_by_datum_range(ndv: Ndv, min: &Datum, max: &Datum) -> Ndv {
179+
match (max, min) {
180+
(Datum::UInt(m), Datum::UInt(n)) if m >= n => {
181+
ndv.reduce(m.saturating_sub(*n).saturating_add(1) as _)
182+
}
183+
(Datum::Int(m), Datum::Int(n)) if m >= n => {
184+
ndv.reduce(m.saturating_add(1).saturating_sub(*n) as _)
185+
}
186+
_ if max == min => Ndv::Stat(1.0),
187+
_ => ndv,
188+
}
189+
}
190+
178191
impl PartialEq for Scan {
179192
fn eq(&self, other: &Self) -> bool {
180193
self.table_index == other.table_index
@@ -282,14 +295,7 @@ impl Operator for Scan {
282295
};
283296

284297
// Alter ndv based on min and max if the datum is uint or int.
285-
let ndv = match (&max, &min) {
286-
(Datum::UInt(m), Datum::UInt(n)) if m >= n => ndv.reduce((m - n + 1) as _),
287-
(Datum::Int(m), Datum::Int(n)) if m >= n => {
288-
ndv.reduce(m.saturating_add(1).saturating_sub(*n) as _)
289-
}
290-
_ if max == min => Ndv::Stat(1.0),
291-
_ => ndv,
292-
};
298+
let ndv = reduce_ndv_by_datum_range(ndv, &min, &max);
293299

294300
let histogram = if let Some(histogram) = self.statistics.histograms.get(k)
295301
&& histogram.is_some()
@@ -367,3 +373,19 @@ impl Operator for Scan {
367373
))
368374
}
369375
}
376+
377+
#[cfg(test)]
378+
mod tests {
379+
use super::*;
380+
381+
#[test]
382+
fn test_reduce_ndv_by_uint_full_range_saturates() {
383+
let reduced = reduce_ndv_by_datum_range(
384+
Ndv::Stat((u64::MAX as f64) + 1.0),
385+
&Datum::UInt(0),
386+
&Datum::UInt(u64::MAX),
387+
);
388+
389+
assert_eq!(reduced.value(), u64::MAX as f64);
390+
}
391+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# https://github.com/databendlabs/databend/issues/19555
2+
3+
statement ok
4+
create or replace database issue_19555;
5+
6+
statement ok
7+
use issue_19555;
8+
9+
statement ok
10+
drop table if exists t;
11+
12+
statement ok
13+
create table t as
14+
select d::UInt64 as d
15+
from (values
16+
(0),
17+
(18446744073709551615)
18+
) as v(d);
19+
20+
query T
21+
select d::String from t order by d;
22+
----
23+
0
24+
18446744073709551615
25+
26+
query I
27+
select count(*) from t;
28+
----
29+
2

0 commit comments

Comments
 (0)