Merge remote-tracking branch 'upstream/feature/pagination/integ' into feature/pagination/integ

Author: MaxKsyunz

core/src/main/java/org/opensearch/sql/executor/ExecutionEngine.java

@@ -54,7 +54,6 @@ void execute(PhysicalPlan plan, ExecutionContext context,
   class QueryResponse {
     private final Schema schema;
     private final List<ExprValue> results;
-    private final long total;
     private final Cursor cursor;
   }
 

core/src/main/java/org/opensearch/sql/planner/physical/CursorCloseOperator.java

@@ -49,12 +49,6 @@ public ExecutionEngine.Schema schema() {
     return new ExecutionEngine.Schema(List.of());
   }
 
-  // TODO remove
-  @Override
-  public long getTotalHits() {
-    return 0;
-  }
-
   @Override
   public void open() {
     // no-op, no search should be invoked.

core/src/main/java/org/opensearch/sql/planner/physical/FilterOperator.java

@@ -32,7 +32,6 @@ public class FilterOperator extends PhysicalPlan {
   private final Expression conditions;
   @ToString.Exclude
   private ExprValue next = null;
-  private long totalHits = 0;
 
   @Override
   public <R, C> R accept(PhysicalPlanNodeVisitor<R, C> visitor, C context) {
@@ -51,7 +50,6 @@ public boolean hasNext() {
       ExprValue exprValue = conditions.valueOf(inputValue.bindingTuples());
       if (!(exprValue.isNull() || exprValue.isMissing()) && (exprValue.booleanValue())) {
         next = inputValue;
-        totalHits++;
         return true;
       }
     }
@@ -62,10 +60,4 @@ public boolean hasNext() {
   public ExprValue next() {
     return next;
   }
-
-  @Override
-  public long getTotalHits() {
-    // ignore `input.getTotalHits()`, because it returns wrong (unfiltered) value
-    return totalHits;
-  }
 }

core/src/main/java/org/opensearch/sql/planner/physical/NestedOperator.java

@@ -47,8 +47,6 @@ public class NestedOperator extends PhysicalPlan {
   @EqualsAndHashCode.Exclude
   private ListIterator<Map<String, ExprValue>> flattenedResult = result.listIterator();
 
-  private long totalHits = 0;
-
   /**
    * Constructor for NestedOperator with list of map as arg.
    * @param input : PhysicalPlan input.
@@ -121,13 +119,11 @@ public ExprValue next() {
 
       if (result.isEmpty()) {
         flattenedResult = result.listIterator();
-        totalHits++;
         return new ExprTupleValue(new LinkedHashMap<>());
       }
 
       flattenedResult = result.listIterator();
     }
-    totalHits++;
     return new ExprTupleValue(new LinkedHashMap<>(flattenedResult.next()));
   }
 
@@ -283,9 +279,4 @@ private void getNested(
           row, ret, currentObj);
     }
   }
-
-  @Override
-  public long getTotalHits() {
-    return totalHits;
-  }
 }

core/src/main/java/org/opensearch/sql/planner/physical/PhysicalPlan.java

@@ -44,15 +44,4 @@ public ExecutionEngine.Schema schema() {
     throw new IllegalStateException(String.format("[BUG] schema can been only applied to "
         + "ProjectOperator, instead of %s", this.getClass().getSimpleName()));
   }
-
-  /**
-   * Returns Total hits matched the search criteria. Note: query may return less if limited.
-   * {@see Settings#QUERY_SIZE_LIMIT}.
-   * Any plan which adds/removes rows to the response should overwrite it to provide valid values.
-   *
-   * @return Total hits matched the search criteria.
-   */
-  public long getTotalHits() {
-    return getChild().stream().mapToLong(PhysicalPlan::getTotalHits).max().orElse(0);
-  }
 }

core/src/main/java/org/opensearch/sql/planner/physical/ValuesOperator.java

@@ -56,13 +56,6 @@ public boolean hasNext() {
     return valuesIterator.hasNext();
   }
 
-  @Override
-  public long getTotalHits() {
-    // ValuesOperator used for queries without `FROM` clause, e.g. `select 1`.
-    // Such query always returns 1 row.
-    return 1;
-  }
-
   @Override
   public ExprValue next() {
     List<ExprValue> values = valuesIterator.next().stream()

core/src/test/java/org/opensearch/sql/executor/QueryServiceTest.java

@@ -133,7 +133,7 @@ Helper executeSuccess(Split split) {
           invocation -> {
             ResponseListener<ExecutionEngine.QueryResponse> listener = invocation.getArgument(2);
             listener.onResponse(
-                new ExecutionEngine.QueryResponse(schema, Collections.emptyList(), 0,
+                new ExecutionEngine.QueryResponse(schema, Collections.emptyList(),
                     Cursor.None));
             return null;
           })

core/src/test/java/org/opensearch/sql/executor/streaming/MicroBatchStreamingExecutionTest.java

@@ -170,7 +170,7 @@ Helper executeSuccess(Long... offsets) {
                 ResponseListener<ExecutionEngine.QueryResponse> listener =
                     invocation.getArgument(2);
                 listener.onResponse(
-                    new ExecutionEngine.QueryResponse(null, Collections.emptyList(), 0,
+                    new ExecutionEngine.QueryResponse(null, Collections.emptyList(),
                         Cursor.None));
 
                 PlanContext planContext = invocation.getArgument(1);

core/src/test/java/org/opensearch/sql/planner/physical/CursorCloseOperatorTest.java

@@ -27,15 +27,6 @@ public void never_hasNext() {
     assertFalse(plan.hasNext());
   }
 
-  // TODO remove
-  @Test
-  public void no_total_hits() {
-    var plan = new CursorCloseOperator(null);
-    assertEquals(0, plan.getTotalHits());
-    plan.open();
-    assertEquals(0, plan.getTotalHits());
-  }
-
   @Test
   public void open_is_not_propagated() {
     var child = mock(PhysicalPlan.class);

core/src/test/java/org/opensearch/sql/planner/physical/FilterOperatorTest.java

@@ -50,7 +50,6 @@ public void filter_test() {
         .tupleValue(ImmutableMap
             .of("ip", "209.160.24.63", "action", "GET", "response", 404, "referer",
                 "www.amazon.com"))));
-    assertEquals(1, plan.getTotalHits());
   }
 
   @Test
@@ -64,7 +63,6 @@ public void null_value_should_been_ignored() {
         DSL.equal(DSL.ref("response", INTEGER), DSL.literal(404)));
     List<ExprValue> result = execute(plan);
     assertEquals(0, result.size());
-    assertEquals(0, plan.getTotalHits());
   }
 
   @Test
@@ -78,21 +76,5 @@ public void missing_value_should_been_ignored() {
         DSL.equal(DSL.ref("response", INTEGER), DSL.literal(404)));
     List<ExprValue> result = execute(plan);
     assertEquals(0, result.size());
-    assertEquals(0, plan.getTotalHits());
-  }
-
-  @Test
-  public void totalHits() {
-    when(inputPlan.hasNext()).thenReturn(true, true, true, true, true, false);
-    var answers = Stream.of(200, 240, 300, 403, 404).map(c ->
-            new ExprTupleValue(new LinkedHashMap<>(Map.of("response", new ExprIntegerValue(c)))))
-        .collect(Collectors.toList());
-    when(inputPlan.next()).thenAnswer(AdditionalAnswers.returnsElementsOf(answers));
-
-    FilterOperator plan = new FilterOperator(inputPlan,
-        DSL.less(DSL.ref("response", INTEGER), DSL.literal(400)));
-    List<ExprValue> result = execute(plan);
-    assertEquals(3, result.size());
-    assertEquals(3, plan.getTotalHits());
   }
 }