Skip to content

Commit

Permalink
QL: Cleanup BWC tests for unsupported version (#86336) (#86581)
Browse files Browse the repository at this point in the history 
Since the jump to 8.x we no longer need some of the version switches in
the bwc tests because we no longer test against version before 7.17.
  • Loading branch information
Lukas Wegmann committed May 10, 2022
1 parent 671fb4a commit e057517
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 108 deletions.
34 changes: 16 additions & 18 deletions ...ql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/eql/qa/mixed_node/EqlSearchIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.elasticsearch.test.NotEqualMessageBuilder;
import org.elasticsearch.test.rest.ESRestTestCase;
import org.elasticsearch.xcontent.json.JsonXContent;
import org.elasticsearch.xpack.eql.execution.search.RuntimeUtils;
import org.elasticsearch.xpack.eql.expression.function.EqlFunctionRegistry;
import org.elasticsearch.xpack.ql.TestNode;
import org.elasticsearch.xpack.ql.TestNodes;
Expand Down Expand Up @@ -118,7 +117,6 @@ public void testMultiValueFields() throws Exception {
.collect(Collectors.toSet());
// each function has a query and query results associated to it
Set<String> testedFunctions = new HashSet<>();
boolean multiValued = nodes.getBWCVersion().onOrAfter(RuntimeUtils.SWITCH_TO_MULTI_VALUE_FIELDS_VERSION);
try (
RestClient client = buildClient(
restClientSettings(),
Expand All @@ -136,7 +134,7 @@ public void testMultiValueFields() throws Exception {
client,
"between",
"PROCESS where between(process_name, \\\"w\\\", \\\"s\\\") : \\\"indow\\\"",
multiValued ? new int[] { 120, 121 } : new int[] { 121 }
new int[] { 120, 121 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -145,7 +143,7 @@ public void testMultiValueFields() throws Exception {
client,
"cidrmatch",
"PROCESS where string(cidrmatch(source_address, \\\"10.6.48.157/24\\\")) : \\\"true\\\"",
multiValued ? new int[] { 121, 122 } : new int[] { 122 }
new int[] { 121, 122 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -154,7 +152,7 @@ public void testMultiValueFields() throws Exception {
client,
"concat",
"PROCESS where concat(file_name, process_name) == \\\"foo\\\" or add(pid, ppid) > 100",
multiValued ? new int[] { 116, 117, 120, 121, 122 } : new int[] { 120, 121 }
new int[] { 116, 117, 120, 121, 122 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -163,7 +161,7 @@ public void testMultiValueFields() throws Exception {
client,
"endswith",
"PROCESS where string(endswith(process_name, \\\"s\\\")) : \\\"true\\\"",
multiValued ? new int[] { 120, 121 } : new int[] { 121 }
new int[] { 120, 121 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -172,7 +170,7 @@ public void testMultiValueFields() throws Exception {
client,
"indexof",
"PROCESS where indexof(file_name, \\\"x\\\", 2) > 0",
multiValued ? new int[] { 116, 117 } : new int[] { 117 }
new int[] { 116, 117 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -181,7 +179,7 @@ public void testMultiValueFields() throws Exception {
client,
"length",
"PROCESS where length(file_name) >= 3 and length(file_name) == 1",
multiValued ? new int[] { 116 } : new int[] {}
new int[] { 116 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -190,7 +188,7 @@ public void testMultiValueFields() throws Exception {
client,
"startswith",
"PROCESS where string(startswith~(file_name, \\\"F\\\")) : \\\"true\\\"",
multiValued ? new int[] { 116, 117, 120, 121 } : new int[] { 116, 120, 121 }
new int[] { 116, 117, 120, 121 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -199,7 +197,7 @@ public void testMultiValueFields() throws Exception {
client,
"string",
"PROCESS where string(concat(file_name, process_name) == \\\"foo\\\") : \\\"true\\\"",
multiValued ? new int[] { 116, 120 } : new int[] { 120 }
new int[] { 116, 120 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -208,7 +206,7 @@ public void testMultiValueFields() throws Exception {
client,
"stringcontains",
"PROCESS where string(stringcontains(file_name, \\\"txt\\\")) : \\\"true\\\"",
multiValued ? new int[] { 117 } : new int[] {}
new int[] { 117 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -217,7 +215,7 @@ public void testMultiValueFields() throws Exception {
client,
"substring",
"PROCESS where substring(file_name, -4) : \\\".txt\\\"",
multiValued ? new int[] { 117 } : new int[] {}
new int[] { 117 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -226,7 +224,7 @@ public void testMultiValueFields() throws Exception {
client,
"add",
"PROCESS where add(pid, 1) == 2",
multiValued ? new int[] { 120, 121, 122 } : new int[] { 120, 121, 122 }
new int[] { 120, 121, 122 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -235,7 +233,7 @@ public void testMultiValueFields() throws Exception {
client,
"divide",
"PROCESS where divide(pid, 12) == 1",
multiValued ? new int[] { 116, 117, 118, 119, 120, 122 } : new int[] { 116, 117, 118, 119 }
new int[] { 116, 117, 118, 119, 120, 122 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -244,7 +242,7 @@ public void testMultiValueFields() throws Exception {
client,
"modulo",
"PROCESS where modulo(ppid, 10) == 0",
multiValued ? new int[] { 121, 122 } : new int[] { 121 }
new int[] { 121, 122 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -253,7 +251,7 @@ public void testMultiValueFields() throws Exception {
client,
"multiply",
"PROCESS where string(multiply(pid, 10) == 120) == \\\"true\\\"",
multiValued ? new int[] { 116, 117, 118, 119, 120, 122 } : new int[] { 116, 117, 118, 119 }
new int[] { 116, 117, 118, 119, 120, 122 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -262,7 +260,7 @@ public void testMultiValueFields() throws Exception {
client,
"number",
"PROCESS where number(command_line) + pid >= 360",
multiValued ? new int[] { 122, 123 } : new int[] { 123 }
new int[] { 122, 123 }
);
assertMultiValueFunctionQuery(
availableFunctions,
Expand All @@ -271,7 +269,7 @@ public void testMultiValueFields() throws Exception {
client,
"subtract",
"PROCESS where subtract(pid, 1) == 0",
multiValued ? new int[] { 120, 121, 122 } : new int[] { 120, 121, 122 }
new int[] { 120, 121, 122 }
);
}

Expand Down
2 changes: 0 additions & 2 deletions .../plugin/ql/src/main/java/org/elasticsearch/xpack/ql/execution/search/QlSourceBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/
package org.elasticsearch.xpack.ql.execution.search;

import org.elasticsearch.Version;
import org.elasticsearch.script.Script;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.fetch.subphase.FieldAndFormat;
Expand All @@ -22,7 +21,6 @@
* the resulting ES document as a field.
*/
public class QlSourceBuilder {
public static final Version INTRODUCING_MISSING_ORDER_IN_COMPOSITE_AGGS_VERSION = Version.V_7_16_0;
// The LinkedHashMaps preserve the order of the fields in the response
private final Set<FieldAndFormat> fetchFields = new LinkedHashSet<>();
private final Map<String, Script> scriptFields = new LinkedHashMap<>();
Expand Down
28 changes: 0 additions & 28 deletions ...ql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlCompatIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.util.stream.Collectors;

import static org.elasticsearch.xpack.ql.TestUtils.buildNodeAndVersions;
import static org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder.INTRODUCING_MISSING_ORDER_IN_COMPOSITE_AGGS_VERSION;

public class SqlCompatIT extends BaseRestSqlTestCase {

Expand Down Expand Up @@ -69,28 +68,6 @@ public static void cleanUpClients() throws IOException {
});
}

public void testNullsOrderBeforeMissingOrderSupportQueryingNewNode() throws IOException {
testNullsOrderBeforeMissingOrderSupport(newNodesClient);
}

public void testNullsOrderBeforeMissingOrderSupportQueryingOldNode() throws IOException {
testNullsOrderBeforeMissingOrderSupport(oldNodesClient);
}

private void testNullsOrderBeforeMissingOrderSupport(RestClient client) throws IOException {
assumeTrue(
"expected some nodes without support for missing_order but got none",
bwcVersion.before(INTRODUCING_MISSING_ORDER_IN_COMPOSITE_AGGS_VERSION)
);

List<Integer> result = runOrderByNullsLastQuery(client);

assertEquals(3, result.size());
assertNull(result.get(0));
assertEquals(Integer.valueOf(1), result.get(1));
assertEquals(Integer.valueOf(2), result.get(2));
}

public void testNullsOrderWithMissingOrderSupportQueryingNewNode() throws IOException {
testNullsOrderWithMissingOrderSupport(newNodesClient);
}
Expand All @@ -100,11 +77,6 @@ public void testNullsOrderWithMissingOrderSupportQueryingOldNode() throws IOExce
}

private void testNullsOrderWithMissingOrderSupport(RestClient client) throws IOException {
assumeTrue(
"expected all nodes with support for missing_order but got some without",
bwcVersion.onOrAfter(INTRODUCING_MISSING_ORDER_IN_COMPOSITE_AGGS_VERSION)
);

List<Integer> result = runOrderByNullsLastQuery(client);

assertEquals(3, result.size());
Expand Down
92 changes: 32 additions & 60 deletions ...ql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@

public class SqlSearchIT extends ESRestTestCase {

/*
* The version where we made a significant change to how we query ES and how we interpret the results we get from ES, is 7.12
* (the switch from extracting from _source and docvalues to using the "fields" API). The behavior of the tests is slightly
* changed on some versions and it all depends on when this above mentioned change was made.
*/
private static final Version FIELDS_API_QL_INTRODUCTION = Version.V_7_12_0;
private static final String index = "test_sql_mixed_versions";
private static int numShards;
private static int numReplicas = 1;
Expand All @@ -55,7 +49,6 @@ public class SqlSearchIT extends ESRestTestCase {
private static List<TestNode> newNodes;
private static List<TestNode> bwcNodes;
private static Version bwcVersion;
private static boolean isBwcNodeBeforeFieldsApiInQL;

@Before
public void createIndex() throws IOException {
Expand All @@ -65,7 +58,6 @@ public void createIndex() throws IOException {
newNodes = new ArrayList<>(nodes.getNewNodes());
bwcNodes = new ArrayList<>(nodes.getBWCNodes());
bwcVersion = nodes.getBWCNodes().get(0).getVersion();
isBwcNodeBeforeFieldsApiInQL = bwcVersion.before(FIELDS_API_QL_INTRODUCTION);

String mappings = readResource(SqlSearchIT.class.getResourceAsStream("/all_field_types.json"));
createIndex(
Expand Down Expand Up @@ -96,32 +88,22 @@ public void testAllTypesWithRequestToOldNodes() throws Exception {
// indexing docvalues and for floating point numbers this may be different from the actual value passed in the _source
// floats were indexed as Doubles and the values returned had a greater precision and more decimals
builder.append(",");
if (isBwcNodeBeforeFieldsApiInQL) {
builder.append("""
"geo_point_field":{"lat":"37.386483", "lon":"-122.083843"},""");
fieldValues.put("geo_point_field", "POINT (-122.08384302444756 37.38648299127817)");
builder.append("\"float_field\":" + randomFloat + ",");
fieldValues.put("float_field", Double.valueOf(randomFloat));
builder.append("\"half_float_field\":123.456");
fieldValues.put("half_float_field", 123.45600128173828d);
} else {
builder.append("""
"geo_point_field":{"lat":"37.386483", "lon":"-122.083843"},""");
fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)");
builder.append("\"float_field\":" + randomFloat + ",");
/*
* Double.valueOf(float.toString) gets a `double` representing
* the `float` that we'd get by going through json which is
* base 10. just casting the `float` to a `double` will get
* a lower number with a lot more trailing digits because
* the cast adds *binary* 0s to the end. And those binary
* 0s don't translate the same as json's decimal 0s.
*/
fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString()));
float roundedHalfFloat = HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(randomFloat));
builder.append("\"half_float_field\":\"" + randomFloat + "\"");
fieldValues.put("half_float_field", Double.valueOf(Float.toString(roundedHalfFloat)));
}
builder.append("""
"geo_point_field":{"lat":"37.386483", "lon":"-122.083843"},""");
fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)");
builder.append("\"float_field\":" + randomFloat + ",");
/*
* Double.valueOf(float.toString) gets a `double` representing
* the `float` that we'd get by going through json which is
* base 10. just casting the `float` to a `double` will get
* a lower number with a lot more trailing digits because
* the cast adds *binary* 0s to the end. And those binary
* 0s don't translate the same as json's decimal 0s.
*/
fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString()));
float roundedHalfFloat = HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(randomFloat));
builder.append("\"half_float_field\":\"" + randomFloat + "\"");
fieldValues.put("half_float_field", Double.valueOf(Float.toString(roundedHalfFloat)));
});
assertAllTypesWithNodes(expectedResponse, bwcNodes);
}
Expand All @@ -134,32 +116,22 @@ public void testAllTypesWithRequestToUpgradedNodes() throws Exception {
}, (builder, fieldValues) -> {
Float randomFloat = randomFloat();
builder.append(",");
if (isBwcNodeBeforeFieldsApiInQL) {
builder.append("""
"geo_point_field":{"lat":"37.386483", "lon":"-122.083843"},""");
fieldValues.put("geo_point_field", "POINT (-122.08384302444756 37.38648299127817)");
builder.append("\"float_field\":" + randomFloat + ",");
fieldValues.put("float_field", Double.valueOf(randomFloat));
builder.append("\"half_float_field\":123.456");
fieldValues.put("half_float_field", 123.45600128173828d);
} else {
builder.append("""
"geo_point_field":{"lat":"37.386483", "lon":"-122.083843"},""");
fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)");
builder.append("\"float_field\":" + randomFloat + ",");
/*
* Double.valueOf(float.toString) gets a `double` representing
* the `float` that we'd get by going through json which is
* base 10. just casting the `float` to a `double` will get
* a lower number with a lot more trailing digits because
* the cast adds *binary* 0s to the end. And those binary
* 0s don't translate the same as json's decimal 0s.
*/
fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString()));
float roundedHalfFloat = HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(randomFloat));
builder.append("\"half_float_field\":\"" + randomFloat + "\"");
fieldValues.put("half_float_field", Double.valueOf(Float.toString(roundedHalfFloat)));
}
builder.append("""
"geo_point_field":{"lat":"37.386483", "lon":"-122.083843"},""");
fieldValues.put("geo_point_field", "POINT (-122.083843 37.386483)");
builder.append("\"float_field\":" + randomFloat + ",");
/*
* Double.valueOf(float.toString) gets a `double` representing
* the `float` that we'd get by going through json which is
* base 10. just casting the `float` to a `double` will get
* a lower number with a lot more trailing digits because
* the cast adds *binary* 0s to the end. And those binary
* 0s don't translate the same as json's decimal 0s.
*/
fieldValues.put("float_field", Double.valueOf(Float.valueOf(randomFloat).toString()));
float roundedHalfFloat = HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(randomFloat));
builder.append("\"half_float_field\":\"" + randomFloat + "\"");
fieldValues.put("half_float_field", Double.valueOf(Float.toString(roundedHalfFloat)));
});
assertAllTypesWithNodes(expectedResponse, newNodes);
}
Expand Down

0 comments on commit e057517

Please sign in to comment.