Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set skip and limit for the recursive union cursor correctly. #3111

Merged
merged 3 commits into from
Feb 7, 2025
+95 −35
Merged

set skip and limit for the recursive union cursor correctly. #3111

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86ef7ae
set skip and limit for the recursive union cursor correctly.
hatyo Feb 5, 2025
9c750e7
address comments.
hatyo Feb 6, 2025
67def2e
set supported version to current.
hatyo Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions ...in/java/com/apple/foundationdb/record/query/plan/plans/RecordQueryRecursiveUnionPlan.java
View file
Open in desktop
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
69 changes: 37 additions & 32 deletions yaml-tests/src/main/java/com/apple/foundationdb/relational/yamltests/Matchers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static List<?> arrayList(@Nonnull final Object obj, @Nonnull final String
if (obj instanceof List) {
return (List<?>) obj;
}
fail(String.format("Expecting '%s' to be of type '%s'", desc, List.class.getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting '%s' to be of type '%s'", desc, List.class.getSimpleName()));
return null;
}

Expand All @@ -79,15 +79,15 @@ public static List<?> arrayList(@Nonnull final Object obj, @Nonnull final String
if (obj instanceof Map<?, ?>) {
return (Map<?, ?>) obj;
}
fail(String.format("Expecting %s to be of type %s", desc, Map.class.getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s", desc, Map.class.getSimpleName()));
return null;
}

public static Map.Entry<?, ?> mapEntry(@Nonnull final Object obj, @Nonnull final String desc) {
if (obj instanceof Map.Entry<?, ?>) {
return (Map.Entry<?, ?>) obj;
}
fail(String.format("Expecting %s to be of type %s", desc, Map.Entry.class.getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s", desc, Map.Entry.class.getSimpleName()));
return null;
}

Expand All @@ -97,21 +97,21 @@ public static Object first(@Nonnull final List<?> obj) {

public static Object first(@Nonnull final List<?> obj, @Nonnull final String desc) {
if (obj.isEmpty()) {
fail(String.format("Expecting %s to contain at least one element, however it is empty", desc));
fail(String.format(Locale.ROOT, "Expecting %s to contain at least one element, however it is empty", desc));
}
return obj.get(0);
}

public static Object second(@Nonnull final List<?> obj) {
if (obj.size() <= 1) {
fail(String.format("Expecting %s to contain at least two elements, however it contains %s element", obj, obj.size()));
fail(String.format(Locale.ROOT, "Expecting %s to contain at least two elements, however it contains %s element", obj, obj.size()));
}
return obj.get(1);
}

public static Object third(@Nonnull final List<?> obj) {
if (obj.size() <= 2) {
fail(String.format("Expecting %s to contain at least three elements, however it contains %s element", obj, obj.size()));
fail(String.format(Locale.ROOT, "Expecting %s to contain at least three elements, however it contains %s element", obj, obj.size()));
}
return obj.get(2);
}
Expand All @@ -121,7 +121,7 @@ public static String string(@Nonnull final Object obj, @Nonnull final String des
// <NULL> should return null maybe?
return (String) obj;
}
fail(String.format("Expecting %s to be of type %s, however it is of type %s", desc, String.class.getSimpleName(), obj.getClass().getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s", desc, String.class.getSimpleName(), obj.getClass().getSimpleName()));
return null;
}

Expand All @@ -146,7 +146,7 @@ public static boolean bool(@Nonnull final Object obj) {
// <NULL> should return null maybe?
return (Boolean) obj;
}
fail(String.format("Expecting %s to be of type %s, however it is of type %s", obj, Boolean.class.getSimpleName(), obj.getClass().getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s", obj, Boolean.class.getSimpleName(), obj.getClass().getSimpleName()));
return false; // never reached.
}

Expand All @@ -161,7 +161,7 @@ public static long longValue(@Nonnull final Object obj) {
if (obj instanceof Integer) {
return Long.valueOf((Integer) obj);
}
fail(String.format("Expecting %s to be of type %s, however it is of type %s", obj, Long.class.getSimpleName(), obj.getClass().getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s", obj, Long.class.getSimpleName(), obj.getClass().getSimpleName()));
return -1; // never reached.
}

Expand All @@ -173,7 +173,7 @@ public static int intValue(@Nonnull final Object obj) {
if (obj instanceof Integer) {
return (Integer) obj;
}
fail(String.format("Expecting %s to be of type %s, however it is of type %s", obj, Integer.class.getSimpleName(), obj.getClass().getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s", obj, Integer.class.getSimpleName(), obj.getClass().getSimpleName()));
return -1; // never reached.
}

Expand All @@ -185,7 +185,7 @@ public static double doubleValue(@Nonnull final Object obj) {
if (obj instanceof Double) {
return (Double) obj;
}
fail(String.format("Expecting %s to be of type %s, however it is of type %s", obj, Double.class.getSimpleName(), obj.getClass().getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s", obj, Double.class.getSimpleName(), obj.getClass().getSimpleName()));
return -1; // never reached.
}

Expand All @@ -207,15 +207,15 @@ public static Message message(@Nonnull final Object obj) {
if (obj instanceof Message) {
return (Message) obj;
}
fail(String.format("Expecting %s to be of type %s, however it is of type %s.", obj, Message.class.getSimpleName(), obj.getClass().getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s.", obj, Message.class.getSimpleName(), obj.getClass().getSimpleName()));
return null;
}

@SpotBugsSuppressWarnings(value = "NP_NONNULL_RETURN_VIOLATION", justification = "should never happen, fail throws")
@Nonnull
public static <T> T notNull(@Nullable final T object, @Nonnull final String desc) {
if (object == null) {
fail(String.format("unexpected %s to be null", desc));
fail(String.format(Locale.ROOT, "unexpected %s to be null", desc));
}
return object;
}
Expand All @@ -226,14 +226,14 @@ public static <T> T notNull(@Nullable final T object, @Nonnull final String desc
if (obj instanceof Map) {
return ((Map<?, ?>) obj).entrySet().iterator().next();
}
fail(String.format("Expecting %s to be of type %s, however it is of type %s.", desc, Map.class.getSimpleName(), obj.getClass().getSimpleName()));
fail(String.format(Locale.ROOT, "Expecting %s to be of type %s, however it is of type %s.", desc, Map.class.getSimpleName(), obj.getClass().getSimpleName()));
return null;
}

@Nonnull
public static Object valueElseKey(@Nonnull final Map.Entry<?, ?> entry) {
if (isNull(entry.getKey()) && isNull(entry.getValue())) {
fail(String.format("encountered YAML-style 'null' which is not supported, consider using '%s' instead", CustomTag.NullPlaceholder.INSTANCE));
fail(String.format(Locale.ROOT, "encountered YAML-style 'null' which is not supported, consider using '%s' instead", CustomTag.NullPlaceholder.INSTANCE));
}
return entry.getValue() == null ? entry.getKey() : entry.getValue();
}
Expand Down Expand Up @@ -428,7 +428,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(Locale.ROOT, "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 @@ -442,27 +443,31 @@ private static ImmutablePair<ResultSetMatchResult, ResultSetPrettyPrinter> match
}
if (printRemaining(actual, resultSetPrettyPrinter)) {
final var leftActualRowCount = resultSetPrettyPrinter.getRowCount();
return ImmutablePair.of(ResultSetMatchResult.fail(String.format("too many rows in actual result set! expected %d rows, got %d.", expectedAsList.size(), expectedAsList.size() + leftActualRowCount)), resultSetPrettyPrinter);
return ImmutablePair.of(ResultSetMatchResult.fail(String.format(Locale.ROOT, "too many rows in actual result set! expected %d rows, got %d.", expectedAsList.size(), expectedAsList.size() + leftActualRowCount)), resultSetPrettyPrinter);
}
return ImmutablePair.of(ResultSetMatchResult.success(), null);
}

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
normen662 marked this conversation as resolved.
Show resolved Hide resolved
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(Locale.ROOT, "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 +476,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(Locale.ROOT, "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(Locale.ROOT, "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 Expand Up @@ -511,7 +516,7 @@ private static ResultSetMatchResult matchRow(@Nonnull final Map<?, ?> expected,
int rowNumber) throws SQLException {
final var expectedColCount = expected.entrySet().size();
if (actualEntriesCount != expectedColCount) {
return ResultSetMatchResult.fail(String.format("row cardinality mismatch at %d! expected a row comprising %d column(s), received %d column(s) instead.", rowNumber, expectedColCount, actualEntriesCount));
return ResultSetMatchResult.fail(String.format(Locale.ROOT, "row cardinality mismatch at %d! expected a row comprising %d column(s), received %d column(s) instead.", rowNumber, expectedColCount, actualEntriesCount));
}
return matchMap(expected, actualEntriesCount, entryByNameAccessor, entryByNumberAccessor, rowNumber, "");
}
Expand All @@ -526,7 +531,7 @@ private static ResultSetMatchResult matchMap(@Nonnull final Map<?, ?> expected,
int counter = 1;
final var expectedColCount = expected.entrySet().size();
if (actualEntriesCount != expectedColCount) {
return ResultSetMatchResult.fail(String.format("! expected a row comprising %d column(s), received %d column(s) instead.", expectedColCount, actualEntriesCount));
return ResultSetMatchResult.fail(String.format(Locale.ROOT, "! expected a row comprising %d column(s), received %d column(s) instead.", expectedColCount, actualEntriesCount));
}
for (final var entry : expected.entrySet()) {
final var expectedField = valueElseKey(entry);
Expand Down Expand Up @@ -572,7 +577,7 @@ private static ResultSetMatchResult matchField(@Nullable final Object expected,
// (nested) message
if (expected instanceof Map<?, ?>) {
if (!(actual instanceof RelationalStruct)) {
return ResultSetMatchResult.fail(String.format("cell mismatch at row: %d cellRef: %s%n expected 🟢 to match a struct, got 🟡 instead.%n🟢 %s (Struct)%n🟡 %s (%s)", rowNumber, cellRef, expected, actual, actual.getClass().getSimpleName()));
return ResultSetMatchResult.fail(String.format(Locale.ROOT, "cell mismatch at row: %d cellRef: %s%n expected 🟢 to match a struct, got 🟡 instead.%n🟢 %s (Struct)%n🟡 %s (%s)", rowNumber, cellRef, expected, actual, actual.getClass().getSimpleName()));
}
final var struct = (RelationalStruct) (actual);
return matchMap(map(expected), struct.getAttributes().length, valueByName(struct), valueByIndex(struct), rowNumber, cellRef);
Expand All @@ -582,13 +587,13 @@ private static ResultSetMatchResult matchField(@Nullable final Object expected,
if (expected instanceof List<?>) {
final var expectedArray = (List<?>) (expected);
if (!(actual instanceof RelationalArray)) {
return ResultSetMatchResult.fail(String.format("cell mismatch at row: %d cellRef: %s%n expected 🟢 to match an array, got 🟡 instead.%n🟢 %s (Array)%n🟡 %s (%s)", rowNumber, cellRef, expected, actual, actual.getClass().getSimpleName()));
return ResultSetMatchResult.fail(String.format(Locale.ROOT, "cell mismatch at row: %d cellRef: %s%n expected 🟢 to match an array, got 🟡 instead.%n🟢 %s (Array)%n🟡 %s (%s)", rowNumber, cellRef, expected, actual, actual.getClass().getSimpleName()));
}
final var actualArray = (RelationalArray) (actual);
final var actualArrayContent = actualArray.getResultSet();
for (int i = 0; i < expectedArray.size(); i++) {
if (!actualArrayContent.next()) {
return ResultSetMatchResult.fail(String.format("cell mismatch at row: %d cellRef: %s%n expected 🟢 (containing %d array items) does not match 🟡 (containing %d array items).%n🟢 %s%n🟡 %s",
return ResultSetMatchResult.fail(String.format(Locale.ROOT, "cell mismatch at row: %d cellRef: %s%n expected 🟢 (containing %d array items) does not match 🟡 (containing %d array items).%n🟢 %s%n🟡 %s",
rowNumber, cellRef, expectedArray.size(), i, expected, actual));
}
if (isMap(expectedArray.get(i))) {
Expand Down Expand Up @@ -643,7 +648,7 @@ private static ResultSetMatchResult matchField(@Nullable final Object expected,
if (Objects.equals(expected, actual)) {
return ResultSetMatchResult.success();
} else {
return ResultSetMatchResult.fail(String.format("cell mismatch at row: %d cellRef: %s%n expected 🟢 does not match 🟡.%n🟢 %s (%s)%n🟡 %s (%s)", rowNumber, cellRef, expected, expected == null ? "NULL" : expected.getClass().getSimpleName(), actual, actual.getClass().getSimpleName()));
return ResultSetMatchResult.fail(String.format(Locale.ROOT, "cell mismatch at row: %d cellRef: %s%n expected 🟢 does not match 🟡.%n🟢 %s (%s)%n🟡 %s (%s)", rowNumber, cellRef, expected, expected == null ? "NULL" : expected.getClass().getSimpleName(), actual, actual.getClass().getSimpleName()));
}
}

Expand All @@ -665,6 +670,6 @@ private static ResultSetMatchResult matchIntField(@Nonnull final Integer expecte
return ResultSetMatchResult.success();
}
}
return ResultSetMatchResult.fail(String.format("cell mismatch at row: %d cellRef: %s%n expected 🟢 does not match 🟡.%n🟢 %s (Integer) %n🟡 %s (%s)", rowNumber, cellRef, expected, actual, actual.getClass().getSimpleName()));
return ResultSetMatchResult.fail(String.format(Locale.ROOT, "cell mismatch at row: %d cellRef: %s%n expected 🟢 does not match 🟡.%n🟢 %s (Integer) %n🟡 %s (%s)", rowNumber, cellRef, expected, actual, actual.getClass().getSimpleName()));
}
}
Loading
Loading