Address Julie's feedback 3

Author: mayya-sharipova

docs/reference/vectors/vector-functions.asciidoc

@@ -213,8 +213,8 @@ you can access dense vectors's values directly through the following functions:
 - `doc[<field>].vectorValue` – returns a vector's value as an array of floats
 
 - `doc[<field>].magnitude` – returns a vector's magnitude as a float
-(for vectors created prior version 7.5 magnitude is not stored.
-So this function calculates it anew every time is called).
+(for vectors created prior to version 7.5 the magnitude is not stored.
+So this function calculates it anew every time it is called).
 
 For example, the script below implements a cosine similarity using these
 two functions:

server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java

@@ -14,7 +14,6 @@
 import org.elasticsearch.script.ExplainableScoreScript;
 import org.elasticsearch.script.ScoreScript;
 import org.elasticsearch.script.Script;
-import org.elasticsearch.Version;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -42,15 +41,13 @@ public float score() {
 
     private final int shardId;
     private final String indexName;
-    private final Version indexVersion;
 
-    public ScriptScoreFunction(Script sScript, ScoreScript.LeafFactory script, String indexName, int shardId, Version indexVersion) {
+    public ScriptScoreFunction(Script sScript, ScoreScript.LeafFactory script, String indexName, int shardId) {
         super(CombineFunction.REPLACE);
         this.sScript = sScript;
         this.script = script;
         this.indexName = indexName;
         this.shardId = shardId;
-        this.indexVersion = indexVersion;
     }
 
     @Override
@@ -60,7 +57,6 @@ public LeafScoreFunction getLeafScoreFunction(LeafReaderContext ctx) throws IOEx
         leafScript.setScorer(scorer);
         leafScript._setIndexName(indexName);
         leafScript._setShard(shardId);
-        leafScript._setIndexVersion(indexVersion);
         return new LeafScoreFunction() {
             @Override
             public double score(int docId, float subQueryScore) throws IOException {

server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreQuery.java

@@ -146,7 +146,6 @@ private ScoreScript makeScoreScript(LeafReaderContext context) throws IOExceptio
                 final ScoreScript scoreScript = scriptBuilder.newInstance(context);
                 scoreScript._setIndexName(indexName);
                 scoreScript._setShard(shardId);
-                scoreScript._setIndexVersion(indexVersion);
                 return scoreScript;
             }
 

server/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java

@@ -83,8 +83,7 @@ protected ScoreFunction doToFunction(SearchExecutionContext context) {
         try {
             ScoreScript.Factory factory = context.compile(script, ScoreScript.CONTEXT);
             ScoreScript.LeafFactory searchScript = factory.newFactory(script.getParams(), context.lookup());
-            return new ScriptScoreFunction(script, searchScript,
-                context.index().getName(), context.getShardId(), context.indexVersionCreated());
+            return new ScriptScoreFunction(script, searchScript, context.index().getName(), context.getShardId());
         } catch (Exception e) {
             throw new QueryShardException(context, "script_score: the script could not be loaded", e);
         }

server/src/main/java/org/elasticsearch/script/ScoreScript.java

@@ -10,7 +10,6 @@
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Explanation;
 import org.apache.lucene.search.Scorable;
-import org.elasticsearch.Version;
 import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.index.fielddata.ScriptDocValues;
@@ -85,7 +84,6 @@ public Explanation get(double score, Explanation subQueryExplanation) {
     private int docId;
     private int shardId = -1;
     private String indexName = null;
-    private Version indexVersion = null;
 
     public ScoreScript(Map<String, Object> params, SearchLookup lookup, LeafReaderContext leafContext) {
         // null check needed b/c of expression engine subclass
@@ -185,19 +183,6 @@ public String _getIndex() {
         }
     }
 
-    /**
-     *  Starting a name with underscore, so that the user cannot access this function directly through a script
-     *  It is only used within predefined painless functions.
-     * @return index version or throws an exception if the index version is not set up for this script instance
-     */
-    public Version _getIndexVersion() {
-        if (indexVersion != null) {
-            return indexVersion;
-        } else {
-            throw new IllegalArgumentException("index version can not be looked up!");
-        }
-    }
-
     /**
      *  Starting a name with underscore, so that the user cannot access this function directly through a script
      */
@@ -212,13 +197,6 @@ public void _setIndexName(String indexName) {
         this.indexName = indexName;
     }
 
-    /**
-     *  Starting a name with underscore, so that the user cannot access this function directly through a script
-     */
-    public void _setIndexVersion(Version indexVersion) {
-        this.indexVersion = indexVersion;
-    }
-
 
     /** A factory to construct {@link ScoreScript} instances. */
     public interface LeafFactory {

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/vectors/30_dense_vector_script_access.yml

@@ -1,7 +1,6 @@
 ---
 "Access to values of dense_vector in script":
   - skip:
-      features: headers
       version: " - 7.12.99"
       reason: "Access to values of dense_vector in script was added in 7.13"
   - do:
@@ -28,84 +27,8 @@
           - '{"index": {"_id": "missing_vector"}}'
           - '{}'
 
-  # check getVectorValue() API
-  - do:
-      search:
-        body:
-          query:
-            script_score:
-              query: { "exists" : { "field" : "v" } }
-              script:
-                source: |
-                  float s = 0;
-                  for (def el : doc['v'].vectorValue) {
-                    s += el;
-                  }
-                  s;
-
-  - match: { hits.hits.0._id: "3" }
-  - match: { hits.hits.0._score: 5 }
-  - match: { hits.hits.1._id: "2" }
-  - match: { hits.hits.1._score: 4 }
-  - match: { hits.hits.2._id: "1" }
-  - match: { hits.hits.2._score: 3 }
-
-
-  # check getMagnitude() API
-  - do:
-      headers:
-        Content-Type: application/json
-      search:
-        body:
-          query:
-            script_score:
-              query: { "exists" : { "field" : "v" } }
-              script:
-                source: "doc['v'].magnitude"
-
-  - match: { hits.hits.0._id: "3" }
-  - gte: {hits.hits.0._score: 3.3166}
-  - lte: {hits.hits.0._score: 3.3167}
-  - match: { hits.hits.1._id: "2" }
-  - gte: {hits.hits.1._score: 2.4494}
-  - lte: {hits.hits.1._score: 2.4495}
-  - match: { hits.hits.2._id: "1" }
-  - gte: {hits.hits.2._score: 1.7320}
-  - lte: {hits.hits.2._score: 1.7321}
-
-  # check failed request on missing values
-  - do:
-      catch: bad_request
-      search:
-        body:
-          query:
-            script_score:
-              query: { match_all: { } }
-              script:
-                source: "doc['v'].vectorValue[0]"
-
-  - match: { status: 400 }
-  - match: { error.root_cause.0.type: "script_exception" }
-
-  # check failed request on missing values
-  - do:
-      catch: bad_request
-      search:
-        body:
-          query:
-            script_score:
-              query: { match_all: { } }
-              script:
-                source: "doc['v'].magnitude"
-
-  - match: { status: 400 }
-  - match: { error.root_cause.0.type: "script_exception" }
-
-
   # vector functions in loop – return the index of the closest parameter vector based on cosine similarity
   - do:
-      headers:
-        Content-Type: application/json
       search:
         body:
           query:
@@ -123,7 +46,7 @@
                     for (int j = 0; j < v.length; j++) {
                       dotProduct += v[j] * params.pvs[i][j];
                     }
-                    float cosSim = dotProduct / (vm * (float) params.pvs_lengths[i]);
+                    float cosSim = dotProduct / (vm * (float) params.pvs_magnts[i]);
                     if (maxCosSim < cosSim) {
                       maxCosSim = cosSim;
                       closestPv = i;
@@ -132,7 +55,7 @@
                   closestPv;
                 params:
                   pvs: [ [ 1, 1, 1 ], [ 1, 1, 2 ], [ 1, 1, 3 ] ]
-                  pvs_lengths: [1.7320, 2.4495, 3.3166]
+                  pvs_magnts: [1.7320, 2.4495, 3.3166]
 
   - match: { hits.hits.0._id: "3" }
   - match: { hits.hits.0._score: 2 }

x-pack/plugin/vectors/src/test/java/org/elasticsearch/xpack/vectors/query/DenseVectorFunctionTests.java

@@ -52,7 +52,6 @@ public void testVectorFunctions() {
             when(docValues.dims()).thenReturn(docVector.length);
 
             ScoreScript scoreScript = mock(ScoreScript.class);
-            when(scoreScript._getIndexVersion()).thenReturn(indexVersion);
             when(scoreScript.getDoc()).thenReturn(Collections.singletonMap(field, docValues));
 
             testDotProduct(scoreScript);
@@ -101,7 +100,7 @@ private void testL2Norm(ScoreScript scoreScript) {
         assertThat(e.getMessage(), containsString("query vector has a different number of dimensions [2] than the document vectors [5]"));
     }
 
-    private static BytesRef mockEncodeDenseVector(float[] values, Version indexVersion) {
+    static BytesRef mockEncodeDenseVector(float[] values, Version indexVersion) {
         byte[] bytes = indexVersion.onOrAfter(Version.V_7_5_0)
             ? new byte[VectorEncoderDecoder.INT_BYTES * values.length + VectorEncoderDecoder.INT_BYTES]
             : new byte[VectorEncoderDecoder.INT_BYTES * values.length];

x-pack/plugin/vectors/src/test/java/org/elasticsearch/xpack/vectors/query/DenseVectorScriptDocValuesTests.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.vectors.query;
+
+import org.apache.lucene.index.BinaryDocValues;
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.Version;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+import static org.hamcrest.Matchers.containsString;
+
+public class DenseVectorScriptDocValuesTests extends ESTestCase {
+
+    private static BinaryDocValues wrap(float[][] vectors, Version indexVersion) {
+        return new BinaryDocValues() {
+            int idx = -1;
+            int maxIdx = vectors.length;
+            @Override
+            public BytesRef binaryValue() {
+                if (idx >= maxIdx) {
+                    throw new IllegalStateException("max index exceeded");
+                }
+                return DenseVectorFunctionTests.mockEncodeDenseVector(vectors[idx], indexVersion);
+            }
+
+            @Override
+            public boolean advanceExact(int target) {
+                idx = target;
+                if (target < maxIdx) {
+                    return true;
+                }
+                return false;
+            }
+
+            @Override
+            public int docID() {
+                return idx;
+            }
+
+            @Override
+            public int nextDoc() {
+                return idx++;
+            }
+
+            @Override
+            public int advance(int target) {
+                throw new IllegalArgumentException("not defined!");
+            }
+
+            @Override
+            public long cost() {
+                throw new IllegalArgumentException("not defined!");
+            }
+        };
+    }
+
+    public void testGetVectorValueAndGetMagnitude() throws IOException {
+        final int dims = 3;
+        float[][] vectors = {{ 1, 1, 1 }, { 1, 1, 2 }, { 1, 1, 3 } };
+        float[] expectedMagnitudes = { 1.7320f, 2.4495f, 3.3166f };
+
+        for (Version indexVersion : Arrays.asList(Version.V_7_4_0, Version.CURRENT)) {
+            BinaryDocValues docValues = wrap(vectors, indexVersion);
+            final DenseVectorScriptDocValues scriptDocValues = new DenseVectorScriptDocValues(docValues, indexVersion, dims);
+            for (int i = 0; i < vectors.length; i++) {
+                scriptDocValues.setNextDocId(i);
+                assertArrayEquals(vectors[i], scriptDocValues.getVectorValue(), 0.0001f);
+                assertEquals(expectedMagnitudes[i], scriptDocValues.getMagnitude(), 0.0001f);
+            }
+        }
+    }
+
+    public void testMissingValues() throws IOException {
+        final int dims = 3;
+        float[][] vectors = {{ 1, 1, 1 }, { 1, 1, 2 }, { 1, 1, 3 } };
+        BinaryDocValues docValues = wrap(vectors, Version.CURRENT);
+        final DenseVectorScriptDocValues scriptDocValues = new DenseVectorScriptDocValues(docValues, Version.CURRENT, dims);
+
+        scriptDocValues.setNextDocId(3);
+        Exception e = expectThrows(IllegalArgumentException.class, () -> scriptDocValues.getVectorValue());
+        assertEquals("A document doesn't have a value for a vector field!", e.getMessage());
+
+        e = expectThrows(IllegalArgumentException.class, () -> scriptDocValues.getMagnitude());
+        assertEquals("A document doesn't have a value for a vector field!", e.getMessage());
+    }
+
+    public void testGetFunctionIsNotAccessible() throws IOException {
+        final int dims = 3;
+        float[][] vectors = {{ 1, 1, 1 }, { 1, 1, 2 }, { 1, 1, 3 } };
+        BinaryDocValues docValues = wrap(vectors, Version.CURRENT);
+        final DenseVectorScriptDocValues scriptDocValues = new DenseVectorScriptDocValues(docValues, Version.CURRENT, dims);
+
+        scriptDocValues.setNextDocId(0);
+        Exception e = expectThrows(UnsupportedOperationException.class, () -> scriptDocValues.get(0));
+        assertThat(e.getMessage(), containsString("accessing a vector field's value through 'get' or 'value' is not supported!"));
+    }
+}