Skip to content

Commit

Permalink
Render all datatypes in pipetables error output (elastic#118758)
Browse files Browse the repository at this point in the history 
Some types (such as ip or time) require custom formatting logic and were not correctly rendered in actual/expected comparison table.

This change:

* properly formats such data types
* add data types to the column headers
* removes outer pipes
  • Loading branch information
@idegtiarenko
idegtiarenko authored Dec 16, 2024
1 parent b461baf commit caf8afc
Showing 1 changed file with 98 additions and 54 deletions.
152 changes: 98 additions & 54 deletions x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvAssert.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package org.elasticsearch.xpack.esql;

import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.compute.data.Page;
import org.elasticsearch.logging.Logger;
Expand Down Expand Up @@ -197,7 +198,13 @@ public static void assertData(
for (int row = 0; row < expectedValues.size(); row++) {
try {
if (row >= actualValues.size()) {
dataFailure("Expected more data but no more entries found after [" + row + "]", dataFailures, expected, actualValues);
dataFailure(
"Expected more data but no more entries found after [" + row + "]",
dataFailures,
expected,
actualValues,
valueTransformer
);
}

if (logger != null) {
Expand All @@ -208,45 +215,17 @@ public static void assertData(
var actualRow = actualValues.get(row);

for (int column = 0; column < expectedRow.size(); column++) {
var expectedValue = expectedRow.get(column);
var actualValue = actualRow.get(column);
var expectedType = expected.columnTypes().get(column);
var expectedValue = convertExpectedValue(expectedType, expectedRow.get(column));
var actualValue = actualRow.get(column);

if (expectedValue != null) {
// convert the long from CSV back to its STRING form
if (expectedType == Type.DATETIME) {
expectedValue = rebuildExpected(expectedValue, Long.class, x -> UTC_DATE_TIME_FORMATTER.formatMillis((long) x));
} else if (expectedType == Type.DATE_NANOS) {
expectedValue = rebuildExpected(
expectedValue,
Long.class,
x -> DateFormatter.forPattern("strict_date_optional_time_nanos").formatNanos((long) x)
);
} else if (expectedType == Type.GEO_POINT) {
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbToWkt((BytesRef) x));
} else if (expectedType == Type.CARTESIAN_POINT) {
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> CARTESIAN.wkbToWkt((BytesRef) x));
} else if (expectedType == Type.GEO_SHAPE) {
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbToWkt((BytesRef) x));
} else if (expectedType == Type.CARTESIAN_SHAPE) {
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> CARTESIAN.wkbToWkt((BytesRef) x));
} else if (expectedType == Type.IP) {
// convert BytesRef-packed IP to String, allowing subsequent comparison with what's expected
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> DocValueFormat.IP.format((BytesRef) x));
} else if (expectedType == Type.VERSION) {
// convert BytesRef-packed Version to String
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> new Version((BytesRef) x).toString());
} else if (expectedType == UNSIGNED_LONG) {
expectedValue = rebuildExpected(expectedValue, Long.class, x -> unsignedLongAsNumber((long) x));
}
}
var transformedExpected = valueTransformer.apply(expectedType, expectedValue);
var transformedActual = valueTransformer.apply(expectedType, actualValue);
if (Objects.equals(transformedExpected, transformedActual) == false) {
dataFailures.add(new DataFailure(row, column, transformedExpected, transformedActual));
}
if (dataFailures.size() > 10) {
dataFailure("", dataFailures, expected, actualValues);
dataFailure("", dataFailures, expected, actualValues, valueTransformer);
}
}

Expand All @@ -255,7 +234,8 @@ public static void assertData(
"Plan has extra columns, returned [" + actualRow.size() + "], expected [" + expectedRow.size() + "]",
dataFailures,
expected,
actualValues
actualValues,
valueTransformer
);
}
} catch (AssertionError ae) {
Expand All @@ -267,53 +247,89 @@ public static void assertData(
}
}
if (dataFailures.isEmpty() == false) {
dataFailure("", dataFailures, expected, actualValues);
dataFailure("", dataFailures, expected, actualValues, valueTransformer);
}
if (expectedValues.size() < actualValues.size()) {
dataFailure("Elasticsearch still has data after [" + expectedValues.size() + "] entries", dataFailures, expected, actualValues);
dataFailure(
"Elasticsearch still has data after [" + expectedValues.size() + "] entries",
dataFailures,
expected,
actualValues,
valueTransformer
);
}
}

private static void dataFailure(
String description,
List<DataFailure> dataFailures,
ExpectedResults expectedValues,
List<List<Object>> actualValues
List<List<Object>> actualValues,
BiFunction<Type, Object, Object> valueTransformer
) {
var expected = pipeTable("Expected:", expectedValues.columnNames(), expectedValues.values(), 25);
var actual = pipeTable("Actual:", expectedValues.columnNames(), actualValues, 25);
var expected = pipeTable(
"Expected:",
expectedValues.columnNames(),
expectedValues.columnTypes(),
expectedValues.values(),
(type, value) -> valueTransformer.apply(type, convertExpectedValue(type, value))
);
var actual = pipeTable("Actual:", expectedValues.columnNames(), expectedValues.columnTypes(), actualValues, valueTransformer);
fail(description + System.lineSeparator() + describeFailures(dataFailures) + actual + expected);
}

private static String pipeTable(String description, List<String> headers, List<List<Object>> values, int maxRows) {
private static final int MAX_ROWS = 25;

private static String pipeTable(
String description,
List<String> headers,
List<Type> types,
List<List<Object>> values,
BiFunction<Type, Object, Object> valueTransformer
) {
int rows = Math.min(MAX_ROWS, values.size());
int[] width = new int[headers.size()];
for (int i = 0; i < width.length; i++) {
width[i] = headers.get(i).length();
for (List<Object> row : values) {
width[i] = Math.max(width[i], String.valueOf(row.get(i)).length());
String[][] printableValues = new String[rows][headers.size()];
for (int c = 0; c < headers.size(); c++) {
width[c] = header(headers.get(c), types.get(c)).length();
}
for (int r = 0; r < rows; r++) {
for (int c = 0; c < headers.size(); c++) {
printableValues[r][c] = String.valueOf(valueTransformer.apply(types.get(c), values.get(r).get(c)));
width[c] = Math.max(width[c], printableValues[r][c].length());
}
}

var result = new StringBuilder().append(System.lineSeparator()).append(description).append(System.lineSeparator());
for (int c = 0; c < width.length; c++) {
appendValue(result, headers.get(c), width[c]);
// headers
appendPaddedValue(result, header(headers.get(0), types.get(0)), width[0]);
for (int c = 1; c < width.length; c++) {
result.append(" | ");
appendPaddedValue(result, header(headers.get(c), types.get(c)), width[c]);
}
result.append('|').append(System.lineSeparator());
for (int r = 0; r < Math.min(maxRows, values.size()); r++) {
for (int c = 0; c < width.length; c++) {
appendValue(result, values.get(r).get(c), width[c]);
result.append(System.lineSeparator());
// values
for (int r = 0; r < printableValues.length; r++) {
appendPaddedValue(result, printableValues[r][0], width[0]);
for (int c = 1; c < printableValues[r].length; c++) {
result.append(" | ");
appendPaddedValue(result, printableValues[r][c], width[c]);
}
result.append('|').append(System.lineSeparator());
result.append(System.lineSeparator());
}
if (values.size() > maxRows) {
if (values.size() > rows) {
result.append("...").append(System.lineSeparator());
}
return result.toString();
}

private static void appendValue(StringBuilder result, Object value, int width) {
result.append('|').append(value);
for (int i = 0; i < width - String.valueOf(value).length(); i++) {
private static String header(String name, Type type) {
return name + ':' + Strings.toLowercaseAscii(type.name());
}

private static void appendPaddedValue(StringBuilder result, String value, int width) {
result.append(value);
for (int i = 0; i < width - (value != null ? value.length() : 4); i++) {
result.append(' ');
}
}
Expand Down Expand Up @@ -369,6 +385,34 @@ private static Comparator<List<Object>> resultRowComparator(List<Type> types) {
};
}

private static Object convertExpectedValue(Type expectedType, Object expectedValue) {
if (expectedValue == null) {
return null;
}

// convert the long from CSV back to its STRING form
return switch (expectedType) {
case Type.DATETIME -> rebuildExpected(expectedValue, Long.class, x -> UTC_DATE_TIME_FORMATTER.formatMillis((long) x));
case Type.DATE_NANOS -> rebuildExpected(
expectedValue,
Long.class,
x -> DateFormatter.forPattern("strict_date_optional_time_nanos").formatNanos((long) x)
);
case Type.GEO_POINT, Type.GEO_SHAPE -> rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbToWkt((BytesRef) x));
case Type.CARTESIAN_POINT, Type.CARTESIAN_SHAPE -> rebuildExpected(
expectedValue,
BytesRef.class,
x -> CARTESIAN.wkbToWkt((BytesRef) x)
);
case Type.IP -> // convert BytesRef-packed IP to String, allowing subsequent comparison with what's expected
rebuildExpected(expectedValue, BytesRef.class, x -> DocValueFormat.IP.format((BytesRef) x));
case Type.VERSION -> // convert BytesRef-packed Version to String
rebuildExpected(expectedValue, BytesRef.class, x -> new Version((BytesRef) x).toString());
case UNSIGNED_LONG -> rebuildExpected(expectedValue, Long.class, x -> unsignedLongAsNumber((long) x));
default -> expectedValue;
};
}

private static Object rebuildExpected(Object expectedValue, Class<?> clazz, Function<Object, Object> mapper) {
if (List.class.isAssignableFrom(expectedValue.getClass())) {
assertThat(((List<?>) expectedValue).get(0), instanceOf(clazz));
Expand Down

0 comments on commit caf8afc

Please sign in to comment.