Skip to content

Commit 16d7575

Browse files
idegtiarenkoidegtiarenko
authored
[8.x] Backport pipe tables error messages (#118763)
* Update esql error message (#118277) This change update esql comparison error message to also include expected and actual values in order to make it easier to spot the incorrect actual data. (cherry picked from commit 655090b) * Render all datatypes in pipetables error output (#118758) 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 (cherry picked from commit caf8afc) * remove qualifiers * spotless
1 parent 9166cd8 commit 16d7575

File tree

1 file changed

+126
-48
lines changed
  • x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql

1 file changed

+126
-48
lines changed

‎x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvAssert.java‎

Lines changed: 126 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package org.elasticsearch.xpack.esql;
99

1010
import org.apache.lucene.util.BytesRef;
11+
import org.elasticsearch.common.Strings;
1112
import org.elasticsearch.common.time.DateFormatter;
1213
import org.elasticsearch.compute.data.Page;
1314
import org.elasticsearch.logging.Logger;
@@ -39,9 +40,9 @@
3940
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.unsignedLongAsNumber;
4041
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.CARTESIAN;
4142
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO;
43+
import static org.hamcrest.MatcherAssert.assertThat;
4244
import static org.hamcrest.Matchers.instanceOf;
4345
import static org.junit.Assert.assertEquals;
44-
import static org.junit.Assert.assertThat;
4546
import static org.junit.Assert.fail;
4647

4748
public final class CsvAssert {
@@ -197,11 +198,13 @@ public static void assertData(
197198
for (int row = 0; row < expectedValues.size(); row++) {
198199
try {
199200
if (row >= actualValues.size()) {
200-
if (dataFailures.isEmpty()) {
201-
fail("Expected more data but no more entries found after [" + row + "]");
202-
} else {
203-
dataFailure(dataFailures, "Expected more data but no more entries found after [" + row + "]\n");
204-
}
201+
dataFailure(
202+
"Expected more data but no more entries found after [" + row + "]",
203+
dataFailures,
204+
expected,
205+
actualValues,
206+
valueTransformer
207+
);
205208
}
206209

207210
if (logger != null) {
@@ -212,51 +215,28 @@ public static void assertData(
212215
var actualRow = actualValues.get(row);
213216

214217
for (int column = 0; column < expectedRow.size(); column++) {
215-
var expectedValue = expectedRow.get(column);
216-
var actualValue = actualRow.get(column);
217218
var expectedType = expected.columnTypes().get(column);
219+
var expectedValue = convertExpectedValue(expectedType, expectedRow.get(column));
220+
var actualValue = actualRow.get(column);
218221

219-
if (expectedValue != null) {
220-
// convert the long from CSV back to its STRING form
221-
if (expectedType == Type.DATETIME) {
222-
expectedValue = rebuildExpected(expectedValue, Long.class, x -> UTC_DATE_TIME_FORMATTER.formatMillis((long) x));
223-
} else if (expectedType == Type.DATE_NANOS) {
224-
expectedValue = rebuildExpected(
225-
expectedValue,
226-
Long.class,
227-
x -> DateFormatter.forPattern("strict_date_optional_time_nanos").formatNanos((long) x)
228-
);
229-
} else if (expectedType == Type.GEO_POINT) {
230-
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbToWkt((BytesRef) x));
231-
} else if (expectedType == Type.CARTESIAN_POINT) {
232-
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> CARTESIAN.wkbToWkt((BytesRef) x));
233-
} else if (expectedType == Type.GEO_SHAPE) {
234-
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbToWkt((BytesRef) x));
235-
} else if (expectedType == Type.CARTESIAN_SHAPE) {
236-
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> CARTESIAN.wkbToWkt((BytesRef) x));
237-
} else if (expectedType == Type.IP) {
238-
// convert BytesRef-packed IP to String, allowing subsequent comparison with what's expected
239-
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> DocValueFormat.IP.format((BytesRef) x));
240-
} else if (expectedType == Type.VERSION) {
241-
// convert BytesRef-packed Version to String
242-
expectedValue = rebuildExpected(expectedValue, BytesRef.class, x -> new Version((BytesRef) x).toString());
243-
} else if (expectedType == UNSIGNED_LONG) {
244-
expectedValue = rebuildExpected(expectedValue, Long.class, x -> unsignedLongAsNumber((long) x));
245-
}
246-
}
247222
var transformedExpected = valueTransformer.apply(expectedType, expectedValue);
248223
var transformedActual = valueTransformer.apply(expectedType, actualValue);
249224
if (Objects.equals(transformedExpected, transformedActual) == false) {
250225
dataFailures.add(new DataFailure(row, column, transformedExpected, transformedActual));
251226
}
252227
if (dataFailures.size() > 10) {
253-
dataFailure(dataFailures);
228+
dataFailure("", dataFailures, expected, actualValues, valueTransformer);
254229
}
255230
}
256231

257-
var delta = actualRow.size() - expectedRow.size();
258-
if (delta > 0) {
259-
fail("Plan has extra columns, returned [" + actualRow.size() + "], expected [" + expectedRow.size() + "]");
232+
if (actualRow.size() != expectedRow.size()) {
233+
dataFailure(
234+
"Plan has extra columns, returned [" + actualRow.size() + "], expected [" + expectedRow.size() + "]",
235+
dataFailures,
236+
expected,
237+
actualValues,
238+
valueTransformer
239+
);
260240
}
261241
} catch (AssertionError ae) {
262242
if (logger != null && row + 1 < actualValues.size()) {
@@ -267,21 +247,95 @@ public static void assertData(
267247
}
268248
}
269249
if (dataFailures.isEmpty() == false) {
270-
dataFailure(dataFailures);
250+
dataFailure("", dataFailures, expected, actualValues, valueTransformer);
271251
}
272252
if (expectedValues.size() < actualValues.size()) {
273-
fail(
274-
"Elasticsearch still has data after [" + expectedValues.size() + "] entries:\n" + row(actualValues, expectedValues.size())
253+
dataFailure(
254+
"Elasticsearch still has data after [" + expectedValues.size() + "] entries",
255+
dataFailures,
256+
expected,
257+
actualValues,
258+
valueTransformer
275259
);
276260
}
277261
}
278262

279-
private static void dataFailure(List<DataFailure> dataFailures) {
280-
dataFailure(dataFailures, "");
263+
private static void dataFailure(
264+
String description,
265+
List<DataFailure> dataFailures,
266+
ExpectedResults expectedValues,
267+
List<List<Object>> actualValues,
268+
BiFunction<Type, Object, Object> valueTransformer
269+
) {
270+
var expected = pipeTable(
271+
"Expected:",
272+
expectedValues.columnNames(),
273+
expectedValues.columnTypes(),
274+
expectedValues.values(),
275+
(type, value) -> valueTransformer.apply(type, convertExpectedValue(type, value))
276+
);
277+
var actual = pipeTable("Actual:", expectedValues.columnNames(), expectedValues.columnTypes(), actualValues, valueTransformer);
278+
fail(description + System.lineSeparator() + describeFailures(dataFailures) + actual + expected);
279+
}
280+
281+
private static final int MAX_ROWS = 25;
282+
283+
private static String pipeTable(
284+
String description,
285+
List<String> headers,
286+
List<Type> types,
287+
List<List<Object>> values,
288+
BiFunction<Type, Object, Object> valueTransformer
289+
) {
290+
int rows = Math.min(MAX_ROWS, values.size());
291+
int[] width = new int[headers.size()];
292+
String[][] printableValues = new String[rows][headers.size()];
293+
for (int c = 0; c < headers.size(); c++) {
294+
width[c] = header(headers.get(c), types.get(c)).length();
295+
}
296+
for (int r = 0; r < rows; r++) {
297+
for (int c = 0; c < headers.size(); c++) {
298+
printableValues[r][c] = String.valueOf(valueTransformer.apply(types.get(c), values.get(r).get(c)));
299+
width[c] = Math.max(width[c], printableValues[r][c].length());
300+
}
301+
}
302+
303+
var result = new StringBuilder().append(System.lineSeparator()).append(description).append(System.lineSeparator());
304+
// headers
305+
appendPaddedValue(result, header(headers.get(0), types.get(0)), width[0]);
306+
for (int c = 1; c < width.length; c++) {
307+
result.append(" | ");
308+
appendPaddedValue(result, header(headers.get(c), types.get(c)), width[c]);
309+
}
310+
result.append(System.lineSeparator());
311+
// values
312+
for (int r = 0; r < printableValues.length; r++) {
313+
appendPaddedValue(result, printableValues[r][0], width[0]);
314+
for (int c = 1; c < printableValues[r].length; c++) {
315+
result.append(" | ");
316+
appendPaddedValue(result, printableValues[r][c], width[c]);
317+
}
318+
result.append(System.lineSeparator());
319+
}
320+
if (values.size() > rows) {
321+
result.append("...").append(System.lineSeparator());
322+
}
323+
return result.toString();
324+
}
325+
326+
private static String header(String name, Type type) {
327+
return name + ':' + Strings.toLowercaseAscii(type.name());
328+
}
329+
330+
private static void appendPaddedValue(StringBuilder result, String value, int width) {
331+
result.append(value);
332+
for (int i = 0; i < width - (value != null ? value.length() : 4); i++) {
333+
result.append(' ');
334+
}
281335
}
282336

283-
private static void dataFailure(List<DataFailure> dataFailures, String prefixError) {
284-
fail(prefixError + "Data mismatch:\n" + dataFailures.stream().map(f -> {
337+
private static String describeFailures(List<DataFailure> dataFailures) {
338+
return "Data mismatch:" + System.lineSeparator() + dataFailures.stream().map(f -> {
285339
Description description = new StringDescription();
286340
ListMatcher expected;
287341
if (f.expected instanceof List<?> e) {
@@ -299,7 +353,7 @@ private static void dataFailure(List<DataFailure> dataFailures, String prefixErr
299353
expected.describeMismatch(actualList, description);
300354
String prefix = "row " + f.row + " column " + f.column + ":";
301355
return prefix + description.toString().replace("\n", "\n" + prefix);
302-
}).collect(Collectors.joining("\n")));
356+
}).collect(Collectors.joining(System.lineSeparator()));
303357
}
304358

305359
private static Comparator<List<Object>> resultRowComparator(List<Type> types) {
@@ -331,6 +385,30 @@ private static Comparator<List<Object>> resultRowComparator(List<Type> types) {
331385
};
332386
}
333387

388+
private static Object convertExpectedValue(Type expectedType, Object expectedValue) {
389+
if (expectedValue == null) {
390+
return null;
391+
}
392+
393+
// convert the long from CSV back to its STRING form
394+
return switch (expectedType) {
395+
case DATETIME -> rebuildExpected(expectedValue, Long.class, x -> UTC_DATE_TIME_FORMATTER.formatMillis((long) x));
396+
case DATE_NANOS -> rebuildExpected(
397+
expectedValue,
398+
Long.class,
399+
x -> DateFormatter.forPattern("strict_date_optional_time_nanos").formatNanos((long) x)
400+
);
401+
case GEO_POINT, GEO_SHAPE -> rebuildExpected(expectedValue, BytesRef.class, x -> GEO.wkbToWkt((BytesRef) x));
402+
case CARTESIAN_POINT, CARTESIAN_SHAPE -> rebuildExpected(expectedValue, BytesRef.class, x -> CARTESIAN.wkbToWkt((BytesRef) x));
403+
case IP -> // convert BytesRef-packed IP to String, allowing subsequent comparison with what's expected
404+
rebuildExpected(expectedValue, BytesRef.class, x -> DocValueFormat.IP.format((BytesRef) x));
405+
case VERSION -> // convert BytesRef-packed Version to String
406+
rebuildExpected(expectedValue, BytesRef.class, x -> new Version((BytesRef) x).toString());
407+
case UNSIGNED_LONG -> rebuildExpected(expectedValue, Long.class, x -> unsignedLongAsNumber((long) x));
408+
default -> expectedValue;
409+
};
410+
}
411+
334412
private static Object rebuildExpected(Object expectedValue, Class<?> clazz, Function<Object, Object> mapper) {
335413
if (List.class.isAssignableFrom(expectedValue.getClass())) {
336414
assertThat(((List<?>) expectedValue).get(0), instanceOf(clazz));

0 commit comments

Comments
 (0)