Skip to content

Commit

Permalink
set skip and limit for the recursive union cursor correctly.
Browse files Browse the repository at this point in the history
  • Loading branch information
hatyo committed Feb 5, 2025
1 parent 847ffe6 commit 86ef7ae
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,18 @@ public <M extends Message> RecordCursor<QueryResult> executePlan(@Nonnull final
@Nullable final byte[] continuation,
@Nonnull final ExecuteProperties executeProperties) {
final var type = getInnerTypeDescriptor(context);
final var childExecuteProperties = executeProperties.clearSkipAndLimit();
final var recursiveStateManager = new RecursiveStateManagerImpl(
(initialContinuation, evaluationContext) -> getInitialStatePlan().executePlan(store, evaluationContext,
initialContinuation == null ? null : initialContinuation.toByteArray(), executeProperties),
initialContinuation == null ? null : initialContinuation.toByteArray(), childExecuteProperties),
(recursiveContinuation, evaluationContext) -> getRecursiveStatePlan().executePlan(store, evaluationContext,
recursiveContinuation == null ? null : recursiveContinuation.toByteArray(), executeProperties),
recursiveContinuation == null ? null : recursiveContinuation.toByteArray(), childExecuteProperties),
context,
tempTableScanAlias,
tempTableInsertAlias,
proto -> TempTable.from(proto, type), store.getContext().getTempTableFactory(), continuation);
return new RecursiveUnionCursor<>(recursiveStateManager, store.getExecutor());
return new RecursiveUnionCursor<>(recursiveStateManager, store.getExecutor())
.skipThenLimit(executeProperties.getSkip(), executeProperties.getReturnedRowLimit());
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.apple.foundationdb.relational.util.SpotBugsSuppressWarnings;

import com.google.common.collect.HashMultiset;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Multiset;
import com.google.protobuf.Descriptors;
import com.google.protobuf.Message;
Expand Down Expand Up @@ -428,7 +429,8 @@ private static ImmutablePair<ResultSetMatchResult, ResultSetPrettyPrinter> match
var i = 1;
for (final var expectedRow : expectedAsList) {
if (!actual.next()) {
return ImmutablePair.of(ResultSetMatchResult.fail("result does not contain all expected rows."), null);
printCurrentAndRemaining(actual, resultSetPrettyPrinter);
return ImmutablePair.of(ResultSetMatchResult.fail(String.format("result does not contain all expected rows! expected %d rows, got %d", expectedAsList.size(), i - 1)), null);
}
if (!isMap(expectedRow)) { // I think it should be possible to expect a result set like: [[1,2,3], [4,5,6]]. But ok for now.
printCurrentAndRemaining(actual, resultSetPrettyPrinter);
Expand All @@ -449,20 +451,24 @@ private static ImmutablePair<ResultSetMatchResult, ResultSetPrettyPrinter> match

private static ImmutablePair<ResultSetMatchResult, ResultSetPrettyPrinter> matchUnorderedResultSet(final RelationalResultSet actual, final @Nonnull Multiset<?> expectedAsMultiSet) throws SQLException {
final ResultSetPrettyPrinter resultSetPrettyPrinter = new ResultSetPrettyPrinter();
var i = 1;
// O(n^2) -- we all got M1s
final var expectedRowCount = expectedAsMultiSet.size();
var actualRowsCounter = 1;
while (actual.next()) {
boolean found = false;
if (expectedAsMultiSet.isEmpty()) {
printCurrentAndRemaining(actual, resultSetPrettyPrinter);
return ImmutablePair.of(ResultSetMatchResult.fail(String.format("too many rows in actual result set! expected %d rows, got %d.", i - 1, i - 1 + resultSetPrettyPrinter.getRowCount())), resultSetPrettyPrinter);
// count the remaining actual rows.
while (actual.next()) {
actualRowsCounter++;
}
return ImmutablePair.of(ResultSetMatchResult.fail(String.format("too many rows in actual result set! expected %d row(s), got %d row(s) instead.", expectedRowCount, actualRowsCounter - 1)), resultSetPrettyPrinter);
}
for (final var expectedRow : expectedAsMultiSet) {
for (final var expectedRow : expectedAsMultiSet.elementSet()) {
if (!isMap(expectedRow)) { // I think it should be possible to expect a result set like: [[1,2,3], [4,5,6]]. But ok for now.
printCurrentAndRemaining(actual, resultSetPrettyPrinter);
return ImmutablePair.of(ResultSetMatchResult.fail("unknown format of expected result set"), resultSetPrettyPrinter);
}
final var matchResult = matchRow(map(expectedRow), actual.getMetaData().getColumnCount(), valueByName(actual), valueByIndex(actual), i);
final var matchResult = matchRow(map(expectedRow), actual.getMetaData().getColumnCount(), valueByName(actual), valueByIndex(actual), actualRowsCounter);
if (matchResult.equals(ResultSetMatchResult.success())) {
found = true;
expectedAsMultiSet.remove(expectedRow);
Expand All @@ -471,12 +477,12 @@ private static ImmutablePair<ResultSetMatchResult, ResultSetPrettyPrinter> match
}
if (!found) {
printCurrentAndRemaining(actual, resultSetPrettyPrinter);
return ImmutablePair.of(ResultSetMatchResult.fail(String.format("result row at %d does not match any expected records", i)), resultSetPrettyPrinter);
return ImmutablePair.of(ResultSetMatchResult.fail(String.format("actual row at %d does not match any expected records", actualRowsCounter)), resultSetPrettyPrinter);
}
i++;
actualRowsCounter++;
}
if (!expectedAsMultiSet.isEmpty()) {
return ImmutablePair.of(ResultSetMatchResult.fail("result does not contain all expected rows"), null);
return ImmutablePair.of(ResultSetMatchResult.fail(String.format("result does not contain all expected rows, expected %d row(s), got %d row(s) instead.", expectedRowCount, actualRowsCounter - 1)), null);
}
return ImmutablePair.of(ResultSetMatchResult.success(), null);
}
Expand Down
8 changes: 8 additions & 0 deletions yaml-tests/src/test/resources/cte.yamsql
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,12 @@ test_block:
# TODO (CTEs and subqueries are not fully optimized)
- query: with c1(x, y) as (select col1, col2 from t1) select x from c1 where y < 3
- explain: "SCAN(<,>) | MAP (_.COL1 AS COL1, _.COL2 AS COL2) | FILTER _.COL2 LESS_THAN promote(@c24 AS LONG) | MAP (_.COL1 AS X)"
-
- query: with c1 as (select col1, col2 from t1) select col1, col2 from c1
- maxRows: 1
- result: [{COL1: 10, COL2: 1}]
- result: [{COL1: 10, COL2: 2}]
- result: [{COL1: 20, COL2: 6}]
- result: [{COL1: 20, COL2: 7}]
- result: []
...
43 changes: 43 additions & 0 deletions yaml-tests/src/test/resources/recursive-cte.yamsql
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,49 @@ test_block:
{210, 20},
{250, 50},
{250, 50}]
-
- query: with recursive c1 as (
select id, parent from t1 where parent = -1
union all
select b.id, b.parent from c1 as a, t1 as b where a.id = b.parent) select id from c1
- maxRows: 1
- result: [{ID: 1}]
- result: [{ID: 10}]
- result: [{ID: 20}]
- result: [{ID: 40}]
- result: [{ID: 50}]
- result: [{ID: 70}]
- result: [{ID: 100}]
- result: [{ID: 210}]
- result: [{ID: 250}]
- result: []
-
- query: with recursive allDescendants as (
with recursive ancestorsOf250 as (
select id, parent from t1 where id = 250
union all
select b.id, b.parent from ancestorsOf250 as a, t1 as b where a.parent = b.id) select id, parent from ancestorsOf250
union all
select b.id, b.parent from allDescendants as a, t1 as b where a.id = b.parent) select id, parent from allDescendants
- maxRows: 1
- result: [{250, 50}]
- result: [{50, 10}]
- result: [{10, 1}]
- result: [{1, -1}]
- result: [{10, 1}]
- result: [{20, 1}]
- result: [{40, 10}]
- result: [{50, 10}]
- result: [{70, 10}]
- result: [{250, 50}]
- result: [{40, 10}]
- result: [{50, 10}]
- result: [{70, 10}]
- result: [{100, 20}]
- result: [{210, 20}]
- result: [{250, 50}]
- result: [{250, 50}]
- result: []
# -
# does not currently work due to bug in NLJ planning, see https://github.com/FoundationDB/fdb-record-layer/issues/2997
# - query: with recursive c1 as (
Expand Down

0 comments on commit 86ef7ae

Please sign in to comment.