[BugFix] Reject vectorSearch on index with user _score field

vectorSearch() exposes a synthetic v._score column backed by the
document metadata _score. If a user index also defines a stored field
named _score, the response tuple builder collides the two values into
the same key and fails with an opaque duplicate-key error from
ImmutableMap.Builder.

Probe result on OpenSearch 3.6:
  - _id: mapping creation is rejected by OpenSearch at index time, so
    no collision is possible. A lock-in IT guards that assumption.
  - _score: mapping creation is accepted. Without a guard,
    vectorSearch() against such an index fails opaquely at response
    time.

Fix: reject the mapping in VectorSearchIndex.createScanBuilder() with
a clear, user-facing SQL error before any request is sent. The guard
runs during planning, so _explain also surfaces the problem without
needing the k-NN plugin. The check lives in VectorSearchIndex to keep
the AstBuilder and TableFunction resolver paths untouched.

Tests:
  - VectorSearchIndexTest.createScanBuilderRejectsIndexWithScoreField
    covers the guard via a mocked mapping.
  - VectorSearchIT.testUserMappingWithIdFieldIsRejectedByOpenSearch
    locks in OpenSearch's _id-rejection behavior.
  - VectorSearchIT.testVectorSearchAgainstIndexWithScoreFieldRejects
    exercises the guard end-to-end via _explain (no k-NN plugin needed).
Signed-off-by: Eric Wei <mengwei.eric@gmail.com>

Author: mengweieric

integ-test/src/test/java/org/opensearch/sql/sql/VectorSearchIT.java

@@ -9,6 +9,7 @@
 
 import java.io.IOException;
 import org.junit.Test;
+import org.opensearch.client.Request;
 import org.opensearch.client.ResponseException;
 import org.opensearch.sql.legacy.SQLIntegTestCase;
 import org.opensearch.sql.legacy.TestsConstants;
@@ -434,4 +435,70 @@ public void testMissingRequiredArgRejected() throws IOException {
 
     assertThat(ex.getMessage(), containsString("requires 4 arguments"));
   }
+
+  // Synthetic column collision (metadata vs. user field).
+  //
+  // v._score and v._id on a vectorSearch() relation are synthetic columns that expose the k-NN
+  // similarity score and the document metadata _id. A user-defined stored field of the same name
+  // would collide with these synthetic columns in the response tuple. Probe results on
+  // OpenSearch 3.6:
+  //   - _id: OpenSearch's mapper rejects a property named _id at mapping time, so no collision
+  //     is possible. The testUserMappingWithIdFieldIsRejectedByOpenSearch test locks this in.
+  //   - _score: OpenSearch accepts a property named _score. Left unchecked the SQL response
+  //     layer would fail with an opaque duplicate-key error when the hit's _source contains the
+  //     user field and the metadata _score both write to the same tuple key. VectorSearchIndex
+  //     therefore rejects the mapping at scan-build time with a clear SQL error; the
+  //     testVectorSearchAgainstIndexWithScoreFieldRejects test exercises that guard (works via
+  //     _explain so no k-NN plugin is needed).
+
+  @Test
+  public void testUserMappingWithIdFieldIsRejectedByOpenSearch() throws IOException {
+    String indexName = "vs_collision_id";
+    deleteIndexIfExists(indexName);
+
+    // OpenSearch rejects _id as a user mapping property; this guarantees v._id on a
+    // vectorSearch() relation always resolves to the document metadata _id. The exact error
+    // message belongs to OpenSearch; this test only locks in that the mapping creation fails.
+    Request createIndex = new Request("PUT", "/" + indexName);
+    createIndex.setJsonEntity("{\"mappings\":{\"properties\":{\"_id\":{\"type\":\"keyword\"}}}}");
+
+    expectThrows(ResponseException.class, () -> client().performRequest(createIndex));
+  }
+
+  @Test
+  public void testVectorSearchAgainstIndexWithScoreFieldRejects() throws IOException {
+    String indexName = "vs_collision_score";
+    deleteIndexIfExists(indexName);
+
+    // Unlike _id, OpenSearch accepts _score as a user-defined mapping property. Without the
+    // VectorSearchIndex guard, a vectorSearch() query against such an index would fail at
+    // response time with an opaque duplicate-key error because the stored _score field and the
+    // metadata _score both map to the same tuple key. The guard raises a clear SQL error
+    // up-front. Using _explain here exercises planning (which runs the guard) without needing
+    // the k-NN plugin.
+    Request createIndex = new Request("PUT", "/" + indexName);
+    createIndex.setJsonEntity("{\"mappings\":{\"properties\":{\"_score\":{\"type\":\"float\"}}}}");
+    client().performRequest(createIndex);
+
+    ResponseException ex =
+        expectThrows(
+            ResponseException.class,
+            () ->
+                explainQuery(
+                    "SELECT v._score FROM vectorSearch(table='"
+                        + indexName
+                        + "', field='embedding', vector='[1.0, 2.0]', option='k=5') AS v "
+                        + "LIMIT 5"));
+
+    assertThat(ex.getMessage(), containsString("_score"));
+    assertThat(ex.getMessage(), containsString("collides"));
+  }
+
+  private void deleteIndexIfExists(String indexName) {
+    try {
+      client().performRequest(new Request("DELETE", "/" + indexName));
+    } catch (IOException ignored) {
+      // Index does not exist, which is fine.
+    }
+  }
 }

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/VectorSearchIndex.java

@@ -75,6 +75,22 @@ public VectorSearchIndex(
 
   @Override
   public TableScanBuilder createScanBuilder() {
+    // Reject mappings that declare a user field named _score. OpenSearch rejects _id (and most
+    // other metadata names) at mapping time, but _score is not blocked there, so a user index
+    // can legitimately contain a stored _score field. If it did, the response path would collide
+    // with the synthetic v._score column (both get written to the same tuple key) and the query
+    // would fail with an opaque duplicate-key error. We check here so the user sees a clear,
+    // actionable SQL error instead, and so _explain surfaces the problem without sending a
+    // request to k-NN. Running in createScanBuilder keeps this out of the AstBuilder /
+    // TableFunction resolver paths which are off-limits for this change.
+    if (getFieldTypes().containsKey("_score")) {
+      throw new IllegalArgumentException(
+          String.format(
+              "Index '%s' defines a user field named '_score' that collides with the synthetic"
+                  + " _score column exposed by vectorSearch(). Rename the stored field or query"
+                  + " the index without vectorSearch().",
+              getIndexName()));
+    }
     final TimeValue cursorKeepAlive =
         getSettings().getSettingValue(Settings.Key.SQL_CURSOR_KEEP_ALIVE);
     var requestBuilder = createRequestBuilder();

opensearch/src/test/java/org/opensearch/sql/opensearch/storage/VectorSearchIndexTest.java

@@ -7,16 +7,24 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.when;
 
+import com.google.common.collect.ImmutableMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
+import org.opensearch.common.unit.TimeValue;
 import org.opensearch.sql.common.setting.Settings;
 import org.opensearch.sql.opensearch.client.OpenSearchClient;
+import org.opensearch.sql.opensearch.data.type.OpenSearchDataType;
+import org.opensearch.sql.opensearch.data.type.OpenSearchDataType.MappingType;
+import org.opensearch.sql.opensearch.mapping.IndexMapping;
 
 @ExtendWith(MockitoExtension.class)
 class VectorSearchIndexTest {
@@ -25,6 +33,8 @@ class VectorSearchIndexTest {
 
   @Mock private Settings settings;
 
+  @Mock private IndexMapping indexMapping;
+
   @Test
   void buildKnnQueryJsonTopK() {
     VectorSearchIndex index =
@@ -226,4 +236,31 @@ void isInstanceOfOpenSearchIndex() {
             client, settings, "test-index", "embedding", new float[] {1.0f}, Map.of("k", "5"));
     assertTrue(index instanceof OpenSearchIndex);
   }
+
+  @Test
+  void createScanBuilderRejectsIndexWithScoreField() {
+    // A mapping that declares a user field named _score cannot coexist with the synthetic
+    // v._score column exposed by vectorSearch(); the guard in createScanBuilder should reject
+    // it with a clear, user-facing error.
+    lenient()
+        .when(settings.getSettingValue(Settings.Key.SQL_CURSOR_KEEP_ALIVE))
+        .thenReturn(TimeValue.timeValueMinutes(1));
+    when(indexMapping.getFieldMappings())
+        .thenReturn(Map.of("_score", OpenSearchDataType.of(MappingType.Float)));
+    when(client.getIndexMappings("test-index"))
+        .thenReturn(ImmutableMap.of("test-index", indexMapping));
+
+    VectorSearchIndex index =
+        new VectorSearchIndex(
+            client, settings, "test-index", "embedding", new float[] {1.0f}, Map.of("k", "5"));
+
+    IllegalArgumentException ex =
+        assertThrows(IllegalArgumentException.class, index::createScanBuilder);
+    assertTrue(
+        ex.getMessage().contains("_score"),
+        "error message should mention the colliding _score field");
+    assertTrue(
+        ex.getMessage().contains("collides"),
+        "error message should describe the collision, got: " + ex.getMessage());
+  }
 }