Remaining queries for long script field #59816
Conversation
Adds the `terms` and `range` queries for `long` typed `script` fields. It also fixes a bug in a few of `toString` tests for `keyword` typed `script` fields.
nik9000
added
the
:Search/Search
|
Pinging @elastic/es-search (:Search/Search) |
|
Looks like the test failure on this one is real. I'll have a look. |
I've pushed an update! |
| @@ -906,6 +880,51 @@ public static long objectToLong(Object value, boolean coerce) { | |||
| String stringValue = (value instanceof BytesRef) ? ((BytesRef) value).utf8ToString() : value.toString(); | |||
| return Numbers.toLong(stringValue, coerce); | |||
| } | |||
|
|
|||
| @FunctionalInterface | |||
| public interface RangeQueryBuilder { | |||
There was a problem hiding this comment.
functions are cool, but couldn't we have a shared method that does the boiler-plate range conversion, which is called when creating the lucene query?
There was a problem hiding this comment.
potentially, that could even be pushed upstream? I would love to not have changes to this mapper as part of the feature branch, if possible.
There was a problem hiding this comment.
I think Elasticsearch is mostly responsible for the Object to long stuff. I don't think that is something Lucene wants but I could certainly be wrong.
I would love to not have changes to this mapper as part of the feature branch, if possible.
I could copy and paste the code but I think that'd be dangerous as it'd get out of sync if someone made a chance to the mapper.
functions are cool, but couldn't we have a shared method that does the boiler-plate range conversion, which is called when creating the lucene query?
I think that is what this is. I could probably write it in a way that doesn't use the functional interface if you'd prefer, but it'll be a little more jumbled. It might still be more clear just because we avoid the indirection.
There was a problem hiding this comment.
maybe I was not too careful looking, but it looked like you were calling the function last in the method, which made me think that you could rather have one method to convert the ranges, though what is the return type of that method going to be? Maybe I start to see how a function simplifies it. I don't have a strong opinion. Maybe make it a BiFunction ?
There was a problem hiding this comment.
I've pushed a BiFunction. Do you think that is better?
|
Thanks @javanna ! |
Adds the
termsandrangequeries forlongtypedscriptfields.It also fixes a bug in a few of
toStringtests forkeywordtypedscriptfields.Relates to #59332