Move ESQL's LOCATE test cases to cases#107271
Conversation
This moves the test cases declared in the tests for ESQL's LOCATE function to test cases which will cause elastic#106782 to properly generate all of the available signatures. It also buys us all of testing for incorrect parameter combinations.
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| * {@link EvalOperator.ExpressionEvaluator} implementation for {@link Locate}. | ||
| * This class is generated. Do not edit it. | ||
| */ | ||
| public final class LocateNoStartEvaluator implements EvalOperator.ExpressionEvaluator { |
There was a problem hiding this comment.
hmm... yes I noticed this one too locally. Thanks for committing.
| } | ||
|
|
||
| return start == null ? TypeResolution.TYPE_RESOLVED : isInteger(start, sourceText(), THIRD); | ||
| return start == null ? TypeResolution.TYPE_RESOLVED : isType(start, dt -> dt == DataTypes.INTEGER, sourceText(), THIRD, "integer"); |
There was a problem hiding this comment.
This was incorrectly letting through longs. The rest of the class didn't work if you passed longs here so I figured I'd fix it this way. The other option is to keep this and cast the long parameters down to integers when we receive them. Fine either way, but long is very very large an index into an array.
There was a problem hiding this comment.
good catch @nik9000, either solution is fine by me.
| Map.entry(Set.of(DataTypes.LONG, DataTypes.INTEGER, DataTypes.UNSIGNED_LONG, DataTypes.DOUBLE, DataTypes.NULL), "numeric"), | ||
| Map.entry(Set.of(DataTypes.KEYWORD, DataTypes.TEXT, DataTypes.VERSION, DataTypes.NULL), "string or version"), | ||
| Map.entry(Set.of(DataTypes.KEYWORD, DataTypes.TEXT, DataTypes.NULL), "string"), | ||
| Map.entry(Set.of(DataTypes.KEYWORD, DataTypes.TEXT), "string"), |
There was a problem hiding this comment.
I don't believe we need these. Dropping them.
| } | ||
|
|
||
| static String nameFromTypes(List<DataType> types) { | ||
| public static String nameFromTypes(List<DataType> types) { |
There was a problem hiding this comment.
I steal this to name a few things.
| str -> randomRealisticUnicodeOfCodepointLength(2), | ||
| () -> between(0, 3), | ||
| (str, substr, start) -> 1 + str.indexOf(substr, start) | ||
| ) |
There was a problem hiding this comment.
This random unicode tests will cover some of the cases below, but pretty rarely.
| assert "🐱".getBytes(StandardCharsets.UTF_8).length == 4 && "🐶".getBytes(StandardCharsets.UTF_8).length == 4; | ||
| suppliers.add(supplier("🐱Meow!🐶Woof!", "🐱Meow!🐶Woof!", null, 1)); | ||
| suppliers.add(supplier("🐱Meow!🐶Woof!", "Meow!🐶Woof!", 0, 2)); | ||
| suppliers.add(supplier("🐱Meow!🐶Woof!", "eow!🐶Woof!", 0, 3)); |
There was a problem hiding this comment.
All of your tests live here now. It's weird, but it's how the rest of the function tests work. And it works pretty well.
|
Thanks @tteofili ! |
This moves the test cases declared in the tests for ESQL's LOCATE function to test cases which will cause #106782 to properly generate all of the available signatures. It also buys us all of testing for incorrect parameter combinations.