Scripted keyword field type: update family type and test field caps output #59672
Conversation
javanna
added
the
:Search/Search
|
Pinging @elastic/es-search (:Search/Search) |
I already have a single node integration test, I am not sure that a yaml test adds value at this point. Field caps is already tested separately and we only want to make sure that its output is the same for a runtime field compared to an ordinary field. I can imagine that we may want to revisit this once we change the response format of field caps for runtime fields. |
|
run elasticsearch-ci/packaging-sample-windows |
| @Override | ||
| public ScriptBinaryFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) { | ||
| assert fullyQualifiedIndexName.length() > 0 : "index name must not be empty"; |
There was a problem hiding this comment.
I think that I will remove this assertion. I initially assumed that calling this would make us load doc_values for a runtime field which is expensive, but it will only make us instantiate the classes and doc_values will not be advanced, hence no script is going to be executed.
…s_test_field_caps
…s_test_field_caps
…s_test_field_caps
| StringScriptFieldScript.Factory factory = scriptCompiler.compile(script.getValue(), StringScriptFieldScript.CONTEXT); | ||
| mappedFieldType = new RuntimeKeywordMappedFieldType(buildFullName(context), script.getValue(), factory, meta.getValue()); | ||
| } else { | ||
| BiFunction<Builder, BuilderContext, MappedFieldType> fieldTypeResolver = Builder.FIELD_TYPE_RESOLVER.get( |
There was a problem hiding this comment.
@nik9000 you may have opinions on this. I added it because I did not want to have tests to maintain a list of supported runtime types and rather rely on the truth which cannot get outdated. Downside is we have no if but rather a map with a bifunction, maybe a bit cryptic.
There was a problem hiding this comment.
I'm pretty used to maps to functions at this point. I'm happy with this. I'm not 100% this is better than the switch statement that I had in #59721, but it does the job just as well. If you like it, I'm happy with it.
There was a problem hiding this comment.
it does not do a better job than the switch, but it allows to expose all the keys for testing :)
Relates to #59332