Switch type of expandNested from boolean to Boolean #2333
Conversation
heemin32
requested review from
navneet1v,
VijayanB,
vamshin,
jmazanec15,
naveentatikonda,
junqiu-lei,
martin-gaievski,
ryanbogan,
luyuncheng and
shatejas
as code owners
Signed-off-by: Heemin Kim <[email protected]>
|
Rolling upgrade is failing because the query data type is changed. However, it will pass once this change is back ported to 2.x branch. |
Yes. All the existing tests have query without expandNested being set. |
| public Optional<Boolean> getExpandNested() { | ||
| return Optional.ofNullable(expandNested); | ||
| } |
There was a problem hiding this comment.
should we add the default value directly here rather than in src/main/java/org/opensearch/knn/index/query/KNNQueryFactory.java ?
There was a problem hiding this comment.
I will let the factory to decide on default value to be consist with other values. For example, rescoreContext's default value is null but we are still wrapping it as Optional instead of just returning null.
All existing nested queries are actually using null value here. So they are already covered. |
Signed-off-by: Heemin Kim <[email protected]> (cherry picked from commit aa6936a)
Signed-off-by: Heemin Kim <[email protected]> (cherry picked from commit aa6936a) Co-authored-by: Heemin Kim <[email protected]>
Description
Change the type of the
expandNestedvariable frombooleantoBoolean. This modification allows the neural search to pass theexpandNestedDocsparameter as-is (null,true, orfalse), instead of convertingnullorfalsetofalseandtruetotrue.https://github.com/opensearch-project/neural-search/blob/main/src/main/java/org/opensearch/neuralsearch/query/NeuralQueryBuilder.java#L315
Related Issues
N/A
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.