Skip to content

Conversation

@b41sh
Copy link
Contributor

@b41sh b41sh commented Oct 24, 2025

This PR fix json range query with different Numerical type caused panic

thread 'query::range_query::range_query_fastfield::tests::json_range_test' (805162) panicked at src/query/range_query/range_query_fastfield.rs:261:64:
called `Option::unwrap()` on a `None` value

@PSeitz
Copy link
Collaborator

PSeitz commented Oct 24, 2025

I'd rather have the limitation that both bounds provided by the user have to be of the same type and otherwise return an error

Not sure it makes sense to have a range from i64 to f64

@b41sh
Copy link
Contributor Author

b41sh commented Oct 24, 2025

I'd rather have the limitation that both bounds provided by the user have to be of the same type and otherwise return an error

Not sure it makes sense to have a range from i64 to f64

@PSeitz Thanks for your review. Bounds may have different types in many scenarios, such as [-1 to 18446744073709551615], where the types are i64 and u64 respectively. We cannot convert them to a single type unless using i128.

@PSeitz
Copy link
Collaborator

PSeitz commented Oct 24, 2025

I'd rather have the limitation that both bounds provided by the user have to be of the same type and otherwise return an error
Not sure it makes sense to have a range from i64 to f64

@PSeitz Thanks for your review. Bounds may have different types in many scenarios, such as [-1 to 18446744073709551615], where the types are i64 and u64 respectively. We cannot convert them to a single type unless using i128.

In this case you would need to use f64 (same as the data would be coerced to on the column)

@b41sh
Copy link
Contributor Author

b41sh commented Oct 30, 2025

Hi @PSeitz PTAL, I re-implemented this part of the code, converting lower_bound and upper_bound to actual_column_type respectively

@PSeitz
Copy link
Collaborator

PSeitz commented Oct 30, 2025

Hi @PSeitz PTAL, I re-implemented this part of the code, converting lower_bound and upper_bound to actual_column_type respectively

I'd rather have the limitation that both bounds provided by the user have to be of the same type and otherwise return an error. This is a limitation of the API design currently, which should be fixed eventually.

@b41sh
Copy link
Contributor Author

b41sh commented Oct 30, 2025

Hi @PSeitz PTAL, I re-implemented this part of the code, converting lower_bound and upper_bound to actual_column_type respectively

I'd rather have the limitation that both bounds provided by the user have to be of the same type and otherwise return an error. This is a limitation of the API design currently, which should be fixed eventually.

i64, u64, and f64 are all number types, but they are resolved into different types during query parsing.
https://github.com/quickwit-oss/tantivy/blob/main/src/core/json_utils.rs#L296
For example, if the query is json.number:[10.0 TO 10.51], parsing results in lower_bound being u64 and upper_bound being f64. However, the user actually inputs f64-type numbers. This can lead to unexpected panics, so I think this is a bug that needs fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants