Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 10 additions & 4 deletions .github/workflows/mvn-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,16 @@ jobs:
slow-unit-tests:
runs-on: ubuntu-latest
needs: build-and-package
# This lane is the workflow's critical path at ~44 min, and it had no cap at all, so it inherited
# GitHub's 6-hour default: a hung fork would have burned six hours of a runner before anyone saw it.
# 60 min matches unit-tests and leaves ~16 min of headroom over the current steady state.
timeout-minutes: 60
# This lane is the workflow's critical path, and it had no cap at all, so it inherited GitHub's 6-hour
# default: a hung fork would have burned six hours of a runner before anyone saw it.
#
# 90 min, up from the 60 that matched unit-tests. The honest duration of this lane has been measured at
# 39m27s, 46m and 57m40s on `main`, and one run was CANCELLED at 1h0m16s while progressing normally -
# LSMTreeIndexTest had just completed and no test had failed. A cap within a few minutes of that spread
# goes red on runner speed alone, and it goes red as "the job failed", which reads as a regression from
# the PR under test until someone pulls the logs (issue #6323). This cap is a hang detector, so it is
# sized above the slowest honest run rather than around the median.
timeout-minutes: 90
permissions:
contents: read
checks: write
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@
*/
package com.arcadedb.query.opencypher.ast;

import com.arcadedb.database.RID;
import com.arcadedb.function.graph.IdFunction;
import com.arcadedb.query.opencypher.query.OpenCypherQueryEngine;
import com.arcadedb.query.sql.executor.CommandContext;
import com.arcadedb.query.sql.executor.MultiValue;
import com.arcadedb.query.sql.executor.Result;
import com.arcadedb.utility.LongRangeList;

import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -32,6 +35,9 @@
* Example: n.name IN ['Alice', 'Bob', 'Charlie']
*/
public class InExpression implements BooleanExpression {
/** {@code 2^53}: the first magnitude at which a double stops representing every long exactly. */
private static final double EXACT_DOUBLE_LIMIT = 9007199254740992d;

private final Expression expression;
private final List<Expression> list;
private final boolean isNot;
Expand Down Expand Up @@ -100,6 +106,19 @@ public Object evaluateTernary(final Result result, final CommandContext context)
valuesToCheck = evaluated;
}

// Answers the walk below would pay O(n) for. Its cost is the POSITION of the match, and a miss walks all of
// it, so on the lazy range() of advisory GHSA-xmjm-8q85-g778 an element near the end costs seconds (#6323).
if (value == null)
// Every comparison against null is null, whatever the elements are, so only their number matters: a
// non-empty list makes this uncertain, an empty one leaves nothing to be uncertain about.
return valuesToCheck.iterator().hasNext() ? null : Boolean.valueOf(isNot);

if (valuesToCheck instanceof LongRangeList range) {
final Boolean found = rangeMembership(range, value);
if (found != null)
return isNot != found;
}

// 3VL: null IN [1,2,3] -> null, 5 IN [1,null,3] -> null (if not found otherwise)
boolean foundNull = false;
for (final Object checkValue : valuesToCheck) {
Expand All @@ -116,6 +135,59 @@ else if (cmp)
return isNot ? true : false;
}

/**
* Membership in a lazy range, answered from its start, step and size, or null when it cannot be answered that
* way and the walk has to run. Never returns the 3VL {@code null} answer: a range holds longs and no nulls, and
* a null left operand is answered before this is called, so uncertainty is impossible here.
* <p>
* What the walk asks per element is {@link #valuesCompare}, i.e. the {@code =} operator against a {@code Long}.
* The branches of {@code ComparisonExpression.compareValuesTernary} that a {@code Long} right operand can reach
* are exactly the three below - the RID-string interop, the numeric comparison, and "different types are not
* equal" for everything else (a Long is not {@code Identifiable}, not temporal, and
* {@code MultiValue.getMultiValueAsList} does not turn it into a list, so those branches cannot fire). A new
* coercion added there and not learned here would make this diverge, which is what
* {@code CypherInRangeMembershipTest.answersExactlyAsTheWalkDoes} exists to catch.
*/
private static Boolean rangeMembership(final LongRangeList range, final Object value) {
if (range.isEmpty())
return Boolean.FALSE;

// Integral types, answered as longs. The = operator's own long-vs-long branch covers only a Long/Integer
// pair, so a Short or a Byte is compared there through doubleValue() instead - which reaches the same answer:
// a value of at most 15 bits converts to a double exactly, and so does any element that could equal it, since
// an element large enough to lose precision is far larger than any Short or Byte.
if (value instanceof Long || value instanceof Integer || value instanceof Short || value instanceof Byte)
return range.containsLong(((Number) value).longValue());

if (value instanceof Number number) {
// Any other numeric pair is compared by = through doubleValue(), so this has to be too.
final double asDouble = number.doubleValue();
if (Double.isNaN(asDouble) || Double.isInfinite(asDouble))
// NaN equals nothing, not even itself (issue #5293), and no long is infinite.
return Boolean.FALSE;
if (Math.abs(asDouble) >= EXACT_DOUBLE_LIMIT)
// From 2^53 up a double no longer distinguishes adjacent longs, so which elements it equals stops being a
// question of one value: leave it to the walk, which is the definition of the answer. The bound is
// inclusive because 2^53 ITSELF is already ambiguous - 2^53+1 is not representable and rounds ties-to-even
// down onto 2^53, so both longs convert to this same double, and picking one of them would answer FALSE
// for a range that holds only the other.
return null;
if (asDouble != Math.rint(asDouble))
return Boolean.FALSE;
// Below 2^53 every element is converted to a double exactly, so equality of the doubles is equality of the
// longs, and no element at or above 2^53 can equal a double below it.
return range.containsLong((long) asDouble);
}

// The = operator reads a RID-shaped string against a number as the id it denotes (Neo4j-compatible id()
// interop): the same coercion, not a second one.
if (value instanceof String string)
return RID.is(string) ? range.containsLong(IdFunction.encodeRidAsLong(new RID(string))) : Boolean.FALSE;

// Anything else is of a different type than a Long, and for = that is simply not equal.
return Boolean.FALSE;
}

/**
* Three-valued comparison of one list element against the left operand.
* Returns Boolean.TRUE if definitely equal, Boolean.FALSE if definitely not equal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.arcadedb.query.sql.executor.CommandContext;
import com.arcadedb.query.sql.executor.MultiValue;
import com.arcadedb.query.sql.executor.Result;
import com.arcadedb.utility.LongRangeList;

import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -60,40 +61,55 @@ public Object evaluate(final Result result, final CommandContext context) {
if (listValue == null)
return null;

final Integer from = sliceBound(fromExpression, result, context);
if (fromExpression != null && from == null)
return null;
final Integer to = sliceBound(toExpression, result, context);
if (toExpression != null && to == null)
return null;

return slice(listValue, from, to);
}

private static Integer sliceBound(final Expression bound, final Result result, final CommandContext context) {
return bound == null ? null : sliceBound(bound.evaluate(result, context));
}

/**
* A slice bound from its already evaluated value: the index it denotes, or null when the value is null - which
* makes the whole slice null, as any null operand does. Shared with {@code ExpressionEvaluator}, which evaluates
* the operand through itself: without it that path cast straight to {@code Number} and answered a non-numeric
* bound with a raw ClassCastException instead of saying what was wrong with the query (issue #6323).
*/
public static Integer sliceBound(final Object value) {
if (value == null)
return null;
if (value instanceof Number number)
return number.intValue();
throw new IllegalArgumentException("Slice index must be a number, got: " + value.getClass().getSimpleName());
}

/**
* Applies a Cypher slice to an already evaluated list, array or string, with the bounds already evaluated and
* {@code null} meaning "from the beginning" / "to the end". Shared with
* {@code ExpressionEvaluator.evaluateListSlice}, which resolves the operands through itself so aggregation
* overrides apply, but must then slice identically: the two used to carry a copy of this each, and only one of
* them was fixed at a time (issue #6323).
*/
public static Object slice(final Object listValue, final Integer fromIndex, final Integer toIndex) {
// Treat Collections and Java arrays (incl. primitive arrays from numeric-array parameters,
// issue #4284) uniformly as Cypher lists without copying upfront.
final boolean isListLike = listValue instanceof Collection || listValue.getClass().isArray();
final int size;
if (isListLike)
size = MultiValue.getSize(listValue);
else if (listValue instanceof String)
size = ((String) listValue).length();
else if (listValue instanceof String string)
size = string.length();
else
throw new IllegalArgumentException("Cannot slice type: " + listValue.getClass().getSimpleName());

// Resolve from index (default: 0)
int from = 0;
if (fromExpression != null) {
final Object fromValue = fromExpression.evaluate(result, context);
if (fromValue == null)
return null;
if (fromValue instanceof Number)
from = ((Number) fromValue).intValue();
else
throw new IllegalArgumentException("Slice index must be a number, got: " + fromValue.getClass().getSimpleName());
}

// Resolve to index (default: size)
int to = size;
if (toExpression != null) {
final Object toValue = toExpression.evaluate(result, context);
if (toValue == null)
return null;
if (toValue instanceof Number)
to = ((Number) toValue).intValue();
else
throw new IllegalArgumentException("Slice index must be a number, got: " + toValue.getClass().getSimpleName());
}
int from = fromIndex != null ? fromIndex : 0;
int to = toIndex != null ? toIndex : size;

// Handle negative indices
if (from < 0)
Expand All @@ -106,11 +122,14 @@ else if (listValue instanceof String)
to = Math.min(to, size);

// If from >= to, return empty
if (from >= to) {
if (listValue instanceof String)
return "";
return new ArrayList<>();
}
if (from >= to)
return listValue instanceof String ? "" : new ArrayList<>();

if (listValue instanceof LongRangeList range)
// A slice of an arithmetic progression is one, and subList() returns it in constant space. Copying it
// instead reinstated the heap exhaustion the lazy range removed (advisory GHSA-xmjm-8q85-g778): a slice can
// be as large as the range, so range(0, 999999999)[0..1000000000] allocated a billion boxed longs (#6323).
return range.subList(from, to);

if (isListLike) {
// Boxing into a List is unavoidable here - the Cypher slice result is itself a List - but only
Expand Down
Loading
Loading