Setting rescore to false as search parameter is noop

[jmazanec15]

Description

For re-scoring, when the query parameter is set to false but the knn_vector field is configured to use on disk, re-scoring will still take place:

# Re-scoring will still happen for below query
GET my-knn-index-1/_search
{
  "query": {
    "knn": {
      "my_vector": {
        "vector": [ ],
        "k": 3,
        "rescore": false
      }
    }
  }
}

This is incorrect. When set to false, no re-scoring should happen.

Workaround

As a workaround, users can set the index setting index.knn.disk.vector.shard_level_rescoring_disabled to true. However, we should fix this so that the behavior properly works.

Mitigation

The solution was added in the development branch for the disk based feature, but it appears to have gotten lost in shuffle of merging disk based from dev branch to 2.17. The main change will be in KNNQueryBuilderParser and will look something like this:

-        internalParser.declareObjectOrDefault(
-            KNNQueryBuilder.Builder::rescoreContext,
-            (p, v) -> RescoreParser.fromXContent(p),
-            RescoreContext::getDefault,
-            RESCORE_FIELD
-        );
+        internalParser.declareField((p, v, c) -> {
+            BiConsumer<KNNQueryBuilder.Builder, RescoreContext> consumer = KNNQueryBuilder.Builder::rescoreContext;
+            BiFunction<XContentParser, Void, RescoreContext> objectParser = (_p, _v) -> RescoreParser.fromXContent(_p);
+            Supplier<RescoreContext> defaultValue = RescoreContext::getDefault;
+            if (p.currentToken() == XContentParser.Token.VALUE_BOOLEAN) {
+                if (p.booleanValue()) {
+                    consumer.accept(v, defaultValue.get());
+                } else {
+                    // If the user specifies false, I want to explicitly set to empty disabled so we dont
+                    // accidentally resolve.
+                    consumer.accept(v, EXPLICITLY_DISABLED_RESCORE_CONTEXT);
+                }
+            } else {
+                consumer.accept(v, objectParser.apply(p, c));
+            }
+        }, RESCORE_FIELD, ObjectParser.ValueType.OBJECT_OR_BOOLEAN);

This was similar to what we had done in development. See https://github.com/jmazanec15/k-NN-1/blob/6df6b0618db5124d5067eddcb0903aa50eb8194c/src/main/java/org/opensearch/knn/index/query/parser/KNNQueryBuilderParser.java#L89-L104 (And this thrown away PR: jmazanec15#1).

[naveentatikonda]

Workaround

As a workaround, users can set the index setting index.knn.disk.vector.shard_level_rescoring_disabled to false. However, we should fix this so that the behavior properly works.

@jmazanec15 shouldn't we set index.knn.disk.vector.shard_level_rescoring_disabled to true instead of false to disable rescoring ?

[jmazanec15]

@naveentatikonda updated