Skip to content

Commit

Permalink
[ES|QL] tests auto decode unsigned longs (elastic#103164)
Browse files Browse the repository at this point in the history
This extracts the type handling sugar around unsigned longs I added in elastic#102830. Turns out, fixing that was a little bigger than intended, and I wanted to break it out into a distinct PR.

The strategy here is to let the framework convert from the encoded unsigned long into a normal BigInteger, so test case writers can just phrase their expected values as normal BigIntegers rather than having to think about the encoding. Feedback welcome, happy to discuss if folks don't think this is a good direction for the tests.


Co-authored-by: Bogdan Pintea <[email protected]>
  • Loading branch information
not-napoleon and bpintea authored Dec 8, 2023
1 parent 9db4cb4 commit cf7941f
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.elasticsearch.xpack.ql.type.EsField;
import org.elasticsearch.xpack.ql.util.NumericUtils;
import org.elasticsearch.xpack.ql.util.StringUtils;
import org.elasticsearch.xpack.versionfield.Version;
import org.hamcrest.Matcher;
Expand Down Expand Up @@ -92,6 +93,7 @@
import static org.elasticsearch.xpack.ql.util.SpatialCoordinateTypes.GEO;
import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
Expand Down Expand Up @@ -241,7 +243,7 @@ private void testEvaluate(boolean readFloating) {
Object result;
try (ExpressionEvaluator evaluator = evaluator(expression).get(driverContext())) {
try (Block block = evaluator.eval(row(testCase.getDataValues()))) {
result = toJavaObject(block, 0);
result = toJavaObjectUnsignedLongAware(block, 0);
}
}
assertThat(result, not(equalTo(Double.NaN)));
Expand All @@ -255,6 +257,16 @@ private void testEvaluate(boolean readFloating) {
}
}

private Object toJavaObjectUnsignedLongAware(Block block, int position) {
Object result;
result = toJavaObject(block, position);
if (result != null && testCase.expectedType == DataTypes.UNSIGNED_LONG) {
assertThat(result, instanceOf(Long.class));
result = NumericUtils.unsignedLongAsBigInteger((Long) result);
}
return result;
}

/**
* Evaluates a {@link Block} of values, all copied from the input pattern, read directly from the page.
* <p>
Expand Down Expand Up @@ -398,7 +410,7 @@ private void testEvaluateBlock(BlockFactory inputBlockFactory, DriverContext con
assertThat(toJavaObject(block, p), allNullsMatcher());
continue;
}
assertThat(toJavaObject(block, p), testCase.getMatcher());
assertThat(toJavaObjectUnsignedLongAware(block, p), testCase.getMatcher());
}
assertThat(
"evaluates to tracked block",
Expand Down Expand Up @@ -472,7 +484,7 @@ public final void testEvaluateInManyThreads() throws ExecutionException, Interru
try (EvalOperator.ExpressionEvaluator eval = evalSupplier.get(driverContext())) {
for (int c = 0; c < count; c++) {
try (Block block = eval.eval(page)) {
assertThat(toJavaObject(block, 0), testCase.getMatcher());
assertThat(toJavaObjectUnsignedLongAware(block, 0), testCase.getMatcher());
}
}
}
Expand Down Expand Up @@ -513,7 +525,12 @@ public final void testFold() {
expression = new FoldNull().rule(expression);
assertThat(expression.dataType(), equalTo(testCase.expectedType));
assertTrue(expression.foldable());
assertThat(expression.fold(), testCase.getMatcher());
Object result = expression.fold();
// Decode unsigned longs into BigIntegers
if (testCase.expectedType == DataTypes.UNSIGNED_LONG && result != null) {
result = NumericUtils.unsignedLongAsBigInteger((Long) result);
}
assertThat(result, testCase.getMatcher());
if (testCase.getExpectedWarnings() != null) {
assertWarnings(testCase.getExpectedWarnings());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import org.apache.lucene.document.InetAddressPoint;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.network.InetAddresses;
import org.elasticsearch.logging.LogManager;
import org.elasticsearch.logging.Logger;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.esql.expression.function.scalar.convert.AbstractConvertFunction;
import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
Expand Down Expand Up @@ -48,6 +50,8 @@
public record TestCaseSupplier(String name, List<DataType> types, Supplier<TestCase> supplier)
implements
Supplier<TestCaseSupplier.TestCase> {

private static Logger logger = LogManager.getLogger(TestCaseSupplier.class);
/**
* Build a test case without types.
*
Expand Down Expand Up @@ -530,6 +534,8 @@ public static void unary(
supplier.type(),
"value"
);
logger.info("Value is " + value + " of type " + value.getClass());
logger.info("expectedValue is " + expectedValue.apply(value));
TestCase testCase = new TestCase(
List.of(typed),
expectedEvaluatorToString,
Expand Down Expand Up @@ -949,6 +955,9 @@ public TypedData(Object data, String name) {

@Override
public String toString() {
if (type == DataTypes.UNSIGNED_LONG && data instanceof Long longData) {
return type.toString() + "(" + NumericUtils.unsignedLongAsBigInteger(longData).toString() + ")";
}
return type.toString() + "(" + (data == null ? "null" : data.toString()) + ")";
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.elasticsearch.xpack.ql.util.NumericUtils;

import java.math.BigDecimal;
import java.math.BigInteger;
Expand All @@ -26,10 +25,7 @@
import java.util.function.Supplier;

import static org.elasticsearch.xpack.ql.type.DataTypeConverter.safeToUnsignedLong;
import static org.elasticsearch.xpack.ql.util.NumericUtils.ONE_AS_UNSIGNED_LONG;
import static org.elasticsearch.xpack.ql.util.NumericUtils.UNSIGNED_LONG_MAX_AS_DOUBLE;
import static org.elasticsearch.xpack.ql.util.NumericUtils.ZERO_AS_UNSIGNED_LONG;
import static org.elasticsearch.xpack.ql.util.NumericUtils.asLongUnsigned;

public class ToUnsignedLongTests extends AbstractFunctionTestCase {
public ToUnsignedLongTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseSupplier) {
Expand All @@ -47,7 +43,7 @@ public static Iterable<Object[]> parameters() {
suppliers,
read,
DataTypes.UNSIGNED_LONG,
NumericUtils::asLongUnsigned,
n -> n,
BigInteger.ZERO,
UNSIGNED_LONG_MAX,
List.of()
Expand All @@ -57,7 +53,7 @@ public static Iterable<Object[]> parameters() {
suppliers,
evaluatorName.apply("Boolean"),
DataTypes.UNSIGNED_LONG,
b -> b ? ONE_AS_UNSIGNED_LONG : ZERO_AS_UNSIGNED_LONG,
b -> b ? BigInteger.ONE : BigInteger.ZERO,
List.of()
);

Expand All @@ -66,7 +62,7 @@ public static Iterable<Object[]> parameters() {
suppliers,
evaluatorName.apply("Long"),
DataTypes.UNSIGNED_LONG,
instant -> asLongUnsigned(instant.toEpochMilli()),
instant -> BigInteger.valueOf(instant.toEpochMilli()),
List.of()
);
// random strings that don't look like an unsigned_long
Expand All @@ -85,7 +81,7 @@ public static Iterable<Object[]> parameters() {
suppliers,
evaluatorName.apply("Double"),
DataTypes.UNSIGNED_LONG,
d -> asLongUnsigned(BigDecimal.valueOf(d).toBigInteger()), // note: not: new BigDecimal(d).toBigInteger
d -> BigDecimal.valueOf(d).toBigInteger(), // note: not: new BigDecimal(d).toBigInteger
0d,
UNSIGNED_LONG_MAX_AS_DOUBLE,
List.of()
Expand Down Expand Up @@ -122,7 +118,7 @@ public static Iterable<Object[]> parameters() {
suppliers,
evaluatorName.apply("Long"),
DataTypes.UNSIGNED_LONG,
NumericUtils::asLongUnsigned,
BigInteger::valueOf,
0L,
Long.MAX_VALUE,
List.of()
Expand All @@ -146,7 +142,7 @@ public static Iterable<Object[]> parameters() {
suppliers,
evaluatorName.apply("Int"),
DataTypes.UNSIGNED_LONG,
NumericUtils::asLongUnsigned,
BigInteger::valueOf,
0,
Integer.MAX_VALUE,
List.of()
Expand Down Expand Up @@ -180,7 +176,7 @@ public static Iterable<Object[]> parameters() {
)
.toList(),
DataTypes.UNSIGNED_LONG,
bytesRef -> asLongUnsigned(safeToUnsignedLong(((BytesRef) bytesRef).utf8ToString())),
bytesRef -> safeToUnsignedLong(((BytesRef) bytesRef).utf8ToString()),
List.of()
);
// strings of random doubles within unsigned_long's range
Expand All @@ -198,7 +194,7 @@ public static Iterable<Object[]> parameters() {
)
.toList(),
DataTypes.UNSIGNED_LONG,
bytesRef -> asLongUnsigned(safeToUnsignedLong(((BytesRef) bytesRef).utf8ToString())),
bytesRef -> safeToUnsignedLong(((BytesRef) bytesRef).utf8ToString()),
List.of()
);
// strings of random doubles outside unsigned_long's range, negative
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;
Expand All @@ -36,15 +37,15 @@ public static Iterable<Object[]> parameters() {
equalTo(Math.abs(arg))
);
}));
suppliers.add(new TestCaseSupplier(List.of(DataTypes.UNSIGNED_LONG), () -> {
long arg = randomLong();
return new TestCaseSupplier.TestCase(
List.of(new TestCaseSupplier.TypedData(arg, DataTypes.UNSIGNED_LONG, "arg")),
"Attribute[channel=0]",
DataTypes.UNSIGNED_LONG,
equalTo(arg)
);
}));
TestCaseSupplier.forUnaryUnsignedLong(
suppliers,
"Attribute[channel=0]",
DataTypes.UNSIGNED_LONG,
(n) -> n,
BigInteger.ZERO,
UNSIGNED_LONG_MAX,
List.of()
);
suppliers.add(new TestCaseSupplier(List.of(DataTypes.LONG), () -> {
long arg = randomLong();
return new TestCaseSupplier.TestCase(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;

Expand All @@ -29,7 +31,8 @@ public CeilTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCaseS

@ParametersFactory
public static Iterable<Object[]> parameters() {
return parameterSuppliersFromTypedData(List.of(new TestCaseSupplier("large double value", () -> {
List<TestCaseSupplier> suppliers = new ArrayList<>();
suppliers.addAll(List.of(new TestCaseSupplier("large double value", () -> {
double arg = 1 / randomDouble();
return new TestCaseSupplier.TestCase(
List.of(new TestCaseSupplier.TypedData(arg, DataTypes.DOUBLE, "arg")),
Expand All @@ -53,15 +56,18 @@ public static Iterable<Object[]> parameters() {
DataTypes.LONG,
equalTo(arg)
);
}), new TestCaseSupplier("unsigned long value", () -> {
long arg = randomLong();
return new TestCaseSupplier.TestCase(
List.of(new TestCaseSupplier.TypedData(arg, DataTypes.UNSIGNED_LONG, "arg")),
"Attribute[channel=0]",
DataTypes.UNSIGNED_LONG,
equalTo(arg)
);
})));

TestCaseSupplier.forUnaryUnsignedLong(
suppliers,
"Attribute[channel=0]",
DataTypes.UNSIGNED_LONG,
(n) -> n,
BigInteger.ZERO,
UNSIGNED_LONG_MAX,
List.of()
);
return parameterSuppliersFromTypedData(suppliers);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.elasticsearch.xpack.ql.util.NumericUtils;

import java.math.BigInteger;
import java.util.ArrayList;
Expand All @@ -37,7 +36,7 @@ public static Iterable<Object[]> parameters() {
suppliers,
read,
DataTypes.UNSIGNED_LONG,
ul -> NumericUtils.asLongUnsigned(ul),
ul -> ul,
BigInteger.ZERO,
UNSIGNED_LONG_MAX,
List.of()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.util.NumericUtils;

import java.math.BigInteger;
import java.util.ArrayList;
Expand All @@ -37,12 +36,7 @@ public static Iterable<Object[]> parameters() {
doubles(cases, "mv_max", "MvMax", (size, values) -> equalTo(values.max().getAsDouble()));
ints(cases, "mv_max", "MvMax", (size, values) -> equalTo(values.max().getAsInt()));
longs(cases, "mv_max", "MvMax", (size, values) -> equalTo(values.max().getAsLong()));
unsignedLongs(
cases,
"mv_max",
"MvMax",
(size, values) -> equalTo(NumericUtils.asLongUnsigned(values.reduce(BigInteger::max).get()))
);
unsignedLongs(cases, "mv_max", "MvMax", (size, values) -> equalTo(values.reduce(BigInteger::max).get()));
dateTimes(cases, "mv_max", "MvMax", (size, values) -> equalTo(values.max().getAsLong()));
return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(false, cases)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.elasticsearch.xpack.ql.util.NumericUtils;

import java.math.BigInteger;
import java.util.ArrayList;
Expand Down Expand Up @@ -62,12 +61,12 @@ public static Iterable<Object[]> parameters() {
unsignedLongs(cases, "mv_median", "MvMedian", (size, values) -> {
int middle = size / 2;
if (size % 2 == 1) {
return equalTo(NumericUtils.asLongUnsigned(values.sorted().skip(middle).findFirst().get()));
return equalTo(values.sorted().skip(middle).findFirst().get());
}
var s = values.sorted().skip(middle - 1).limit(2).iterator();
BigInteger a = s.next();
BigInteger b = s.next();
return equalTo(NumericUtils.asLongUnsigned(a.add(b.subtract(a).divide(BigInteger.valueOf(2)))));
return equalTo(a.add(b.subtract(a).divide(BigInteger.valueOf(2))));
});

cases.add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.util.NumericUtils;

import java.math.BigInteger;
import java.util.ArrayList;
Expand All @@ -37,12 +36,7 @@ public static Iterable<Object[]> parameters() {
doubles(cases, "mv_min", "MvMin", (size, values) -> equalTo(values.min().getAsDouble()));
ints(cases, "mv_min", "MvMin", (size, values) -> equalTo(values.min().getAsInt()));
longs(cases, "mv_min", "MvMin", (size, values) -> equalTo(values.min().getAsLong()));
unsignedLongs(
cases,
"mv_min",
"MvMin",
(size, values) -> equalTo(NumericUtils.asLongUnsigned(values.reduce(BigInteger::min).get()))
);
unsignedLongs(cases, "mv_min", "MvMin", (size, values) -> equalTo(values.reduce(BigInteger::min).get()));
dateTimes(cases, "mv_min", "MvMin", (size, values) -> equalTo(values.min().getAsLong()));
return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(false, cases)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.function.BiFunction;

import static org.elasticsearch.xpack.ql.util.NumericUtils.asLongUnsigned;
import static org.elasticsearch.xpack.ql.util.NumericUtils.unsignedLongAsBigInteger;
import static org.elasticsearch.xpack.ql.util.NumericUtils.unsignedLongAsNumber;
import static org.elasticsearch.xpack.ql.util.StringUtils.parseIntegral;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -115,6 +116,11 @@ public void testUnsignedLongMultiplyExact() {
assertThat(mulExact("0", "0"), equalTo("0"));
}

public void testRoundTripConversion() {
BigInteger b = randomUnsignedLongBetween(BigInteger.ZERO, UNSIGNED_LONG_MAX);
assertThat(b, equalTo(unsignedLongAsBigInteger(asLongUnsigned(b))));
}

private static String addExact(String x, String y) {
return exactOperation(x, y, NumericUtils::unsignedLongAddExact);
}
Expand Down

0 comments on commit cf7941f

Please sign in to comment.