Skip to content

Wrongly use the function clamp #4654

@HaoYang670

Description

@HaoYang670

Describe the bug
The clamp function in the TDigest doesn't check lo <= hi, which makes it possible to use this function wrongly.
https://github.com/apache/arrow-datafusion/blob/4ec559d1db46afe0ff55eb215b128ab91ac783aa/datafusion/physical-expr/src/aggregate/tdigest.rs#L222-L230

To Reproduce
Steps to reproduce the behavior:

  1. add the assertion:
    fn clamp(v: f64, lo: f64, hi: f64) -> f64 {
        assert!(lo <= hi);
        if v > hi {
            hi
        } else if v < lo {
            lo
        } else {
            v
        }
    }
  1. run cargo test --all and you will get the failure message:
[aggregate.slt] Running query: "SELECT approx_median(a) FROM median_f64_nan"
thread 'main' panicked at 'assertion failed: lo <= hi', datafusion/physical-expr/src/aggregate/tdigest.rs:223:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: test failed, to rerun pass `-p datafusion --test sqllogictests`

Expected behavior

Additional context
Add any other context about the problem here.
Find in #4653

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions