-
Notifications
You must be signed in to change notification settings - Fork 24.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ESQL: Comparing float/half_float to number fails if out of range #100130
Comments
Pinging @elastic/es-ql (Team:QL) |
Pinging @elastic/elasticsearch-esql (:Query Languages/ES|QL) |
Prevent pushing down filters for binary comparisons that would implicitly compare a byte/short/int with an out of range value. This leads to exceptions thrown by Lucene - instead, evaluate the filter in ESQL only. This does not cover the same problem for half_float/scaled_float/float, see elastic#100130. Closes elastic#99960.
In my opinion this is a bug in the field mapper, not in ES|QL. I would prefer fixing the field mapper than introducing a workaround in ES|QL (which makes things a bit more complicated unfortunately because we'll want to check if it's a breaking change or not). |
@jpountz , I think the field mapper is fine. I debugged the issue and confirmed that the data type that ESQL first sees is indeed |
Here's the relevant piece of code that turns the mapping into field attributes and "widens" data types. |
OK. If ES|QL has a natural way of handling this because it needs to check out types anyway, I'm good with this. But if we end up working around bah behaviors in mappings, let's fix the root cause directly. :) |
Pinging @elastic/es-analytics-geo (Team:Analytics) |
Some clarifications, from taking a look at this:
This doesn't actually fail for To be a bit more clear, a field type mapped as
With value as
having a more extended mapping covering multiple other floating point data types
None of the DSL queries fail with exceptions. They all return
At this point, I am not sure what and where we need a fix. Maybe it is good to provide a warning message to ESQL users telling them a math operation doesn't make sense, but at the same time Elasticsearch itself is inconsistent in how it's approaching the impossible math operations. @jpountz thank you for initially chiming in. Would you mind having a second look at my statement here and see if it does make sense from ES side of things? |
Heya, I need to go back on my statement that I think the field mapper is fine. Thanks again @jpountz for bringing up this angle as well. The behavior can be reproduced in ES directly, without going through ES|QL, by simply running a range query on e.g. a
This originates here, while parsing the range query for So, rather than working around this issue in ES|QL, we could indeed make range queries more permissive, although that would change the behavior of |
@alex-spies please raise an issue in ES replicating the problem and let's close this issue for now as it's not clear whether there's anything in ESQL to fix to begin with. |
Superseded by #105079 |
Elasticsearch Version
main@c24cc0f54c216a5bff8e6b0e3ad2d09f7a3eb956
Installed Plugins
No response
Java Version
openjdk version "17.0.8" 2023-07-18
OS Version
Ubuntu 23.04
Problem Description
For an index with a
float
orhalf_float
(probably alsoscaled_float
) field, the following query fails:This throws a
QueryShardException: failed to create query: [float] supports only finite values, but got [-Infinity]
.The culprit is the local physical plan optimization PushFiltersToSource. We should not push filters down to Lucene if the right hand side is out of range.
Unfortunately, we cannot decide during optimization whether this is the case at the moment, because
float
andhalf_float
datatypes are both interpreted asdouble
by ESQL. We need to either keep the initial data type around inFieldAttributes
or stop widening small numeric types.Steps to Reproduce
Run the query from above against any index that has a field
field
with data typefloat
orhalf_float
.Logs (if relevant)
The text was updated successfully, but these errors were encountered: