Skip to content

Commit 8d53897

Browse files
craigtavernercraigtaverner
committed
Add support for scaled_float and some code-review changes
1 parent 8708f9b commit 8d53897

File tree

2 files changed

+38
-25
lines changed
  • x-pack/plugin/esql/src

2 files changed

+38
-25
lines changed

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

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
5050
import static org.elasticsearch.xpack.esql.core.type.DataType.LONG;
5151
import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
52+
import static org.elasticsearch.xpack.esql.core.type.DataType.SCALED_FLOAT;
5253
import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT;
5354
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
5455
import static org.elasticsearch.xpack.esql.core.type.DataType.TSID_DATA_TYPE;
@@ -151,7 +152,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
151152
}
152153

153154
// Test float and double
154-
var floatTypes = List.of(HALF_FLOAT, FLOAT, DOUBLE);
155+
var floatTypes = List.of(HALF_FLOAT, FLOAT, DOUBLE, SCALED_FLOAT);
155156
{
156157
TestConfigs configs = testConfigurations.computeIfAbsent("floats", TestConfigs::new);
157158
for (DataType mainType : floatTypes) {
@@ -172,6 +173,7 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
172173
}
173174
}
174175
}
176+
175177
// Tests for all unsupported types
176178
DataType[] unsupported = Join.UNSUPPORTED_TYPES;
177179
{
@@ -197,17 +199,16 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
197199
}
198200

199201
// Tests for all types where left and right are the same type
200-
DataType[] supported = { BOOLEAN, LONG, INTEGER, DOUBLE, SHORT, BYTE, FLOAT, HALF_FLOAT, DATETIME, IP, KEYWORD };
202+
DataType[] supported = { BOOLEAN, LONG, INTEGER, DOUBLE, SHORT, BYTE, FLOAT, HALF_FLOAT, DATETIME, IP, KEYWORD, SCALED_FLOAT };
201203
{
202204
Collection<TestConfigs> existing = testConfigurations.values();
203205
TestConfigs configs = testConfigurations.computeIfAbsent("same", TestConfigs::new);
204206
for (DataType type : supported) {
205207
assertThat("Claiming supported for unsupported type: " + type, List.of(unsupported).contains(type), is(false));
206-
if (existingIndex(existing, type, type)) {
207-
// Skip existing configurations
208-
continue;
208+
if (existingIndex(existing, type, type) == false) {
209+
// Only add the configuration if it doesn't already exist
210+
configs.addPasses(type, type);
209211
}
210-
configs.addPasses(type, type);
211212
}
212213
}
213214

@@ -233,11 +234,10 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
233234
TestConfigs configs = testConfigurations.computeIfAbsent("others", TestConfigs::new);
234235
for (DataType mainType : supported) {
235236
for (DataType lookupType : supported) {
236-
if (existingIndex(existing, mainType, lookupType)) {
237-
// Skip existing configurations
238-
continue;
237+
if (existingIndex(existing, mainType, lookupType) == false) {
238+
// Only add the configuration if it doesn't already exist
239+
configs.addFails(mainType, lookupType);
239240
}
240-
configs.addFails(mainType, lookupType);
241241
}
242242
}
243243
}
@@ -305,10 +305,13 @@ private void testLookupJoinTypes(String group) {
305305

306306
private void initIndexes(String group) {
307307
Collection<TestConfig> configs = testConfigurations.get(group).configs.values();
308+
String propertyPrefix = "{\n \"properties\" : {\n";
309+
String propertySuffix = " }\n}\n";
308310
// The main index will have many fields, one of each type to use in later type specific joins
309-
String mainFields = "{\n \"properties\" : {\n"
310-
+ configs.stream().map(TestConfig::mainPropertySpec).distinct().collect(Collectors.joining(",\n "))
311-
+ " }\n}\n";
311+
String mainFields = propertyPrefix + configs.stream()
312+
.map(TestConfig::mainPropertySpec)
313+
.distinct()
314+
.collect(Collectors.joining(",\n ")) + propertySuffix;
312315
assertAcked(prepareCreate("index").setMapping(mainFields));
313316

314317
Settings.Builder settings = Settings.builder()
@@ -319,7 +322,7 @@ private void initIndexes(String group) {
319322
// Each lookup index will get a document with a field to join on, and a results field to get back
320323
(c) -> assertAcked(
321324
prepareCreate(c.indexName()).setSettings(settings.build())
322-
.setMapping(c.fieldName(), "type=" + c.lookupType().esType().replace("cartesian_", ""), "other", "type=keyword")
325+
.setMapping(propertyPrefix + c.lookupPropertySpec() + propertySuffix)
323326
)
324327
);
325328
}
@@ -362,6 +365,8 @@ private static String sampleDataTextFor(DataType type) {
362365
return String.valueOf(value);
363366
}
364367

368+
private static final double SCALING_FACTOR = 1.0;
369+
365370
private static Object sampleDataFor(DataType type) {
366371
return switch (type) {
367372
case BOOLEAN -> true;
@@ -371,6 +376,7 @@ private static Object sampleDataFor(DataType type) {
371376
case BYTE, SHORT, INTEGER -> 1;
372377
case LONG, UNSIGNED_LONG -> 1L;
373378
case HALF_FLOAT, FLOAT, DOUBLE -> 1.0;
379+
case SCALED_FLOAT -> SCALING_FACTOR;
374380
case VERSION -> "1.2.19";
375381
case GEO_POINT, CARTESIAN_POINT -> "POINT (1.0 2.0)";
376382
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))";
@@ -402,10 +408,6 @@ private void addPasses(DataType mainType, DataType lookupType) {
402408
add(new TestConfigPasses(mainType, lookupType, true));
403409
}
404410

405-
private void addEmptyResult(DataType mainType, DataType lookupType) {
406-
add(new TestConfigPasses(mainType, lookupType, false));
407-
}
408-
409411
private void addFails(DataType mainType, DataType lookupType) {
410412
String fieldName = "field_" + mainType.esType();
411413
String errorMessage = String.format(
@@ -443,10 +445,6 @@ private void addFailsUnsupported(DataType mainType, DataType lookupType) {
443445
)
444446
);
445447
}
446-
447-
private <E extends Exception> void addFails(DataType mainType, DataType lookupType, Class<E> exception, Consumer<E> assertion) {
448-
add(new TestConfigFails<>(mainType, lookupType, exception, assertion));
449-
}
450448
}
451449

452450
interface TestConfig {
@@ -463,7 +461,11 @@ default String fieldName() {
463461
}
464462

465463
default String mainPropertySpec() {
466-
return "\"" + fieldName() + "\": { \"type\" : \"" + mainType().esType().replaceAll("cartesian_", "") + "\" }";
464+
return propertySpecFor(fieldName(), mainType(), "");
465+
}
466+
467+
default String lookupPropertySpec() {
468+
return propertySpecFor(fieldName(), lookupType(), ", \"other\": { \"type\" : \"keyword\" }");
467469
}
468470

469471
/** Make sure the left index has the expected fields and types */
@@ -479,6 +481,19 @@ default void validateLookupIndex() {
479481
void testQuery(String query);
480482
}
481483

484+
private static String propertySpecFor(String fieldName, DataType type, String extra) {
485+
if (type == SCALED_FLOAT) {
486+
return String.format(
487+
Locale.ROOT,
488+
"\"%s\": { \"type\" : \"%s\", \"scaling_factor\": %f }",
489+
fieldName,
490+
type.esType(),
491+
SCALING_FACTOR
492+
) + extra;
493+
}
494+
return String.format(Locale.ROOT, "\"%s\": { \"type\" : \"%s\" }", fieldName, type.esType().replaceAll("cartesian_", "")) + extra;
495+
}
496+
482497
private static void validateIndex(String indexName, String fieldName, Object expectedValue) {
483498
String query = String.format(Locale.ROOT, "FROM %s | KEEP %s", indexName, fieldName);
484499
try (var response = EsqlQueryRequestBuilder.newRequestBuilder(client()).query(query).get()) {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
4646
import static org.elasticsearch.xpack.esql.core.type.DataType.OBJECT;
4747
import static org.elasticsearch.xpack.esql.core.type.DataType.PARTIAL_AGG;
48-
import static org.elasticsearch.xpack.esql.core.type.DataType.SCALED_FLOAT;
4948
import static org.elasticsearch.xpack.esql.core.type.DataType.SOURCE;
5049
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
5150
import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION;
@@ -71,7 +70,6 @@ public class Join extends BinaryPlan implements PostAnalysisVerificationAware, S
7170
COUNTER_LONG,
7271
COUNTER_INTEGER,
7372
COUNTER_DOUBLE,
74-
SCALED_FLOAT,
7573
DATE_NANOS,
7674
OBJECT,
7775
SOURCE,

0 commit comments

Comments
 (0)