Reject OFFSET, WHERE on _score, and script subtrees under filter_type=efficient

mengweieric · mengweieric · commit 154df52ec426 · 2026-04-22T13:53:32.000-07:00
Three bug fixes in VectorSearchQueryBuilder that close silent-data-loss paths
on vectorSearch() relations:

1. OFFSET on vectorSearch(): pushDownLimit now rejects any LogicalLimit with
   a non-zero offset. The parent path would push `from: &lt;offset&gt;` into the
   OpenSearch request, silently shifting the top-k window and dropping the
   highest-scoring results. LogicalSort carries only a count (no offset),
   so no change is needed on the sort path.

2. WHERE on _score: pushDownFilter walks the condition with an
   ExpressionNodeVisitor before compiling to DSL and rejects any
   ReferenceExpression whose attr matches "_score" case-insensitively.
   Previously the synthetic _score column compiled to a range query on a
   non-existent stored field, which returned zero rows at the cluster with
   no error. Users who want a score floor should use option='min_score=...'.

3. Script subtrees under filter_type=efficient: after building the WHERE
   QueryBuilder, the EFFICIENT branch recursively scans for any
   ScriptQueryBuilder (traversing BoolQueryBuilder's must/filter/should/
   mustNot lists and ConstantScoreQueryBuilder wrappers). If found, refuse
   to embed it under knn.filter. AOSS and serverless vector collections
   reject script queries inside knn.filter; without this guard the user
   saw an opaque cluster-side failure instead of a clear plan-time error.

Unit tests cover each rejection (including the compound predicate path for
_score, the uppercase _SCORE bypass attempt, and the POST-mode negative
case for script predicates). Integration tests assert 400 status and the
user-facing message from the SQL endpoint, including the combined sort +
limit + offset shape that exercises the pushDownSort() path.

Signed-off-by: Eric Wei &lt;mengwei.eric@gmail.com&gt;
diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/VectorSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/VectorSearchIT.java
@@ -263,6 +263,86 @@ public void testBareAggregateRejects() throws IOException {
         containsString("Aggregations are not supported on vectorSearch() relations"));
   }
 
+  // ── OFFSET / WHERE _score / filter_type=efficient script rejection ───
+
+  @Test
+  public void testOffsetRejected() throws IOException {
+    ResponseException ex =
+        expectThrows(
+            ResponseException.class,
+            () ->
+                executeQuery(
+                    "SELECT v._id FROM vectorSearch(table='"
+                        + TEST_INDEX
+                        + "', field='embedding', "
+                        + "vector='[1.0, 2.0]', option='k=5') AS v "
+                        + "LIMIT 5 OFFSET 2"));
+
+    assertThat(ex.getMessage(), containsString("OFFSET is not supported on vectorSearch()"));
+    assertThat(ex.getMessage(), containsString("LIMIT only"));
+  }
+
+  @Test
+  public void testScoreInWhereRejected() throws IOException {
+    ResponseException ex =
+        expectThrows(
+            ResponseException.class,
+            () ->
+                executeQuery(
+                    "SELECT v._id FROM vectorSearch(table='"
+                        + TEST_INDEX
+                        + "', field='embedding', "
+                        + "vector='[1.0, 2.0]', option='k=5') AS v "
+                        + "WHERE v._score > 0.5 "
+                        + "LIMIT 5"));
+
+    assertThat(ex.getMessage(), containsString("WHERE on _score is not supported"));
+    assertThat(ex.getMessage(), containsString("min_score"));
+  }
+
+  @Test
+  public void testOrderByScoreDescLimitOffsetRejected() throws IOException {
+    // The natural user shape pairs sort with pagination: ORDER BY _score DESC LIMIT N OFFSET M.
+    // The planner's pushDownSort() path can collapse the sort+limit into a top-k size, so OFFSET
+    // must still be rejected by pushDownLimit when the combined form is used. Without this guard
+    // the parent builder would push `from: <offset>` and silently shift the top-k window.
+    ResponseException ex =
+        expectThrows(
+            ResponseException.class,
+            () ->
+                executeQuery(
+                    "SELECT v._id FROM vectorSearch(table='"
+                        + TEST_INDEX
+                        + "', field='embedding', "
+                        + "vector='[1.0, 2.0]', option='k=5') AS v "
+                        + "ORDER BY v._score DESC "
+                        + "LIMIT 5 OFFSET 2"));
+
+    assertThat(ex.getMessage(), containsString("OFFSET is not supported on vectorSearch()"));
+  }
+
+  @Test
+  public void testEfficientModeRejectsScriptPredicate() throws IOException {
+    // WHERE age + 1 > 30 compiles to a ScriptQueryBuilder under the hood because the outer >
+    // is applied to an arithmetic expression, not a direct field reference. Efficient mode
+    // cannot embed script queries under knn.filter, so this must be rejected up front with a
+    // clear remediation hint instead of a cluster-side failure.
+    ResponseException ex =
+        expectThrows(
+            ResponseException.class,
+            () ->
+                executeQuery(
+                    "SELECT v._id FROM vectorSearch(table='"
+                        + TEST_INDEX
+                        + "', field='embedding', "
+                        + "vector='[1.0, 2.0]', option='k=5,filter_type=efficient') AS v "
+                        + "WHERE v.age + 1 > 30 "
+                        + "LIMIT 5"));
+
+    assertThat(ex.getMessage(), containsString("filter_type=efficient does not support"));
+    assertThat(ex.getMessage(), containsString("script queries"));
+  }
+
   // ── k-NN plugin capability check ──────────────────────────────────────
   // The default integ-test cluster does not have the k-NN plugin installed. Execution-path
   // queries against vectorSearch() should therefore fail with the clear "k-NN plugin missing"
diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/VectorSearchQueryBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/VectorSearchQueryBuilder.java
@@ -6,15 +6,19 @@
 package org.opensearch.sql.opensearch.storage.scan;
 
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
 import org.apache.commons.lang3.tuple.Pair;
 import org.opensearch.index.query.BoolQueryBuilder;
+import org.opensearch.index.query.ConstantScoreQueryBuilder;
 import org.opensearch.index.query.QueryBuilder;
 import org.opensearch.index.query.QueryBuilders;
+import org.opensearch.index.query.ScriptQueryBuilder;
 import org.opensearch.sql.ast.tree.Sort;
 import org.opensearch.sql.ast.tree.Sort.SortOption;
 import org.opensearch.sql.exception.ExpressionEvaluationException;
 import org.opensearch.sql.expression.Expression;
+import org.opensearch.sql.expression.ExpressionNodeVisitor;
 import org.opensearch.sql.expression.ReferenceExpression;
 import org.opensearch.sql.opensearch.request.OpenSearchRequestBuilder;
 import org.opensearch.sql.opensearch.storage.FilterType;
@@ -78,6 +82,18 @@ public VectorSearchQueryBuilder(
   public boolean pushDownFilter(LogicalFilter filter) {
     FilterQueryBuilder queryBuilder = new FilterQueryBuilder(new DefaultExpressionSerializer());
     Expression queryCondition = filter.getCondition();
+
+    // Reject WHERE predicates that reference the synthetic _score column. The planner surfaces
+    // _score as a projectable/filterable field, but it is not a stored document field in
+    // OpenSearch. If we let this pass through, FilterQueryBuilder produces a range query on a
+    // non-existent field and the cluster silently returns 0 rows. Users who want a score floor
+    // should use option='min_score=...' instead.
+    if (containsScoreReference(queryCondition)) {
+      throw new ExpressionEvaluationException(
+          "WHERE on _score is not supported on vectorSearch()."
+              + " Use option='min_score=...' for score-floor filtering.");
+    }
+
     QueryBuilder whereQuery;
     try {
       whereQuery = queryBuilder.build(queryCondition);
@@ -93,6 +109,16 @@ public boolean pushDownFilter(LogicalFilter filter) {
     filterPushed = true;
 
     if (filterType == FilterType.EFFICIENT) {
+      // knn.filter on AOSS/serverless vector collections rejects script queries. If any WHERE
+      // subtree compiled to a ScriptQueryBuilder (arithmetic, function calls, CASE, date math),
+      // refuse to embed it under knn.filter instead of shipping a request that will fail at the
+      // cluster with an opaque error.
+      if (containsScriptQuery(whereQuery)) {
+        throw new ExpressionEvaluationException(
+            "filter_type=efficient does not support predicates that compile to script queries"
+                + " (arithmetic, function calls, CASE, date math). Rewrite the WHERE clause to"
+                + " use comparable/term/range predicates, or omit filter_type.");
+      }
       QueryBuilder rebuiltKnn = rebuildKnnWithFilter.apply(whereQuery);
       requestBuilder.getSourceBuilder().query(rebuiltKnn);
     } else {
@@ -105,6 +131,14 @@ public boolean pushDownFilter(LogicalFilter filter) {
 
   @Override
   public boolean pushDownLimit(LogicalLimit limit) {
+    // OFFSET on vectorSearch() silently rewrites the search window and drops top results, which
+    // defeats the entire point of a relevance-ranked top-k query. The parent path would push
+    // `from: <offset>` into the OpenSearch request; reject it explicitly so users get a clear
+    // error instead of surprising result shifts.
+    if (limit.getOffset() != null && limit.getOffset() != 0) {
+      throw new ExpressionEvaluationException(
+          "OFFSET is not supported on vectorSearch(). Remove OFFSET and use LIMIT only.");
+    }
     validateLimitWithinK(limit.getLimit());
     limitPushed = true;
     return super.pushDownLimit(limit);
@@ -151,6 +185,74 @@ private void validateLimitWithinK(int limit) {
     }
   }
 
+  /**
+   * Returns true if any subexpression is a ReferenceExpression whose attr is "_score". Uses the
+   * standard ExpressionNodeVisitor so compound predicates (AND/OR/NOT, function calls, CASE) are
+   * walked uniformly.
+   */
+  private static boolean containsScoreReference(Expression expr) {
+    AtomicBoolean found = new AtomicBoolean(false);
+    expr.accept(
+        new ExpressionNodeVisitor<Void, Void>() {
+          @Override
+          public Void visitReference(ReferenceExpression node, Void context) {
+            // Case-insensitive match so _SCORE, _Score, and any quoted/backticked variant that
+            // preserves original casing cannot bypass the guard and reach the cluster as a range
+            // query on a non-existent field.
+            if (node.getAttr() != null && "_score".equalsIgnoreCase(node.getAttr())) {
+              found.set(true);
+            }
+            return null;
+          }
+        },
+        null);
+    return found.get();
+  }
+
+  /**
+   * Recursively scans a QueryBuilder tree for any ScriptQueryBuilder. Handles the common wrappers
+   * that FilterQueryBuilder produces: BoolQueryBuilder (must/should/mustNot/filter) and
+   * ConstantScoreQueryBuilder. Other QueryBuilder subtypes are leaves for our purposes: if the
+   * top-level builder itself is a ScriptQueryBuilder we catch it, otherwise we treat it as
+   * script-free.
+   */
+  private static boolean containsScriptQuery(QueryBuilder qb) {
+    if (qb == null) {
+      return false;
+    }
+    if (qb instanceof ScriptQueryBuilder) {
+      return true;
+    }
+    if (qb instanceof BoolQueryBuilder) {
+      BoolQueryBuilder bool = (BoolQueryBuilder) qb;
+      for (QueryBuilder child : bool.must()) {
+        if (containsScriptQuery(child)) {
+          return true;
+        }
+      }
+      for (QueryBuilder child : bool.filter()) {
+        if (containsScriptQuery(child)) {
+          return true;
+        }
+      }
+      for (QueryBuilder child : bool.should()) {
+        if (containsScriptQuery(child)) {
+          return true;
+        }
+      }
+      for (QueryBuilder child : bool.mustNot()) {
+        if (containsScriptQuery(child)) {
+          return true;
+        }
+      }
+      return false;
+    }
+    if (qb instanceof ConstantScoreQueryBuilder) {
+      return containsScriptQuery(((ConstantScoreQueryBuilder) qb).innerQuery());
+    }
+    return false;
+  }
+
   @Override
   public OpenSearchRequestBuilder build() {
     if (filterTypeExplicit && !filterPushed) {
diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/VectorSearchQueryBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/VectorSearchQueryBuilderTest.java
@@ -594,6 +594,163 @@ void pushDownFilterNonPushdownableWithoutExplicitFilterTypeFallsBack() {
     assertFalse(pushed, "Non-pushdownable filter should return false for in-memory fallback");
   }
 
+  // ── OFFSET rejection ────────────────────────────────────────────────
+
+  @Test
+  void pushDownLimit_rejectsNonZeroOffset() {
+    var requestBuilder = createRequestBuilder();
+    var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
+    var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "5"));
+
+    var dummyChild = new LogicalValues(Collections.emptyList());
+    // LIMIT 3 OFFSET 2: the planner passes both through LogicalLimit
+    var limit = new LogicalLimit(dummyChild, 3, 2);
+
+    ExpressionEvaluationException ex =
+        assertThrows(ExpressionEvaluationException.class, () -> builder.pushDownLimit(limit));
+    assertTrue(
+        ex.getMessage().contains("OFFSET is not supported on vectorSearch()"),
+        "Expected OFFSET rejection message, got: " + ex.getMessage());
+    assertTrue(
+        ex.getMessage().contains("LIMIT only"),
+        "Expected remediation guidance in message, got: " + ex.getMessage());
+  }
+
+  @Test
+  void pushDownLimit_acceptsZeroOffset() {
+    var requestBuilder = createRequestBuilder();
+    var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
+    var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "5"));
+
+    var dummyChild = new LogicalValues(Collections.emptyList());
+    var limit = new LogicalLimit(dummyChild, 3, 0);
+
+    // Zero offset is the normal case; must continue to succeed.
+    assertTrue(builder.pushDownLimit(limit));
+  }
+
+  // ── WHERE on _score rejection ────────────────────────────────────────
+
+  @Test
+  void pushDownFilter_rejectsScoreReferenceInWhere() {
+    var requestBuilder = createRequestBuilder();
+    var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
+    var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "5"));
+
+    // WHERE _score > 0.5 (note: _score is a synthetic column, not a stored field)
+    var condition =
+        DSL.greater(new ReferenceExpression("_score", ExprCoreType.FLOAT), DSL.literal(0.5));
+    var dummyChild = new LogicalValues(Collections.emptyList());
+    var filter = new LogicalFilter(dummyChild, condition);
+
+    ExpressionEvaluationException ex =
+        assertThrows(ExpressionEvaluationException.class, () -> builder.pushDownFilter(filter));
+    assertTrue(
+        ex.getMessage().contains("WHERE on _score is not supported"),
+        "Expected _score rejection message, got: " + ex.getMessage());
+    assertTrue(
+        ex.getMessage().contains("min_score"),
+        "Expected remediation guidance pointing at option='min_score=...', got: "
+            + ex.getMessage());
+  }
+
+  @Test
+  void pushDownFilter_rejectsScoreReferenceInsideCompound() {
+    var requestBuilder = createRequestBuilder();
+    var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
+    var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "5"));
+
+    // WHERE state = 'TX' AND _score > 0.5: rejection must walk compound predicates
+    var condition =
+        DSL.and(
+            DSL.equal(new ReferenceExpression("state", STRING), DSL.literal("TX")),
+            DSL.greater(new ReferenceExpression("_score", ExprCoreType.FLOAT), DSL.literal(0.5)));
+    var dummyChild = new LogicalValues(Collections.emptyList());
+    var filter = new LogicalFilter(dummyChild, condition);
+
+    ExpressionEvaluationException ex =
+        assertThrows(ExpressionEvaluationException.class, () -> builder.pushDownFilter(filter));
+    assertTrue(
+        ex.getMessage().contains("WHERE on _score is not supported"),
+        "Expected _score rejection message, got: " + ex.getMessage());
+  }
+
+  @Test
+  void pushDownFilter_rejectsUppercaseScoreReference() {
+    var requestBuilder = createRequestBuilder();
+    var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
+    var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "5"));
+
+    // WHERE _SCORE > 0.5 must be rejected the same way as _score; the check is case-insensitive
+    // so variants that preserve original casing cannot bypass the guard.
+    var condition =
+        DSL.greater(new ReferenceExpression("_SCORE", ExprCoreType.FLOAT), DSL.literal(0.5));
+    var dummyChild = new LogicalValues(Collections.emptyList());
+    var filter = new LogicalFilter(dummyChild, condition);
+
+    ExpressionEvaluationException ex =
+        assertThrows(ExpressionEvaluationException.class, () -> builder.pushDownFilter(filter));
+    assertTrue(
+        ex.getMessage().contains("WHERE on _score is not supported"),
+        "Expected _score rejection message, got: " + ex.getMessage());
+  }
+
+  // ── filter_type=efficient rejects script subtrees ───────────────────
+
+  @Test
+  void pushDownFilter_efficient_rejectsScriptSubtree() {
+    var requestBuilder = createRequestBuilder();
+    var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
+    Function<QueryBuilder, QueryBuilder> rebuildWithFilter =
+        whereQuery -> new WrapperQueryBuilder("{\"knn\":{\"filter\":\"embedded\"}}");
+    var builder =
+        new VectorSearchQueryBuilder(
+            requestBuilder,
+            knnQuery,
+            Map.of("k", "5"),
+            FilterType.EFFICIENT,
+            true,
+            rebuildWithFilter);
+
+    // WHERE price + 1 > 100: the outer > is not a direct field ref, so FilterQueryBuilder
+    // falls through to buildScriptQuery() and produces a ScriptQueryBuilder. Under filter_type=
+    // efficient that would be embedded under knn.filter, which AOSS rejects.
+    var condition =
+        DSL.greater(
+            DSL.add(new ReferenceExpression("price", ExprCoreType.INTEGER), DSL.literal(1)),
+            DSL.literal(100));
+    var dummyChild = new LogicalValues(Collections.emptyList());
+    var filter = new LogicalFilter(dummyChild, condition);
+
+    ExpressionEvaluationException ex =
+        assertThrows(ExpressionEvaluationException.class, () -> builder.pushDownFilter(filter));
+    assertTrue(
+        ex.getMessage().contains("filter_type=efficient does not support"),
+        "Expected script rejection message, got: " + ex.getMessage());
+    assertTrue(
+        ex.getMessage().contains("script queries"),
+        "Expected script queries guidance in message, got: " + ex.getMessage());
+  }
+
+  @Test
+  void pushDownFilter_post_allowsScriptSubtree() {
+    // Under POST mode, script queries are fine: the WHERE lives in an outer bool.filter
+    // that OpenSearch evaluates against hits, not inside knn.filter. Verify we do not
+    // false-positive on the POST branch.
+    var requestBuilder = createRequestBuilder();
+    var knnQuery = new WrapperQueryBuilder("{\"knn\":{}}");
+    var builder = new VectorSearchQueryBuilder(requestBuilder, knnQuery, Map.of("k", "5"));
+
+    var condition =
+        DSL.greater(
+            DSL.add(new ReferenceExpression("price", ExprCoreType.INTEGER), DSL.literal(1)),
+            DSL.literal(100));
+    var dummyChild = new LogicalValues(Collections.emptyList());
+    var filter = new LogicalFilter(dummyChild, condition);
+
+    assertTrue(builder.pushDownFilter(filter), "POST mode must still accept script predicates");
+  }
+
   @Test
   void buildSucceedsRadialWithSortEmbeddedLimit() {
     var requestBuilder = createRequestBuilder();
