infinite continuations when NULL_ON_EMPTY is in the plan

normen662 · Feb 4, 2025 · fdf069f · fdf069f
1 parent c8fc2a6
commit fdf069f
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 14 deletions.
diff --git a/...r-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/AggregateCursor.java b/...r-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/AggregateCursor.java
@@ -25,7 +25,6 @@
 import com.apple.foundationdb.record.RecordCursor;
 import com.apple.foundationdb.record.RecordCursorContinuation;
 import com.apple.foundationdb.record.RecordCursorResult;
-import com.apple.foundationdb.record.RecordCursorStartContinuation;
 import com.apple.foundationdb.record.RecordCursorVisitor;
 import com.apple.foundationdb.record.query.plan.plans.QueryResult;
 import com.google.common.base.Verify;
@@ -73,7 +72,7 @@ public CompletableFuture<RecordCursorResult<QueryResult>> onNext() {
         return AsyncUtil.whileTrue(() -> inner.onNext().thenApply(innerResult -> {
             previousResult = innerResult;
             if (!innerResult.hasNext()) {
-                if (!isNoRecords() || streamGrouping.isResultOnEmpty()) {
+                if (!isNoRecords()) {
                     streamGrouping.finalizeGroup();
                 }
                 return false;
@@ -86,11 +85,7 @@ public CompletableFuture<RecordCursorResult<QueryResult>> onNext() {
         }), getExecutor()).thenApply(vignore -> {
             if (isNoRecords()) {
                 // Edge case where there are no records at all
-                if (streamGrouping.isResultOnEmpty()) {
-                    return RecordCursorResult.withNextValue(QueryResult.ofComputed(streamGrouping.getCompletedGroupResult()), RecordCursorStartContinuation.START);
-                } else {
-                    return RecordCursorResult.exhausted();
-                }
+                return RecordCursorResult.exhausted();
             }
             // Use the last valid result for the continuation as we need non-terminal one here.
             RecordCursorContinuation continuation = Verify.verifyNotNull(previousValidResult).getContinuation();

diff --git a/...er-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/StreamGrouping.java b/...er-core/src/main/java/com/apple/foundationdb/record/cursors/aggregate/StreamGrouping.java
@@ -193,8 +193,4 @@ private Object evalGroupingKey(@Nullable final Object currentObject) {
         final EvaluationContext nestedContext = context.withBinding(Bindings.Internal.CORRELATION, alias, currentObject);
         return Objects.requireNonNull(groupingKeyValue).eval(store, nestedContext);
     }
-
-    public boolean isResultOnEmpty() {
-        return groupingKeyValue == null;
-    }
 }
diff --git a/...in/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryDefaultOnEmptyPlan.java b/...in/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryDefaultOnEmptyPlan.java
@@ -93,9 +93,10 @@ public <M extends Message> RecordCursor<QueryResult> executePlan(@Nonnull final
                                                                      @Nonnull final EvaluationContext context,
                                                                      @Nullable final byte[] continuation,
                                                                      @Nonnull final ExecuteProperties executeProperties) {
-        return RecordCursor.orElse(cont -> getChild().executePlan(store, context, cont, executeProperties),
-                (executor, cont) -> RecordCursor.fromList(ImmutableList.of(QueryResult.ofComputed(onEmptyResultValue.eval(store, context)))),
-                continuation);
+        return RecordCursor.orElse(cont -> getChild().executePlan(store, context, cont, executeProperties.clearSkipAndLimit()),
+                        (executor, cont) ->
+                                RecordCursor.fromList(ImmutableList.of(QueryResult.ofComputed(onEmptyResultValue.eval(store, context))), cont), continuation)
+                .skipThenLimit(executeProperties.getSkip(), executeProperties.getReturnedRowLimit());
     }
 
     @Override

diff --git a/yaml-tests/src/test/resources/aggregate-empty-table.yamsql b/yaml-tests/src/test/resources/aggregate-empty-table.yamsql
@@ -36,7 +36,9 @@ test_block:
     -
       - query: select count(*) from T1;
       - explain: "SCAN(<,>) | TFILTER T1 | MAP (_ AS _0) | AGG (count_star(*) AS _0) | ON EMPTY NULL | MAP (coalesce_long(_._0._0, promote(0l AS LONG)) AS _0)"
+      - maxRows: 1
       - result: [{0}]
+      - result: []
     -
       - query: select count(*) from T1 where col1 = 0;
       - explain: "SCAN(<,>) | TFILTER T1 | FILTER _.COL1 EQUALS promote(@c11 AS LONG) | MAP (_ AS _0) | AGG (count_star(*) AS _0) | ON EMPTY NULL | MAP (coalesce_long(_._0._0, promote(0l AS LONG)) AS _0)"