Skip to content

Commit 8708f9b

Browse files
craigtavernercraigtaverner
committed
Add negative tests for all unsupported types
1 parent 71a0f59 commit 8708f9b

File tree

3 files changed

+133
-20
lines changed

3 files changed

+133
-20
lines changed

‎x-pack/plugin/esql/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ dependencies {
5353
testImplementation project(path: xpackModule('enrich'))
5454
testImplementation project(path: xpackModule('spatial'))
5555
testImplementation project(path: xpackModule('kql'))
56+
testImplementation project(path: xpackModule('mapper-unsigned-long'))
5657

5758
testImplementation project(path: ':modules:reindex')
5859
testImplementation project(path: ':modules:parent-join')

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/LookupJoinTypesIT.java‎

Lines changed: 84 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,13 @@
1515
import org.elasticsearch.xpack.core.esql.action.ColumnInfo;
1616
import org.elasticsearch.xpack.esql.VerificationException;
1717
import org.elasticsearch.xpack.esql.core.type.DataType;
18+
import org.elasticsearch.xpack.esql.plan.logical.join.Join;
1819
import org.elasticsearch.xpack.esql.plugin.EsqlPlugin;
20+
import org.elasticsearch.xpack.spatial.SpatialPlugin;
21+
import org.elasticsearch.xpack.unsignedlong.UnsignedLongMapperPlugin;
22+
import org.elasticsearch.xpack.versionfield.VersionFieldPlugin;
1923

24+
import java.util.ArrayList;
2025
import java.util.Collection;
2126
import java.util.HashMap;
2227
import java.util.HashSet;
@@ -31,18 +36,22 @@
3136

3237
import static org.elasticsearch.test.ESIntegTestCase.Scope.SUITE;
3338
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
39+
import static org.elasticsearch.xpack.esql.core.type.DataType.AGGREGATE_METRIC_DOUBLE;
3440
import static org.elasticsearch.xpack.esql.core.type.DataType.BOOLEAN;
3541
import static org.elasticsearch.xpack.esql.core.type.DataType.BYTE;
3642
import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME;
43+
import static org.elasticsearch.xpack.esql.core.type.DataType.DOC_DATA_TYPE;
3744
import static org.elasticsearch.xpack.esql.core.type.DataType.DOUBLE;
3845
import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT;
3946
import static org.elasticsearch.xpack.esql.core.type.DataType.HALF_FLOAT;
4047
import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER;
4148
import static org.elasticsearch.xpack.esql.core.type.DataType.IP;
4249
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
4350
import static org.elasticsearch.xpack.esql.core.type.DataType.LONG;
51+
import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
4452
import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT;
4553
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
54+
import static org.elasticsearch.xpack.esql.core.type.DataType.TSID_DATA_TYPE;
4655
import static org.hamcrest.Matchers.containsString;
4756
import static org.hamcrest.Matchers.equalTo;
4857
import static org.hamcrest.Matchers.is;
@@ -108,7 +117,13 @@
108117
@ClusterScope(scope = SUITE, numClientNodes = 1, numDataNodes = 1)
109118
public class LookupJoinTypesIT extends ESIntegTestCase {
110119
protected Collection<Class<? extends Plugin>> nodePlugins() {
111-
return List.of(EsqlPlugin.class, MapperExtrasPlugin.class);
120+
return List.of(
121+
EsqlPlugin.class,
122+
MapperExtrasPlugin.class,
123+
VersionFieldPlugin.class,
124+
UnsignedLongMapperPlugin.class,
125+
SpatialPlugin.class
126+
);
112127
}
113128

114129
private static final Map<String, TestConfigs> testConfigurations = new HashMap<>();
@@ -118,8 +133,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
118133
TestConfigs configs = testConfigurations.computeIfAbsent("strings", TestConfigs::new);
119134
configs.addPasses(KEYWORD, KEYWORD);
120135
configs.addPasses(TEXT, KEYWORD);
121-
configs.addFailsText(KEYWORD);
122-
configs.addFailsText(TEXT);
136+
configs.addFailsUnsupported(KEYWORD, TEXT);
123137
}
124138

125139
// Test integer types
@@ -158,13 +172,37 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
158172
}
159173
}
160174
}
175+
// Tests for all unsupported types
176+
DataType[] unsupported = Join.UNSUPPORTED_TYPES;
177+
{
178+
Collection<TestConfigs> existing = testConfigurations.values();
179+
TestConfigs configs = testConfigurations.computeIfAbsent("unsupported", TestConfigs::new);
180+
for (DataType type : unsupported) {
181+
if (type == NULL
182+
|| type == DOC_DATA_TYPE
183+
|| type == TSID_DATA_TYPE
184+
|| type == AGGREGATE_METRIC_DOUBLE
185+
|| type.esType() == null
186+
|| type.isCounter()
187+
|| DataType.isRepresentable(type) == false) {
188+
// Skip unmappable types, or types not supported in ES|QL in general
189+
continue;
190+
}
191+
if (existingIndex(existing, type, type)) {
192+
// Skip existing configurations
193+
continue;
194+
}
195+
configs.addFailsUnsupported(type, type);
196+
}
197+
}
161198

162199
// Tests for all types where left and right are the same type
163-
DataType[] all = { BOOLEAN, LONG, INTEGER, DOUBLE, SHORT, BYTE, FLOAT, HALF_FLOAT, DATETIME, IP, KEYWORD };
200+
DataType[] supported = { BOOLEAN, LONG, INTEGER, DOUBLE, SHORT, BYTE, FLOAT, HALF_FLOAT, DATETIME, IP, KEYWORD };
164201
{
165202
Collection<TestConfigs> existing = testConfigurations.values();
166203
TestConfigs configs = testConfigurations.computeIfAbsent("same", TestConfigs::new);
167-
for (DataType type : all) {
204+
for (DataType type : supported) {
205+
assertThat("Claiming supported for unsupported type: " + type, List.of(unsupported).contains(type), is(false));
168206
if (existingIndex(existing, type, type)) {
169207
// Skip existing configurations
170208
continue;
@@ -173,12 +211,28 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
173211
}
174212
}
175213

214+
// Assert that unsupported types are not in the supported list
215+
for (DataType type : unsupported) {
216+
assertThat("Claiming supported for unsupported type: " + type, List.of(supported).contains(type), is(false));
217+
}
218+
219+
// Assert that unsupported+supported covers all types:
220+
List<DataType> missing = new ArrayList<>();
221+
for (DataType type : DataType.values()) {
222+
boolean isUnsupported = List.of(unsupported).contains(type);
223+
boolean isSupported = List.of(supported).contains(type);
224+
if (isUnsupported == false && isSupported == false) {
225+
missing.add(type);
226+
}
227+
}
228+
assertThat(missing + " are not in the supported or unsupported list", missing.size(), is(0));
229+
176230
// Tests for all other type combinations
177231
{
178232
Collection<TestConfigs> existing = testConfigurations.values();
179233
TestConfigs configs = testConfigurations.computeIfAbsent("others", TestConfigs::new);
180-
for (DataType mainType : all) {
181-
for (DataType lookupType : all) {
234+
for (DataType mainType : supported) {
235+
for (DataType lookupType : supported) {
182236
if (existingIndex(existing, mainType, lookupType)) {
183237
// Skip existing configurations
184238
continue;
@@ -188,8 +242,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
188242
}
189243
}
190244

191-
// TODO: Add tests for more types, eg. unsigned_long, version, spatial types, date/temporal types.
192-
193245
// Make sure we have never added two configurations with the same index name
194246
Set<String> knownTypes = new HashSet<>();
195247
for (TestConfigs configs : testConfigurations.values()) {
@@ -227,6 +279,10 @@ public void testLookupJoinSame() {
227279
testLookupJoinTypes("same");
228280
}
229281

282+
public void testLookupJoinUnsupported() {
283+
testLookupJoinTypes("unsupported");
284+
}
285+
230286
public void testLookupJoinOthers() {
231287
testLookupJoinTypes("others");
232288
}
@@ -263,7 +319,7 @@ private void initIndexes(String group) {
263319
// Each lookup index will get a document with a field to join on, and a results field to get back
264320
(c) -> assertAcked(
265321
prepareCreate(c.indexName()).setSettings(settings.build())
266-
.setMapping(c.fieldName(), "type=" + c.lookupType().esType(), "other", "type=keyword")
322+
.setMapping(c.fieldName(), "type=" + c.lookupType().esType().replace("cartesian_", ""), "other", "type=keyword")
267323
)
268324
);
269325
}
@@ -299,10 +355,11 @@ private String mainPropertyFor(TestConfig config) {
299355
}
300356

301357
private static String sampleDataTextFor(DataType type) {
302-
return switch (type) {
303-
case KEYWORD, TEXT, DATETIME, DATE_NANOS, IP -> "\"" + sampleDataFor(type) + "\"";
304-
default -> String.valueOf(sampleDataFor(type));
305-
};
358+
var value = sampleDataFor(type);
359+
if (value instanceof String) {
360+
return "\"" + value + "\"";
361+
}
362+
return String.valueOf(value);
306363
}
307364

308365
private static Object sampleDataFor(DataType type) {
@@ -312,8 +369,11 @@ private static Object sampleDataFor(DataType type) {
312369
case IP -> "127.0.0.1";
313370
case KEYWORD, TEXT -> "key";
314371
case BYTE, SHORT, INTEGER -> 1;
315-
case LONG -> 1L;
372+
case LONG, UNSIGNED_LONG -> 1L;
316373
case HALF_FLOAT, FLOAT, DOUBLE -> 1.0;
374+
case VERSION -> "1.2.19";
375+
case GEO_POINT, CARTESIAN_POINT -> "POINT (1.0 2.0)";
376+
case GEO_SHAPE, CARTESIAN_SHAPE -> "POLYGON ((0.0 0.0, 1.0 0.0, 1.0 1.0, 0.0 1.0, 0.0 0.0))";
317377
default -> throw new IllegalArgumentException("Unsupported type: " + type);
318378
};
319379
}
@@ -366,13 +426,18 @@ private void addFails(DataType mainType, DataType lookupType) {
366426
);
367427
}
368428

369-
private void addFailsText(DataType mainType) {
429+
private void addFailsUnsupported(DataType mainType, DataType lookupType) {
370430
String fieldName = "field_" + mainType.esType();
371-
String errorMessage = String.format(Locale.ROOT, "JOIN with right field [%s] of type [TEXT] is not supported", fieldName);
431+
String errorMessage = String.format(
432+
Locale.ROOT,
433+
"JOIN with right field [%s] of type [%s] is not supported",
434+
fieldName,
435+
lookupType
436+
);
372437
add(
373438
new TestConfigFails<>(
374439
mainType,
375-
DataType.TEXT,
440+
lookupType,
376441
VerificationException.class,
377442
e -> assertThat(e.getMessage(), containsString(errorMessage))
378443
)
@@ -398,7 +463,7 @@ default String fieldName() {
398463
}
399464

400465
default String mainPropertySpec() {
401-
return "\"" + fieldName() + "\": { \"type\" : \"" + mainType().esType() + "\" }";
466+
return "\"" + fieldName() + "\": { \"type\" : \"" + mainType().esType().replaceAll("cartesian_", "") + "\" }";
402467
}
403468

404469
/** Make sure the left index has the expected fields and types */

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,69 @@
1818
import org.elasticsearch.xpack.esql.core.expression.ReferenceAttribute;
1919
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
2020
import org.elasticsearch.xpack.esql.core.tree.Source;
21+
import org.elasticsearch.xpack.esql.core.type.DataType;
2122
import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
2223
import org.elasticsearch.xpack.esql.plan.logical.BinaryPlan;
2324
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
2425
import org.elasticsearch.xpack.esql.plan.logical.SortAgnostic;
2526

2627
import java.io.IOException;
2728
import java.util.ArrayList;
29+
import java.util.Arrays;
2830
import java.util.List;
2931
import java.util.Objects;
3032

3133
import static org.elasticsearch.xpack.esql.common.Failure.fail;
34+
import static org.elasticsearch.xpack.esql.core.type.DataType.AGGREGATE_METRIC_DOUBLE;
35+
import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_POINT;
36+
import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_SHAPE;
37+
import static org.elasticsearch.xpack.esql.core.type.DataType.COUNTER_DOUBLE;
38+
import static org.elasticsearch.xpack.esql.core.type.DataType.COUNTER_INTEGER;
39+
import static org.elasticsearch.xpack.esql.core.type.DataType.COUNTER_LONG;
40+
import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_NANOS;
41+
import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_PERIOD;
42+
import static org.elasticsearch.xpack.esql.core.type.DataType.DOC_DATA_TYPE;
43+
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT;
44+
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_SHAPE;
45+
import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
46+
import static org.elasticsearch.xpack.esql.core.type.DataType.OBJECT;
47+
import static org.elasticsearch.xpack.esql.core.type.DataType.PARTIAL_AGG;
48+
import static org.elasticsearch.xpack.esql.core.type.DataType.SCALED_FLOAT;
49+
import static org.elasticsearch.xpack.esql.core.type.DataType.SOURCE;
3250
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
51+
import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION;
52+
import static org.elasticsearch.xpack.esql.core.type.DataType.TSID_DATA_TYPE;
53+
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
54+
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSUPPORTED;
55+
import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
3356
import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputAttributes;
3457
import static org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.LEFT;
3558

3659
public class Join extends BinaryPlan implements PostAnalysisVerificationAware, SortAgnostic {
3760
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(LogicalPlan.class, "Join", Join::new);
61+
public static final DataType[] UNSUPPORTED_TYPES = {
62+
TEXT,
63+
VERSION,
64+
UNSIGNED_LONG,
65+
GEO_POINT,
66+
GEO_SHAPE,
67+
CARTESIAN_POINT,
68+
CARTESIAN_SHAPE,
69+
UNSUPPORTED,
70+
NULL,
71+
COUNTER_LONG,
72+
COUNTER_INTEGER,
73+
COUNTER_DOUBLE,
74+
SCALED_FLOAT,
75+
DATE_NANOS,
76+
OBJECT,
77+
SOURCE,
78+
DATE_PERIOD,
79+
TIME_DURATION,
80+
DOC_DATA_TYPE,
81+
TSID_DATA_TYPE,
82+
PARTIAL_AGG,
83+
AGGREGATE_METRIC_DOUBLE };
3884

3985
private final JoinConfig config;
4086
private List<Attribute> lazyOutput;
@@ -229,7 +275,8 @@ public void postAnalysisVerification(Failures failures) {
229275
)
230276
);
231277
}
232-
if (rightField.dataType().equals(TEXT)) {
278+
// TODO: Add support for VERSION by implementing QueryList.versionTermQueryList similar to ipTermQueryList
279+
if (Arrays.stream(UNSUPPORTED_TYPES).anyMatch(t -> rightField.dataType().equals(t))) {
233280
failures.add(
234281
fail(leftField, "JOIN with right field [{}] of type [{}] is not supported", rightField.name(), rightField.dataType())
235282
);

0 commit comments

Comments
 (0)