Add boolean values script fields#60830
Add boolean values script fields#60830nik9000 merged 7 commits intoelastic:feature/runtime_fieldsfrom
Conversation
This adds `boolean` values `runtime_script` fields.
|
Pinging @elastic/es-search (:Search/Search) |
| } | ||
|
|
||
| @Override | ||
| public Query rangeQuery( |
There was a problem hiding this comment.
I'm trying to mimic what we support in core. It works, so far as I can tell. It is kind of amazing we allow this at all, but we do!
javanna
left a comment
There was a problem hiding this comment.
thanks for all the tests, left a couple of comments but LGTM. i was wondering if we should have some duel tests to compare behaviour between concrete boolean fields and runtime boolean fields, given that we discovered some nuances around range queries etc.
| @Override | ||
| public long nextValue() throws IOException { | ||
| // Emit all false values before all true values | ||
| return cursor++ < script.falses() ? 0 : 1; |
There was a problem hiding this comment.
I must say that multiple values for booleans look really weird, especially trying to summarize multiple values into one, and re-sorting them. Is this behaviour documented anywhere?
There was a problem hiding this comment.
I'm not sure it is, though it is what happens when you use doc values for out of the box booleans.
|
|
||
| @Override | ||
| public NumericType getNumericType() { | ||
| return NumericType.DOUBLE; |
There was a problem hiding this comment.
I never realized this either: booleans are double????
There was a problem hiding this comment.
Ooops, copy and paste error. Fixing.
|
|
||
| @Override | ||
| public Object valueForDisplay(Object value) { | ||
| throw new UnsupportedOperationException(); |
There was a problem hiding this comment.
why do we throw? Shouldn't we borrow the logic from https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java#L157 ?
There was a problem hiding this comment.
I can do that, but I don't think it is ever called!
There was a problem hiding this comment.
oh well I did not realize it was unused, in that case I am good with throwing, but add a comment?
| } | ||
|
|
||
| @Override | ||
| public Query rangeQuery( |
I've added some dual tests. I think it'd probably be good to add them for every field to be honest. I can work on that. |
f7c099e to
21a810f
Compare
|
run elasticsearch-ci/2 |
This adds
booleanvaluesruntime_scriptfields.