From 0e7f61ae4db0293a08038ba6fa67fae332239b76 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 30 Mar 2022 17:40:53 +0200 Subject: [PATCH 01/20] Add support for VERSION field type in SQL and EQL --- .../sql/language/data-types.asciidoc | 1 + .../xpack/eql/analysis/VerifierTests.java | 5 ++++ .../src/test/resources/mapping-version.json | 17 +++++++++++ .../xpack/ql/type/DataTypeConverter.java | 15 ++++++++++ .../xpack/ql/type/DataTypes.java | 3 ++ .../ql/type/DataTypeConversionTests.java | 19 +++++++++++++ .../xpack/ql/type/TypesTests.java | 7 +++++ .../src/test/resources/mapping-version.json | 7 +++++ .../elasticsearch/xpack/sql/jdbc/EsType.java | 1 + .../xpack/sql/jdbc/TypeConverter.java | 2 +- .../xpack/sql/jdbc/TypeUtils.java | 1 + .../xpack/sql/qa/mixed_node/SqlSearchIT.java | 7 +++++ .../src/test/resources/all_field_types.json | 5 +++- .../xpack/sql/qa/FieldExtractorTestCase.java | 28 +++++++++++++++++++ .../xpack/sql/querydsl/agg/GroupByKey.java | 1 + .../xpack/sql/type/SqlDataTypes.java | 10 +++++++ .../logical/command/sys/SysTypesTests.java | 5 +++- .../sql/type/SqlDataTypeConverterTests.java | 19 +++++++++++++ 18 files changed, 150 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugin/eql/src/test/resources/mapping-version.json create mode 100644 x-pack/plugin/ql/src/test/resources/mapping-version.json diff --git a/docs/reference/sql/language/data-types.asciidoc b/docs/reference/sql/language/data-types.asciidoc index 948f7cb8c60ba..a2a89deb745e6 100644 --- a/docs/reference/sql/language/data-types.asciidoc +++ b/docs/reference/sql/language/data-types.asciidoc @@ -28,6 +28,7 @@ s|SQL precision | <> | binary | VARBINARY | 2,147,483,647 | <> | datetime | TIMESTAMP | 29 | <> | ip | VARCHAR | 39 +| <> | version | VARCHAR | 32,766 4+h| Complex types diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java index 5abd4dfee9f82..cf5c0f19471eb 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java @@ -363,6 +363,11 @@ public void testIP() { accept(idxr, "foo where ip_addr == 0"); } + public void testVersion() { + final IndexResolution idxr = loadIndexResolution("mapping-version.json"); + accept(idxr, "foo where version_number == \"2.1.4\""); + } + public void testJoin() { final IndexResolution idxr = loadIndexResolution("mapping-join.json"); accept(idxr, "foo where serial_event_id == 0"); diff --git a/x-pack/plugin/eql/src/test/resources/mapping-version.json b/x-pack/plugin/eql/src/test/resources/mapping-version.json new file mode 100644 index 0000000000000..3f0d1cf708c03 --- /dev/null +++ b/x-pack/plugin/eql/src/test/resources/mapping-version.json @@ -0,0 +1,17 @@ +{ + "properties" : { + "event" : { + "properties" : { + "category" : { + "type" : "keyword" + } + } + }, + "@timestamp" : { + "type" : "date" + }, + "version_number" : { + "type" : "version" + } + } +} diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java index c2319ab8bde9d..c88afe89eeb6a 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java @@ -34,6 +34,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.SHORT; import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.ql.type.DataTypes.isDateTime; import static org.elasticsearch.xpack.ql.type.DataTypes.isPrimitive; import static org.elasticsearch.xpack.ql.type.DataTypes.isString; @@ -170,6 +171,9 @@ public static Converter converterFor(DataType from, DataType to) { if (to == IP) { return conversionToIp(from); } + if (to == VERSION) { + return conversionToVersion(from); + } return null; } @@ -187,6 +191,13 @@ private static Converter conversionToIp(DataType from) { return null; } + private static Converter conversionToVersion(DataType from) { + if (isString(from)) { + return DefaultConverter.STRING_TO_VERSION; + } + return null; + } + private static Converter conversionToUnsignedLong(DataType from) { if (from.isRational()) { return DefaultConverter.RATIONAL_TO_UNSIGNED_LONG; @@ -537,6 +548,10 @@ public enum DefaultConverter implements Converter { throw new QlIllegalArgumentException("[" + o + "] is not a valid IPv4 or IPv6 address"); } return o; + }), + STRING_TO_VERSION(o -> { + // TODO validate...? + return o; }); public static final String NAME = "dtc-def"; diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypes.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypes.java index 4dc8bf046a90f..8741025c34b17 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypes.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypes.java @@ -44,6 +44,8 @@ public final class DataTypes { public static final DataType DATETIME = new DataType("DATETIME", "date", Long.BYTES, false, false, true); // ip public static final DataType IP = new DataType("ip", 45, false, false, true); + // version + public static final DataType VERSION = new DataType("version", Integer.MAX_VALUE, false, false, true); // binary public static final DataType BINARY = new DataType("binary", Integer.MAX_VALUE, false, false, true); // complex types @@ -68,6 +70,7 @@ public final class DataTypes { TEXT, DATETIME, IP, + VERSION, BINARY, OBJECT, NESTED diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java index 13a1182c1f7f6..3f9665c90421d 100644 --- a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java @@ -32,6 +32,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSUPPORTED; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.ql.type.DateUtils.asDateTime; import static org.elasticsearch.xpack.ql.util.NumericUtils.UNSIGNED_LONG_MAX; @@ -563,4 +564,22 @@ public void testIpToString() { Converter stringToIp = converterFor(KEYWORD, IP); assertEquals("10.0.0.1", ipToString.convert(stringToIp.convert(new Literal(s, "10.0.0.1", KEYWORD)))); } + + public void testStringToVersion() { + Converter conversion = converterFor(KEYWORD, VERSION); + assertNull(conversion.convert(null)); + assertEquals("2.1.4", conversion.convert("2.1.4")); + assertEquals("2.1.4-SNAPSHOT", conversion.convert("2.1.4-SNAPSHOT")); + } + + public void testVersionToString() { + Source s = new Source(Location.EMPTY, "2.1.4"); + Source s2 = new Source(Location.EMPTY, "2.1.4"); + Converter ipToString = converterFor(VERSION, KEYWORD); + assertEquals("2.1.4", ipToString.convert(new Literal(s, "2.1.4", VERSION))); + assertEquals("2.1.4-SNAPSHOT", ipToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); + Converter stringToIp = converterFor(KEYWORD, VERSION); + assertEquals("2.1.4", ipToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); + assertEquals("2.1.4-SNAPSHOT", ipToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); + } } diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/TypesTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/TypesTests.java index fa916cebd73fc..d8633532f02f1 100644 --- a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/TypesTests.java +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/TypesTests.java @@ -186,6 +186,13 @@ public void testIpField() { assertThat(dt.getDataType().typeName(), is("ip")); } + public void testVersionField() { + Map mapping = loadMapping("mapping-version.json"); + assertThat(mapping.size(), is(1)); + EsField dt = mapping.get("version_number"); + assertThat(dt.getDataType().typeName(), is("version")); + } + public void testConstantKeywordField() { Map mapping = loadMapping("mapping-constant-keyword.json"); assertThat(mapping.size(), is(1)); diff --git a/x-pack/plugin/ql/src/test/resources/mapping-version.json b/x-pack/plugin/ql/src/test/resources/mapping-version.json new file mode 100644 index 0000000000000..d258804445843 --- /dev/null +++ b/x-pack/plugin/ql/src/test/resources/mapping-version.json @@ -0,0 +1,7 @@ +{ + "properties" : { + "version_number" : { + "type" : "version" + } + } +} diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java index 6fd6bb7e83d6e..cb31cbb4956b8 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java @@ -32,6 +32,7 @@ public enum EsType implements SQLType { TIME(Types.TIME), DATETIME(Types.TIMESTAMP), IP(Types.VARCHAR), + VERSION(Types.VARCHAR), INTERVAL_YEAR(ExtraTypes.INTERVAL_YEAR), INTERVAL_MONTH(ExtraTypes.INTERVAL_MONTH), INTERVAL_YEAR_TO_MONTH(ExtraTypes.INTERVAL_YEAR_MONTH), diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeConverter.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeConverter.java index a0717cb53ada3..80645f963ea40 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeConverter.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeConverter.java @@ -255,8 +255,8 @@ static Object convert(Object v, EsType columnType, String typeString) throws SQL case GEO_POINT: case GEO_SHAPE: case SHAPE: - return v.toString(); case IP: + case VERSION: return v.toString(); default: throw new SQLException("Unexpected column type [" + typeString + "]"); diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeUtils.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeUtils.java index c3be465e42b7e..c8b7b954e2e0c 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeUtils.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/TypeUtils.java @@ -93,6 +93,7 @@ private TypeUtils() {} types.put(EsType.BINARY, byte[].class); types.put(EsType.DATETIME, Timestamp.class); types.put(EsType.IP, String.class); + types.put(EsType.VERSION, String.class); types.put(EsType.INTERVAL_YEAR, Period.class); types.put(EsType.INTERVAL_MONTH, Period.class); types.put(EsType.INTERVAL_YEAR_TO_MONTH, Period.class); diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java index 501c1bed2a446..f9e17968ba386 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java @@ -47,6 +47,7 @@ public class SqlSearchIT extends ESRestTestCase { * 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 Version VERSION_FIELD_QL_INTRODUCTION = Version.V_8_2_0; private static final String index = "test_sql_mixed_versions"; private static int numShards; private static int numReplicas = 1; @@ -181,6 +182,9 @@ private Map prepareTestData( columns.add(columnInfo("scaled_float_field", "scaled_float")); columns.add(columnInfo("boolean_field", "boolean")); columns.add(columnInfo("ip_field", "ip")); + if (bwcVersion.onOrAfter(VERSION_FIELD_QL_INTRODUCTION)) { + columns.add(columnInfo("version_field", "version")); + } columns.add(columnInfo("text_field", "text")); columns.add(columnInfo("keyword_field", "keyword")); columns.add(columnInfo("constant_keyword_field", "keyword")); @@ -212,6 +216,9 @@ private Map prepareTestData( builder.append("\"scaled_float_field\":" + fieldValues.computeIfAbsent("scaled_float_field", v -> 123.5d) + ","); builder.append("\"boolean_field\":" + fieldValues.computeIfAbsent("boolean_field", v -> randomBoolean()) + ","); builder.append("\"ip_field\":\"" + fieldValues.computeIfAbsent("ip_field", v -> "123.123.123.123") + "\","); + if (bwcVersion.onOrAfter(VERSION_FIELD_QL_INTRODUCTION)) { + columns.add(columnInfo("version_field", "2.11.4")); + } builder.append("\"text_field\": \"" + fieldValues.computeIfAbsent("text_field", v -> randomAlphaOfLength(5)) + "\","); builder.append("\"keyword_field\": \"" + fieldValues.computeIfAbsent("keyword_field", v -> randomAlphaOfLength(5)) + "\","); builder.append( diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json b/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json index 491be6f0f96b6..a5f4a00681103 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json @@ -29,6 +29,9 @@ }, "ip_field": { "type": "ip" + }, + "version_field": { + "type": "version" }, "text_field": { "type": "text" @@ -56,4 +59,4 @@ "shape_field": { "type": "shape" } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java index 9bd70ae6b51fa..8240d4cdd9ecd 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java @@ -400,6 +400,34 @@ public void testIpField() throws IOException { } } + /* + * "version_field": { + * "type": "version", + * } + */ + public void testVersionField() throws IOException { + String query = "SELECT version_field FROM test"; + String actualValue = "2.11.4"; + boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting + boolean enableSource = randomBoolean(); // enable _source at index level + + Map indexProps = Maps.newMapWithExpectedSize(1); + indexProps.put("_source", enableSource); + + Map> fieldProps = null; + createIndexWithFieldTypeAndProperties("version", fieldProps, explicitSourceSetting ? indexProps : null); + index("{\"version_field\":\"" + actualValue + "\"}"); + + if (explicitSourceSetting == false || enableSource) { + Map expected = new HashMap<>(); + expected.put("columns", asList(columnInfo("plain", "version_field", "version", JDBCType.VARCHAR, Integer.MAX_VALUE))); + expected.put("rows", singletonList(singletonList(actualValue))); + assertResponse(expected, runSql(query)); + } else { + expectSourceDisabledError(query); + } + } + /* * "geo_point_field": { * "type": "geo_point", diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java index 3754082a7fcd9..b72335f9146de 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java @@ -62,6 +62,7 @@ public final CompositeValuesSourceBuilder asValueSource() { } else if (script.outputType() == IP) { builder.userValuetypeHint(ValueType.IP); } + // TODO hint for VERSION } // field based else { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java index 08467e33af91e..25eb1c09a5aaf 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java @@ -47,6 +47,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSUPPORTED; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.ql.type.DataTypes.isDateTime; import static org.elasticsearch.xpack.ql.util.CollectionUtils.mapSize; @@ -362,6 +363,9 @@ public static SQLType sqlType(DataType dataType) { if (dataType == IP) { return JDBCType.VARCHAR; } + if (dataType == VERSION) { + return JDBCType.VARCHAR; + } if (dataType == BINARY) { return JDBCType.BINARY; } @@ -488,6 +492,9 @@ public static int defaultPrecision(DataType dataType) { if (dataType == IP) { return dataType.size(); } + if (dataType == VERSION) { + return dataType.size(); + } if (dataType == BINARY) { return dataType.size(); } @@ -608,6 +615,9 @@ public static int displaySize(DataType dataType) { if (dataType == IP) { return dataType.size(); } + if (dataType == VERSION) { + return dataType.size(); + } if (dataType == BINARY) { return dataType.size(); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java index 44ee3741a2d1d..75c57985253e4 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java @@ -94,6 +94,7 @@ public void testSysTypes() { "IP", "KEYWORD", "TEXT", + "VERSION", "BOOLEAN", "DATE", "TIME", @@ -194,12 +195,14 @@ public void testSysTypesMultipleMatches() { cmd.v1().execute(cmd.v2(), wrap(p -> { SchemaRowSet r = (SchemaRowSet) p.rowSet(); - assertEquals(3, r.size()); + assertEquals(4, r.size()); assertEquals("IP", r.column(0)); assertTrue(r.advanceRow()); assertEquals("KEYWORD", r.column(0)); assertTrue(r.advanceRow()); assertEquals("TEXT", r.column(0)); + assertTrue(r.advanceRow()); + assertEquals("VERSION", r.column(0)); }, ex -> fail(ex.getMessage()))); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java index 490fb935308d2..f767938c065c0 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java @@ -36,6 +36,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSUPPORTED; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.sql.type.SqlDataTypeConverter.commonType; import static org.elasticsearch.xpack.sql.type.SqlDataTypeConverter.converterFor; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.DATE; @@ -774,6 +775,24 @@ public void testIpToString() { assertEquals("10.0.0.1", ipToString.convert(stringToIp.convert(new Literal(s, "10.0.0.1", KEYWORD)))); } + public void testStringToVersion() { + Converter conversion = converterFor(KEYWORD, VERSION); + assertNull(conversion.convert(null)); + assertEquals("2.1.4", conversion.convert("2.1.4")); + assertEquals("2.1.4-SNAPSHOT", conversion.convert("2.1.4-SNAPSHOT")); + } + + public void testVersionToString() { + Source s = new Source(Location.EMPTY, "2.1.4"); + Source s2 = new Source(Location.EMPTY, "2.1.4-SNAPSHOT"); + Converter ipToString = converterFor(VERSION, KEYWORD); + assertEquals("2.1.4", ipToString.convert(new Literal(s, "2.1.4", VERSION))); + assertEquals("2.1.4-SNAPSHOT", ipToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); + Converter stringToIp = converterFor(KEYWORD, VERSION); + assertEquals("2.1.4", ipToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); + assertEquals("2.1.4-SNAPSHOT", ipToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); + } + private DataType randomInterval() { return randomFrom(SqlDataTypes.types().stream().filter(SqlDataTypes::isInterval).collect(toList())); } From f5c79f851ae26cf8c45790eb650c22bf8acbf167 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 30 Mar 2022 17:44:42 +0200 Subject: [PATCH 02/20] Update docs/changelog/85502.yaml --- docs/changelog/85502.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/85502.yaml diff --git a/docs/changelog/85502.yaml b/docs/changelog/85502.yaml new file mode 100644 index 0000000000000..b812505c0a9bc --- /dev/null +++ b/docs/changelog/85502.yaml @@ -0,0 +1,6 @@ +pr: 85502 +summary: Add support for VERSION field type in SQL and EQL +area: Query Languages +type: enhancement +issues: + - 83375 From dbe5c67168244b51412f20f1fd60554103263d52 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 30 Mar 2022 18:49:04 +0200 Subject: [PATCH 03/20] Add SQL csv-spec tests for VERSION fields --- .../xpack/sql/qa/jdbc/DataLoader.java | 18 +++++ .../sql/qa/server/src/main/resources/apps.csv | 7 ++ .../src/main/resources/version.csv-spec | 69 +++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 x-pack/plugin/sql/qa/server/src/main/resources/apps.csv create mode 100644 x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java index eac026689f72f..a4abc1d64782e 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java @@ -65,6 +65,7 @@ public static void createEmptyIndex(RestClient client, String index) throws Exce protected static void loadEmpDatasetIntoEs(RestClient client) throws Exception { loadEmpDatasetIntoEs(client, "test_emp", "employees"); loadEmpDatasetWithExtraIntoEs(client, "test_emp_copy", "employees"); + loadAppsDatasetIntoEs(client, "apps", "apps"); loadLogsDatasetIntoEs(client, "logs", "logs"); loadLogNanosDatasetIntoEs(client, "logs_nanos", "logs_nanos"); loadLogUnsignedLongIntoEs(client, "logs_unsigned_long", "logs_unsigned_long"); @@ -274,6 +275,23 @@ private static void loadEmpDatasetIntoEs(RestClient client, String index, String client.performRequest(request); } + protected static void loadAppsDatasetIntoEs(RestClient client, String index, String filename) throws Exception { + XContentBuilder createIndex = JsonXContent.contentBuilder().startObject(); + createIndex.startObject("mappings"); + { + createIndex.startObject("properties"); + { + createIndex.startObject("id").field("type", "integer").endObject(); + createIndex.startObject("version").field("type", "version").endObject(); + createIndex.startObject("name").field("type", "text").endObject(); + } + createIndex.endObject(); + } + createIndex.endObject(); + + loadDatasetIntoEs(client, index, filename, createIndex); + } + protected static void loadLogsDatasetIntoEs(RestClient client, String index, String filename) throws Exception { XContentBuilder createIndex = JsonXContent.contentBuilder().startObject(); createIndex.startObject("mappings"); diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv new file mode 100644 index 0000000000000..625e45a54b806 --- /dev/null +++ b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv @@ -0,0 +1,7 @@ +id,version,name +1,1.0.0,aaaaa +2,2.1.14,bbbbb +3,2.3.4,ccccc +4,2.12.0,ddddd +5,1.11.0,eeeee +6,5.2.9,fffff diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec new file mode 100644 index 0000000000000..90d9089cb96c8 --- /dev/null +++ b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec @@ -0,0 +1,69 @@ +// To mute tests follow example in file: example.csv-spec + +// +// Tests for VERSION fields +// + +selectAll +SELECT * FROM apps ORDER BY id; + + id | name | version +------+-----------+--------- +1 |aaaaa |1.0.0 +2 |bbbbb |2.1.14 +3 |ccccc |2.3.4 +4 |ddddd |2.12.0 +5 |eeeee |1.11.0 +6 |fffff |5.2.9 +; + +filterByVersion +SELECT * FROM apps WHERE version = '2.12.0'; + + id | name | version +------+-----------+--------- +4 |ddddd |2.12.0 +; + +projectionVersion +SELECT version FROM apps WHERE id = 3; + + version +--------- +2.3.4 +; + + +versionRange1 +SELECT * FROM apps WHERE version > '2.2' ORDER BY id; + + id | name | version +------+-----------+--------- +3 |ccccc |2.3.4 +4 |ddddd |2.12.0 +6 |fffff |5.2.9 +; + +versionRange2 +SELECT * FROM apps WHERE version >= '2.3.4' ORDER BY id; + + id | name | version +------+-----------+--------- +3 |ccccc |2.3.4 +4 |ddddd |2.12.0 +6 |fffff |5.2.9 +; + + +orderByVersion +SELECT * FROM apps ORDER BY version; + + id | name | version +------+-----------+--------- +1 |aaaaa |1.0.0 +5 |eeeee |1.11.0 +2 |bbbbb |2.1.14 +3 |ccccc |2.3.4 +4 |ddddd |2.12.0 +6 |fffff |5.2.9 +; From 965b6c1866eed5f589a44546a1e5038a0e93bd24 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Thu, 31 Mar 2022 09:08:48 +0200 Subject: [PATCH 04/20] Fix test cases --- .../src/main/resources/command.csv-spec | 2 + .../single-node-only/command-sys.csv-spec | 159 +++++++++--------- 2 files changed, 83 insertions(+), 78 deletions(-) diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec index c3f3af296adb3..78097f8629493 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/command.csv-spec @@ -232,6 +232,7 @@ showTables SHOW TABLES; catalog | name | type | kind +integTest |apps |TABLE |INDEX integTest |empty_mapping |TABLE |INDEX integTest |logs |TABLE |INDEX integTest |logs_nanos |TABLE |INDEX @@ -283,6 +284,7 @@ showTablesLocalCatalog SHOW TABLES CATALOG 'integTest'; catalog | name | type | kind +integTest |apps |TABLE |INDEX integTest |empty_mapping |TABLE |INDEX integTest |logs |TABLE |INDEX integTest |logs_nanos |TABLE |INDEX diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec index 006b1ffa3e37f..af3a0be9416af 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec @@ -102,82 +102,85 @@ SYS COLUMNS TABLE LIKE '%'; TABLE_CAT:s | TABLE_SCHEM:s| TABLE_NAME:s | COLUMN_NAME:s | DATA_TYPE:i | TYPE_NAME:s | COLUMN_SIZE:i|BUFFER_LENGTH:i|DECIMAL_DIGITS:i|NUM_PREC_RADIX:i | NULLABLE:i| REMARKS:s | COLUMN_DEF:s |SQL_DATA_TYPE:i|SQL_DATETIME_SUB:i|CHAR_OCTET_LENGTH:i|ORDINAL_POSITION:i|IS_NULLABLE:s|SCOPE_CATALOG:s|SCOPE_SCHEMA:s|SCOPE_TABLE:s|SOURCE_DATA_TYPE:sh|IS_AUTOINCREMENT:s|IS_GENERATEDCOLUMN:s -----------------+---------------+-------------------+------------------+---------------+----------------+---------------+---------------+----------------+---------------+---------------+---------------+---------------+---------------+----------------+-----------------+----------------+---------------+---------------+---------------+---------------+----------------+----------------+------------------ -integTest |null |logs |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO -integTest |null |logs |bytes_in |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |2 |YES |null |null |null |null |NO |NO -integTest |null |logs |bytes_out |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO -integTest |null |logs |client_ip |12 |IP |45 |45 |null |null |1 |null |null |12 |0 |null |4 |YES |null |null |null |null |NO |NO -integTest |null |logs |client_port |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |5 |YES |null |null |null |null |NO |NO -integTest |null |logs |dest_ip |12 |IP |45 |45 |null |null |1 |null |null |12 |0 |null |6 |YES |null |null |null |null |NO |NO -integTest |null |logs |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |7 |YES |null |null |null |null |NO |NO -integTest |null |logs |status |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |8 |YES |null |null |null |null |NO |NO -integTest |null |logs_nanos |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO -integTest |null |logs_nanos |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |2 |YES |null |null |null |null |NO |NO -integTest |null |logs_nanos |status |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |3 |YES |null |null |null |null |NO |NO -integTest |null |logs_unsigned_long |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO -integTest |null |logs_unsigned_long |bytes_in |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |2 |YES |null |null |null |null |NO |NO -integTest |null |logs_unsigned_long |bytes_out |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |3 |YES |null |null |null |null |NO |NO -integTest |null |logs_unsigned_long |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |4 |YES |null |null |null |null |NO |NO -integTest |null |logs_unsigned_long |status |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |5 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |extra.info.gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |extra_gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |7 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |extra_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |8 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |12 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |13 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |14 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |15 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |16 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |null_constant |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |17 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |18 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |salary_ul |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |19 |YES |null |null |null |null |NO |NO -integTest |null |test_alias |wildcard_name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |20 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |extra.info.gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |extra_gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |7 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |extra_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |8 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |12 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |13 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |14 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |15 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |16 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |null_constant |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |17 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |18 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |salary_ul |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |19 |YES |null |null |null |null |NO |NO -integTest |null |test_alias_emp |wildcard_name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |20 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |4 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |5 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |7 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |8 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO -integTest |null |test_emp |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |12 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |extra.info.gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |extra_gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |7 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |extra_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |8 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |12 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |13 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |14 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |15 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |16 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |null_constant |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |17 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |18 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |salary_ul |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |19 |YES |null |null |null |null |NO |NO -integTest |null |test_emp_copy |wildcard_name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |20 |YES |null |null |null |null |NO |NO +integTest |null |apps |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |apps |name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |2 |YES |null |null |null |null |NO |NO +integTest |null |apps |version |12 |VERSION |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |null |3 |YES |null |null |null |null |NO |NO +integTest |null |logs |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |logs |bytes_in |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |2 |YES |null |null |null |null |NO |NO +integTest |null |logs |bytes_out |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO +integTest |null |logs |client_ip |12 |IP |45 |45 |null |null |1 |null |null |12 |0 |null |4 |YES |null |null |null |null |NO |NO +integTest |null |logs |client_port |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |5 |YES |null |null |null |null |NO |NO +integTest |null |logs |dest_ip |12 |IP |45 |45 |null |null |1 |null |null |12 |0 |null |6 |YES |null |null |null |null |NO |NO +integTest |null |logs |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |7 |YES |null |null |null |null |NO |NO +integTest |null |logs |status |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |8 |YES |null |null |null |null |NO |NO +integTest |null |logs_nanos |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |logs_nanos |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |2 |YES |null |null |null |null |NO |NO +integTest |null |logs_nanos |status |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |3 |YES |null |null |null |null |NO |NO +integTest |null |logs_unsigned_long |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |logs_unsigned_long |bytes_in |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |2 |YES |null |null |null |null |NO |NO +integTest |null |logs_unsigned_long |bytes_out |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |3 |YES |null |null |null |null |NO |NO +integTest |null |logs_unsigned_long |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |4 |YES |null |null |null |null |NO |NO +integTest |null |logs_unsigned_long |status |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |5 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |extra.info.gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |extra_gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |7 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |extra_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |8 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |12 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |13 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |14 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |15 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |16 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |null_constant |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |17 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |18 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |salary_ul |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |19 |YES |null |null |null |null |NO |NO +integTest |null |test_alias |wildcard_name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |20 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |extra.info.gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |extra_gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |7 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |extra_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |8 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |12 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |13 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |14 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |15 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |16 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |null_constant |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |17 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |18 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |salary_ul |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |19 |YES |null |null |null |null |NO |NO +integTest |null |test_alias_emp |wildcard_name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |20 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |4 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |5 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |7 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |8 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO +integTest |null |test_emp |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |12 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |birth_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |emp_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |extra.info.gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |6 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |extra_gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |7 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |extra_no |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |8 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |first_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |9 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |first_name.keyword|12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |10 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |gender |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |11 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |hire_date |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |12 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |languages |-6 |BYTE |5 |1 |null |10 |1 |null |null |-6 |0 |null |13 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |last_name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |14 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |last_name.keyword |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |15 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |16 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |null_constant |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |17 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |salary |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |18 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |salary_ul |2 |UNSIGNED_LONG |20 |8 |null |10 |1 |null |null |2 |0 |null |19 |YES |null |null |null |null |NO |NO +integTest |null |test_emp_copy |wildcard_name |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |20 |YES |null |null |null |null |NO |NO ; From 288b5f862b9fc9885a6e2f80c5de8e75ead2529e Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Thu, 31 Mar 2022 09:15:51 +0200 Subject: [PATCH 05/20] Fix test to consider v 8.3 as version of introduction of the feature --- .../org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java index f9e17968ba386..c1b15ff318269 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java @@ -47,7 +47,7 @@ public class SqlSearchIT extends ESRestTestCase { * 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 Version VERSION_FIELD_QL_INTRODUCTION = Version.V_8_2_0; + private static final Version VERSION_FIELD_QL_INTRODUCTION = Version.V_8_3_0; private static final String index = "test_sql_mixed_versions"; private static int numShards; private static int numReplicas = 1; From 9f8cb6e1183aaaabde38f49bbab2bc7cfcdaa485 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Thu, 31 Mar 2022 10:02:25 +0200 Subject: [PATCH 06/20] Fix frozen csv test cases --- .../plugin/sql/qa/server/src/main/resources/slow/frozen.csv-spec | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/slow/frozen.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/slow/frozen.csv-spec index b68d6e49443f1..b9e7f93e662fe 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/slow/frozen.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/slow/frozen.csv-spec @@ -8,6 +8,7 @@ showTables SHOW TABLES INCLUDE FROZEN; catalog | name | type | kind +integTest |apps |TABLE |INDEX integTest |empty_mapping |TABLE |INDEX integTest |frozen_emp |TABLE |FROZEN INDEX integTest |logs |TABLE |INDEX From dfe40138e7f7c9d88c9d04ac76a371b143e64206 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Thu, 31 Mar 2022 12:35:43 +0200 Subject: [PATCH 07/20] Fix further tests --- .../multi-cluster-command-sys.csv-spec | 5 +++++ .../multi-cluster-command.csv-spec | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec index 843b7ea3aa056..fb61f270515a3 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec @@ -121,6 +121,9 @@ SYS COLUMNS CATALOG 'my_remote_cluster' TABLE LIKE '%'; TABLE_CAT:s | TABLE_SCHEM:s| TABLE_NAME:s | COLUMN_NAME:s | DATA_TYPE:i | TYPE_NAME:s | COLUMN_SIZE:i| BUFFER_LENGTH:i|DECIMAL_DIGITS:i|NUM_PREC_RADIX:i | NULLABLE:i| REMARKS:s | COLUMN_DEF:s |SQL_DATA_TYPE:i|SQL_DATETIME_SUB:i|CHAR_OCTET_LENGTH:i|ORDINAL_POSITION:i|IS_NULLABLE:s|SCOPE_CATALOG:s|SCOPE_SCHEMA:s|SCOPE_TABLE:s|SOURCE_DATA_TYPE:sh|IS_AUTOINCREMENT:s|IS_GENERATEDCOLUMN:s -----------------+---------------+-------------------+------------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+----------------+-----------------+----------------+---------------+---------------+---------------+---------------+----------------+----------------+------------------ +my_remote_cluster|null |apps |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |1 |YES |null |null |null |null |NO |NO +my_remote_cluster|null |apps |name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |2 |YES |null |null |null |null |NO |NO +my_remote_cluster|null |apps |version |12 |VERSION |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |null |3 |YES |null |null |null |null |NO |NO my_remote_cluster|null |logs |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO my_remote_cluster|null |logs |bytes_in |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |2 |YES |null |null |null |null |NO |NO my_remote_cluster|null |logs |bytes_out |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO @@ -172,6 +175,7 @@ SYS TABLES CATALOG LIKE 'my_remote_cluster'; TABLE_CAT:s | TABLE_SCHEM:s| TABLE_NAME:s | TABLE_TYPE:s | REMARKS:s | TYPE_CAT:s | TYPE_SCHEM:S | TYPE_NAME:s |SELF_REFERENCING_COL_NAME:s|REF_GENERATION:s -----------------+---------------+-------------------+---------------+---------------+---------------+---------------+---------------+-------------------------+--------------- +my_remote_cluster|null |apps |TABLE | |null |null |null |null |null my_remote_cluster|null |empty_mapping |TABLE | |null |null |null |null |null my_remote_cluster|null |logs |TABLE | |null |null |null |null |null my_remote_cluster|null |logs_nanos |TABLE | |null |null |null |null |null @@ -201,6 +205,7 @@ SYS TABLES CATALOG LIKE 'my_remote_cluster' "" TYPE ''; TABLE_CAT:s | TABLE_SCHEM:s| TABLE_NAME:s | TABLE_TYPE:s | REMARKS:s | TYPE_CAT:s | TYPE_SCHEM:S | TYPE_NAME:s |SELF_REFERENCING_COL_NAME:s|REF_GENERATION:s -----------------+---------------+-------------------+---------------+---------------+---------------+---------------+---------------+-------------------------+--------------- +my_remote_cluster|null |apps |TABLE | |null |null |null |null |null my_remote_cluster|null |empty_mapping |TABLE | |null |null |null |null |null my_remote_cluster|null |logs |TABLE | |null |null |null |null |null my_remote_cluster|null |logs_nanos |TABLE | |null |null |null |null |null diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command.csv-spec index 2d77fd476c1a2..e8038857a08e1 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command.csv-spec @@ -23,6 +23,7 @@ SHOW TABLES CATALOG 'my_remote_cluster'; catalog | name | type | kind -----------------+-------------------+---------------+--------------- +my_remote_cluster|apps |TABLE |INDEX my_remote_cluster|empty_mapping |TABLE |INDEX my_remote_cluster|logs |TABLE |INDEX my_remote_cluster|logs_nanos |TABLE |INDEX @@ -36,6 +37,7 @@ SHOW TABLES CATALOG LIKE 'my_remote_%'; catalog | name | type | kind -----------------+-------------------+---------------+--------------- +my_remote_cluster|apps |TABLE |INDEX my_remote_cluster|empty_mapping |TABLE |INDEX my_remote_cluster|logs |TABLE |INDEX my_remote_cluster|logs_nanos |TABLE |INDEX @@ -49,6 +51,7 @@ SHOW TABLES CATALOG 'my_remote_cluster' INCLUDE FROZEN; catalog | name | type | kind -----------------+-------------------+---------------+--------------- +my_remote_cluster|apps |TABLE |INDEX my_remote_cluster|empty_mapping |TABLE |INDEX my_remote_cluster|frozen_emp |TABLE |INDEX my_remote_cluster|logs |TABLE |INDEX @@ -119,6 +122,7 @@ SHOW TABLES CATALOG '*'; catalog | name | type | kind -----------------+-------------------+---------------+--------------- +my_remote_cluster|apps |TABLE |INDEX my_remote_cluster|empty_mapping |TABLE |INDEX my_remote_cluster|logs |TABLE |INDEX my_remote_cluster|logs_nanos |TABLE |INDEX From 2f16de63a5ec9a780accff05f3023b2a4b456e8e Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Fri, 1 Apr 2022 13:05:03 +0200 Subject: [PATCH 08/20] Implement comparison of Version fields (folding) --- .../xpack/versionfield/VersionEncoder.java | 4 +- x-pack/plugin/ql/build.gradle | 1 + .../operator/comparison/Comparisons.java | 10 ++ .../xpack/ql/type/DataTypeConverter.java | 5 +- .../xpack/ql/type/VersionFieldValue.java | 53 +++++++ .../xpack/ql/type/VersionFieldValueTests.java | 76 ++++++++++ .../src/test/resources/all_field_types.json | 2 +- .../sql/qa/server/src/main/resources/apps.csv | 8 +- .../src/main/resources/version.csv-spec | 136 ++++++++++++++++-- .../xpack/sql/action/SqlQueryResponse.java | 5 +- 10 files changed, 281 insertions(+), 19 deletions(-) create mode 100644 x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java create mode 100644 x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java diff --git a/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java b/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java index 56558ac3b608f..cffbd04ef1da5 100644 --- a/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java +++ b/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java @@ -35,7 +35,7 @@ * lexically in ASCII sort order. Numeric identifiers always have lower precedence than non-numeric identifiers. * */ -class VersionEncoder { +public class VersionEncoder { public static final byte NUMERIC_MARKER_BYTE = (byte) 0x01; public static final byte PRERELEASE_SEPARATOR_BYTE = (byte) 0x02; @@ -175,7 +175,7 @@ static boolean legalVersionString(VersionParts versionParts) { return legalMainVersion && legalPreRelease && legalBuildSuffix; } - static class EncodedVersion { + public static class EncodedVersion { public final boolean isLegal; public final boolean isPreRelease; diff --git a/x-pack/plugin/ql/build.gradle b/x-pack/plugin/ql/build.gradle index 81ef6a52be28e..153a8bbafe4bd 100644 --- a/x-pack/plugin/ql/build.gradle +++ b/x-pack/plugin/ql/build.gradle @@ -16,6 +16,7 @@ archivesBaseName = 'x-pack-ql' dependencies { api "org.antlr:antlr4-runtime:${antlrVersion}" + api project(path: xpackModule('mapper-version')) compileOnly project(path: xpackModule('core')) testApi(project(xpackModule('ql:test-fixtures'))) { exclude group: 'org.elasticsearch.plugin', module: 'ql' diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java index cbe032c253ad8..f2ac5af3c049d 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java @@ -6,6 +6,8 @@ */ package org.elasticsearch.xpack.ql.expression.predicate.operator.comparison; +import org.elasticsearch.xpack.ql.type.VersionFieldValue; + import java.math.BigInteger; import java.util.Set; @@ -75,6 +77,14 @@ static Integer compare(Object l, Object r) { return compare((Number) l, (Number) r); } + // automatic conversion for versions + if (l instanceof VersionFieldValue && r instanceof String) { + return ((VersionFieldValue) l).compareTo(new VersionFieldValue((String) r)); + } + if (l instanceof String && r instanceof VersionFieldValue) { + return new VersionFieldValue((String) l).compareTo((VersionFieldValue) r); + } + if (l instanceof Comparable && r instanceof Comparable) { try { return Integer.valueOf(((Comparable) l).compareTo(r)); diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java index c88afe89eeb6a..57278432b0a5a 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java @@ -549,10 +549,7 @@ public enum DefaultConverter implements Converter { } return o; }), - STRING_TO_VERSION(o -> { - // TODO validate...? - return o; - }); + STRING_TO_VERSION(o -> new VersionFieldValue(o.toString())); public static final String NAME = "dtc-def"; diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java new file mode 100644 index 0000000000000..9fd8e9516a9ca --- /dev/null +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ql.type; + +import org.elasticsearch.xpack.versionfield.VersionEncoder; + +import java.util.Objects; + +public final class VersionFieldValue implements Comparable { + private final String stringValue; + private final VersionEncoder.EncodedVersion encoded; + + public VersionFieldValue(String str) { + if (str == null) { + throw new IllegalArgumentException("Invalid version field value: null"); + } + stringValue = str; + encoded = VersionEncoder.encodeVersion(str); + } + + @Override + public int compareTo(VersionFieldValue o) { + return this.encoded.bytesRef.compareTo(o.encoded.bytesRef); + } + + @Override + public String toString() { + return stringValue; + } + + public boolean isValid() { + return encoded.isLegal; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + VersionFieldValue that = (VersionFieldValue) o; + return Objects.equals(stringValue, that.stringValue); + } + + @Override + public int hashCode() { + return Objects.hash(stringValue); + } + +} diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java new file mode 100644 index 0000000000000..4a837bb7daa7f --- /dev/null +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java @@ -0,0 +1,76 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ql.type; + +import org.elasticsearch.test.ESTestCase; + +public class VersionFieldValueTests extends ESTestCase { + + public void testInvalidInput() { + expectThrows(IllegalArgumentException.class, () -> new VersionFieldValue(null)); + } + + public void testValid() { + assertTrue(new VersionFieldValue("1").isValid()); + assertTrue(new VersionFieldValue("1.0").isValid()); + assertTrue(new VersionFieldValue("1.0.0").isValid()); + assertTrue(new VersionFieldValue("1.0.0.2").isValid()); + assertTrue( + new VersionFieldValue( + "" + + randomIntBetween(0, Integer.MAX_VALUE) + + "." + + randomIntBetween(0, Integer.MAX_VALUE) + + "." + + randomIntBetween(0, Integer.MAX_VALUE) + ).isValid() + ); + assertTrue(new VersionFieldValue("1.4.0-rc1").isValid()); + assertTrue(new VersionFieldValue("1.0.0.0-SNAPSHOT").isValid()); + + } + + public void testInvalid() { + assertFalse(new VersionFieldValue("-SNAPSHOT").isValid()); + assertFalse(new VersionFieldValue("1.foo.0-SNAPSHOT.1").isValid()); + assertFalse(new VersionFieldValue("rc1").isValid()); + assertFalse(new VersionFieldValue("bad").isValid()); + } + + public void testCompare() { + assertEquals(new VersionFieldValue("1.2.4"), new VersionFieldValue("1.2.4")); + + // left < right + String[][] comparisons = { + { "1.0.0", "1.1.0" }, + { "1.0.0", "1.0.1" }, + { "1.0.0", "2.0.0" }, + { "1.0.0-SNAPSHOT", "1.0.0" }, + { "1.0.0", "1.0.1-SNAPSHOT" }, + { "1.2.0", "1.11.0" }, + { "1.1.2", "1.1.11" }, + { "1.0.0-alpha1", "1.0.0-alpha2" }, + { "1.0.0-alpha", "1.0.0-beta" }, + { "1.0.0-beta", "1.0.0-rc1" }, + { "1000.0.0", "bad" }, + { "1000.0.0", "SNAPSHOT" }, + { "bar", "foo" } }; + for (int i = 0; i < comparisons.length; i++) { + String[] item = comparisons[i]; + assertTrue( + "Wrong version comparison " + item[0] + " < " + item[1], + new VersionFieldValue(item[0]).compareTo(new VersionFieldValue(item[1])) < 0 + ); + assertTrue( + "Wrong version comparison " + item[1] + " > " + item[0], + new VersionFieldValue(item[1]).compareTo(new VersionFieldValue(item[0])) > 0 + ); + } + + } +} diff --git a/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json b/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json index a5f4a00681103..3ac55d06e4d4e 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json +++ b/x-pack/plugin/sql/qa/mixed-node/src/test/resources/all_field_types.json @@ -30,7 +30,7 @@ "ip_field": { "type": "ip" }, - "version_field": { + "version_field": { "type": "version" }, "text_field": { diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv index 625e45a54b806..62c3c1e17d7e6 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv +++ b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv @@ -1,7 +1,11 @@ id,version,name -1,1.0.0,aaaaa -2,2.1.14,bbbbb +1,1,aaaaa +2,2.1,bbbbb 3,2.3.4,ccccc 4,2.12.0,ddddd 5,1.11.0,eeeee 6,5.2.9,fffff +7,5.2.9-SNAPSHOT,ggggg +8,1.2.3.4,hhhhh +9,bad,iiiii +10,5.2.9,jjjjj diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec index 90d9089cb96c8..54989a303f798 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec @@ -7,14 +7,19 @@ selectAll SELECT * FROM apps ORDER BY id; - id | name | version + id:i | name:s | version:s ------+-----------+--------- -1 |aaaaa |1.0.0 -2 |bbbbb |2.1.14 +1 |aaaaa |1 +2 |bbbbb |2.1 3 |ccccc |2.3.4 4 |ddddd |2.12.0 5 |eeeee |1.11.0 6 |fffff |5.2.9 +7 |ggggg |5.2.9-SNAPSHOT +8 |hhhhh |1.2.3.4 +9 |iiiii |bad +10 |jjjjj |5.2.9 + ; filterByVersion @@ -42,28 +47,141 @@ SELECT * FROM apps WHERE version > '2.2' ORDER BY id; 3 |ccccc |2.3.4 4 |ddddd |2.12.0 6 |fffff |5.2.9 +7 |ggggg |5.2.9-SNAPSHOT +9 |iiiii |bad +10 |jjjjj |5.2.9 + ; versionRange2 SELECT * FROM apps WHERE version >= '2.3.4' ORDER BY id; id | name | version -------+-----------+--------- +------+-----------+--------------- 3 |ccccc |2.3.4 4 |ddddd |2.12.0 6 |fffff |5.2.9 +7 |ggggg |5.2.9-SNAPSHOT +9 |iiiii |bad +10 |jjjjj |5.2.9 + ; orderByVersion -SELECT * FROM apps ORDER BY version; +SELECT * FROM apps ORDER BY version, id; - id | name | version -------+-----------+--------- -1 |aaaaa |1.0.0 + id:i | name:s | version:s +------+-----------+--------------- +1 |aaaaa |1 +8 |hhhhh |1.2.3.4 5 |eeeee |1.11.0 -2 |bbbbb |2.1.14 +2 |bbbbb |2.1 3 |ccccc |2.3.4 4 |ddddd |2.12.0 +7 |ggggg |5.2.9-SNAPSHOT 6 |fffff |5.2.9 +10 |jjjjj |5.2.9 +9 |iiiii |bad + +; + +groupByVersion +SELECT max(id) as id, version FROM apps GROUP BY version ORDER BY version; + + id:i | version:s +------+----------- +1 |1 +8 |1.2.3.4 +5 |1.11.0 +2 |2.1 +3 |2.3.4 +4 |2.12.0 +7 |5.2.9-SNAPSHOT +10 |5.2.9 +9 |bad + +; + +groupByVersionScript +SELECT max(id) as id FROM apps GROUP BY CONCAT('1.', CAST(version AS TEXT))::version ORDER BY id; + + id:i +------ +1 +2 +3 +4 +5 +7 +8 +9 +10 +; + +castToVersion +SELECT id, name, CONCAT('1.', CAST(version AS TEXT))::version version FROM apps ORDER BY id; + + id:i | name:s | version:s +------+-----------+--------------- +1 |aaaaa |1.1 +2 |bbbbb |1.2.1 +3 |ccccc |1.2.3.4 +4 |ddddd |1.2.12.0 +5 |eeeee |1.1.11.0 +6 |fffff |1.5.2.9 +7 |ggggg |1.5.2.9-SNAPSHOT +8 |hhhhh |1.1.2.3.4 +9 |iiiii |1.bad +10 |jjjjj |1.5.2.9 + +; + + +castConstantToVersion +SELECT '1.2.3'::version as v; + + v +------ +1.2.3 + +; + + +compareVersions +SELECT '1.2.3'::version < '1.11.4'::version as v; + + v +------ +true + +; + + +filterByVersionConstant +SELECT * FROM apps WHERE '1.2.0'::version < '1.11.0'::version ORDER BY id LIMIT 1; + + id:i | name:s | version:s +------+-----------+--------- +1 |aaaaa |1 + +; + +filterByVersionConstant2 +SELECT * FROM apps WHERE '1.2.0'::version < '1.11.0' ORDER BY id LIMIT 1; + + id:i | name:s | version:s +------+-----------+--------- +1 |aaaaa |1 + +; + + +filterByVersionConstant3 +SELECT * FROM apps WHERE '1.2.0' < '1.11.0'::version ORDER BY id LIMIT 1; + + id:i | name:s | version:s +------+-----------+--------- +1 |aaaaa |1 + ; diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java index 2f52d5aefff13..3ff737007c954 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java @@ -17,6 +17,7 @@ import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xpack.ql.async.QlStatusResponse; +import org.elasticsearch.xpack.ql.type.VersionFieldValue; import org.elasticsearch.xpack.sql.proto.ColumnInfo; import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.SqlVersion; @@ -274,7 +275,9 @@ private static XContentBuilder toXContent(ColumnInfo info, XContentBuilder build * Serializes the provided value in SQL-compatible way based on the client mode */ public static XContentBuilder value(XContentBuilder builder, Mode mode, SqlVersion sqlVersion, Object value) throws IOException { - if (value instanceof ZonedDateTime zdt) { + if (value instanceof VersionFieldValue) { + builder.value(value.toString()); + } else if (value instanceof ZonedDateTime zdt) { // use the ISO format if (mode == JDBC && isClientCompatible(SqlVersion.fromId(CURRENT.id), sqlVersion)) { builder.value(StringUtils.toString(zdt, sqlVersion)); From 85697ad9f84d8773fd03bc649a125b2b80eb366a Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Tue, 12 Apr 2022 12:35:11 +0200 Subject: [PATCH 09/20] Manage compatibility and add new test cases --- .../common/src/main/resources/data/extra.data | 4 ++ .../src/main/resources/data/extra.mapping | 3 + .../common/src/main/resources/test_extra.toml | 48 +++++++++++-- .../xpack/eql/analysis/VerifierTests.java | 1 + .../src/test/resources/mapping-default.json | 3 + .../ql/index/VersionCompatibilityChecks.java | 16 +++++ .../ql/type/DataTypeConversionTests.java | 14 ++-- .../mapping-multi-field-variation.json | 3 +- .../xpack/sql/qa/jdbc/JdbcTestUtils.java | 6 ++ .../xpack/sql/qa/jdbc/ResultSetTestCase.java | 69 +++++++++++++++++++ .../xpack/sql/qa/SqlProtocolTestCase.java | 5 ++ .../logical/command/ShowColumnsTests.java | 17 ++++- .../logical/command/sys/SysColumnsTests.java | 33 +++++++++ .../logical/command/sys/SysTypesTests.java | 22 ++++++ .../sql/type/SqlDataTypeConverterTests.java | 15 ++-- 15 files changed, 239 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data b/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data index 8f9bcd08993cd..36f979336be06 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data +++ b/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data @@ -128,6 +128,7 @@ "transID": 0, "process.entity_id": 512, "process.pid": 123, + "version": "1.2.4", "sequence": 20 }, { @@ -136,6 +137,7 @@ "transID": 0, "process.entity_id": 512, "process.pid": 123, + "version": "1.11.3", "sequence": 21 }, { @@ -144,6 +146,7 @@ "transID": 0, "process.entity_id": 512, "process.pid": 123, + "version": "bad", "sequence": 22 }, { @@ -152,6 +155,7 @@ "transID": 0, "process.entity_id": 512, "process.pid": 123, + "version": "1.2.4-SNAPSHOT", "sequence": 23 } ] diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.mapping b/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.mapping index 44fa94b97ce07..2fde8cf52fdd8 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.mapping +++ b/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.mapping @@ -27,6 +27,9 @@ "optional_field_default_null": { "type": "keyword", "null_value": "NULL" + }, + "version": { + "type": "version" } } } diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml b/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml index 3f8fdb6a1ae50..5f736eb73f13c 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml +++ b/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml @@ -76,7 +76,7 @@ expected_event_ids = [] [[queries]] name = "sequenceOptionalFieldAsKeyAndFirstFilter" query = ''' -sequence by ?x +sequence by ?x [OPTIONAL where ?x == null] [OPTIONAL where true] ''' @@ -137,7 +137,7 @@ expected_event_ids = [12,13,14, join_keys = ["null","123", "null","null", "512","123"] - + [[queries]] name = "sequenceOneKeyOptional_1" query = ''' @@ -159,7 +159,7 @@ expected_event_ids = [12,13,14, 18,19,20] join_keys = ["null","123", "512","123"] - + [[queries]] name = "sequenceNoOptionalKeys" query = ''' @@ -195,7 +195,7 @@ expected_event_ids = [12,13,14, join_keys = ["null","123", "null","null", "512","123"] - + [[queries]] name = "sequenceWithOptionalFieldsAndUntil_2" query = ''' @@ -209,3 +209,43 @@ expected_event_ids = [12,13,14, join_keys = ["null","123", "null","null"] +[[queries]] +name = "sequenceWithVersionCheck" +query = ''' + sequence by transID + [ file where version == "1.2.4" ] + [ file where version == "1.11.3" ] +''' +expected_event_ids = [20, 21] +join_keys = ["0"] + +[[queries]] +name = "sequenceWithVersionRangeCheck" +query = ''' + sequence by transID + [ file where version == "1.2.4" ] + [ file where version > "1.5.0" ] +''' +expected_event_ids = [20, 21] +join_keys = ["0"] + +[[queries]] +name = "sequenceWithVersionRangeCheck2" +query = ''' + sequence by transID + [ file where version == "1.2.4" ] + [ file where version < "1.5.0" ] +''' +expected_event_ids = [20, 23] +join_keys = ["0"] + + +[[queries]] +name = "sequenceWithInvalidVersion" +query = ''' + sequence by transID + [ file where version == "1.2.4" ] + [ file where version == "bad" ] +''' +expected_event_ids = [20, 22] +join_keys = ["0"] diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java index cf5c0f19471eb..48a2d56a3e25f 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java @@ -93,6 +93,7 @@ public void testQueryCondition() { assertEquals("1:11: Condition expression needs to be boolean, found [TEXT]", error("any where hostname")); assertEquals("1:11: Condition expression needs to be boolean, found [KEYWORD]", error("any where constant_keyword")); assertEquals("1:11: Condition expression needs to be boolean, found [IP]", error("any where source_address")); + assertEquals("1:11: Condition expression needs to be boolean, found [VERSION]", error("any where version")); } public void testQueryStartsWithNumber() { diff --git a/x-pack/plugin/eql/src/test/resources/mapping-default.json b/x-pack/plugin/eql/src/test/resources/mapping-default.json index 29a5235a03add..b7f44cbddb8ed 100644 --- a/x-pack/plugin/eql/src/test/resources/mapping-default.json +++ b/x-pack/plugin/eql/src/test/resources/mapping-default.json @@ -90,6 +90,9 @@ }, "bool" : { "type" : "boolean" + }, + "version" : { + "type" : "version" } } } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java index f239e6bc8b08c..fcd414784af3a 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java @@ -12,11 +12,14 @@ import org.elasticsearch.xpack.ql.type.DataType; import static org.elasticsearch.Version.V_8_2_0; +import static org.elasticsearch.Version.V_8_3_0; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; public final class VersionCompatibilityChecks { public static final Version INTRODUCING_UNSIGNED_LONG = V_8_2_0; + public static final Version INTRODUCING_VERSION_FIELD_TYPE = V_8_3_0; private VersionCompatibilityChecks() {} @@ -24,6 +27,9 @@ public static boolean isTypeSupportedInVersion(DataType dataType, Version versio if (dataType == UNSIGNED_LONG) { return supportsUnsignedLong(version); } + if (dataType == VERSION) { + return supportsVersionFieldType(version); + } return true; } @@ -34,10 +40,20 @@ public static boolean supportsUnsignedLong(Version version) { return INTRODUCING_UNSIGNED_LONG.compareTo(version) <= 0; } + /** + * Does the provided {@code version} support the version type (PR#85502)? + */ + public static boolean supportsVersionFieldType(Version version) { + return INTRODUCING_VERSION_FIELD_TYPE.compareTo(version) <= 0; + } + public static @Nullable Version versionIntroducingType(DataType dataType) { if (dataType == UNSIGNED_LONG) { return INTRODUCING_UNSIGNED_LONG; } + if (dataType == VERSION) { + return INTRODUCING_VERSION_FIELD_TYPE; + } return null; } diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java index 3f9665c90421d..5c5f2c8fa0de2 100644 --- a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java @@ -568,18 +568,18 @@ public void testIpToString() { public void testStringToVersion() { Converter conversion = converterFor(KEYWORD, VERSION); assertNull(conversion.convert(null)); - assertEquals("2.1.4", conversion.convert("2.1.4")); - assertEquals("2.1.4-SNAPSHOT", conversion.convert("2.1.4-SNAPSHOT")); + assertEquals(new VersionFieldValue("2.1.4"), conversion.convert("2.1.4")); + assertEquals(new VersionFieldValue("2.1.4-SNAPSHOT"), conversion.convert("2.1.4-SNAPSHOT")); } public void testVersionToString() { Source s = new Source(Location.EMPTY, "2.1.4"); Source s2 = new Source(Location.EMPTY, "2.1.4"); - Converter ipToString = converterFor(VERSION, KEYWORD); - assertEquals("2.1.4", ipToString.convert(new Literal(s, "2.1.4", VERSION))); - assertEquals("2.1.4-SNAPSHOT", ipToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); + Converter versionToString = converterFor(VERSION, KEYWORD); + assertEquals("2.1.4", versionToString.convert(new Literal(s, "2.1.4", VERSION))); + assertEquals("2.1.4-SNAPSHOT", versionToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); Converter stringToIp = converterFor(KEYWORD, VERSION); - assertEquals("2.1.4", ipToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); - assertEquals("2.1.4-SNAPSHOT", ipToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); + assertEquals("2.1.4", versionToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); + assertEquals("2.1.4-SNAPSHOT", versionToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); } } diff --git a/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json b/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json index fa73523d7ea27..c9a41ba7eee96 100644 --- a/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json +++ b/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json @@ -55,6 +55,7 @@ "properties": { "point": {"type" : "geo_point"} } - } + }, + "version": {"type" : "version"} } } diff --git a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java index 120081a7edf7f..a4c0b0e1dcbb1 100644 --- a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java +++ b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java @@ -27,6 +27,7 @@ import java.util.Map; import static org.elasticsearch.Version.V_8_2_0; +import static org.elasticsearch.Version.V_8_3_0; import static org.elasticsearch.common.time.DateUtils.toMilliSeconds; import static org.elasticsearch.test.ESTestCase.randomLongBetween; @@ -155,4 +156,9 @@ static boolean versionSupportsDateNanos() { public static boolean isUnsignedLongSupported() { return JDBC_DRIVER_VERSION.onOrAfter(V_8_2_0); } + + public static boolean isVersionFieldTypeSupported() { + return JDBC_DRIVER_VERSION.onOrAfter(V_8_3_0); + } + } diff --git a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java index cbeeee12262ca..4d9b2ec8a6887 100644 --- a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java +++ b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java @@ -79,6 +79,7 @@ import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.asTime; import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.extractNanosOnly; import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.isUnsignedLongSupported; +import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.isVersionFieldTypeSupported; import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.of; import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.randomTimeInNanos; import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.versionSupportsDateNanos; @@ -2244,6 +2245,32 @@ public void testResultSetConsumed() throws IOException { assertEquals("No row available", sqle.getMessage()); } + public void testSingleVersionFieldValue() throws SQLException, IOException { + assumeTrue("Driver version [" + JDBC_DRIVER_VERSION + "] doesn't support VERSION fields", isVersionFieldTypeSupported()); + + createTestDataForVersionType(); + String query = "SELECT name, version from test where version = '1.3.0'"; + doWithQuery(query, results -> { + results.next(); + assertEquals("version 1.3.0", results.getString("name")); + assertEquals("1.3.0", results.getString("version")); + assertFalse(results.next()); + }); + } + + public void testBadVersionFieldValue() throws SQLException, IOException { + assumeTrue("Driver version [" + JDBC_DRIVER_VERSION + "] doesn't support VERSION fields", isVersionFieldTypeSupported()); + + createTestDataForVersionType(); + String query = "SELECT name, version from test where version = 'foo'"; + doWithQuery(query, results -> { + results.next(); + assertEquals("version foo", results.getString("name")); + assertEquals("foo", results.getString("version")); + assertFalse(results.next()); + }); + } + private void doWithQuery(String query, CheckedConsumer consumer) throws SQLException { doWithQuery(() -> esJdbc(timeZoneId), query, consumer); } @@ -2470,6 +2497,48 @@ private static void indexTestFieldsDoc(String docId, Object... values) throws IO }); } + private void createTestDataForVersionType() throws IOException { + createIndexWithMapping("test"); + updateMapping("test", builder -> { + builder.startObject("name").field("type", "keyword").endObject(); + builder.startObject("version").field("type", "version").endObject(); + }); + + int docId = 1; + // first two deterministic values + index("test", "" + (docId++), builder -> { + builder.array("version", "1.3.0"); + builder.array("name", "version 1.3.0"); + }); + index("test", "" + (docId++), builder -> { + builder.array("version", "1.11.0"); + builder.array("name", "version 1.11.0"); + }); + + // some higher versions + for (int i = 0; i < randomInt(10); i++) { + index("test", "" + (docId++), builder -> { + String versionVal = (2 + randomInt(50)) + "." + randomInt(50) + "." + randomInt(50); + builder.array("version", versionVal); + builder.array("name", "version " + versionVal); + }); + } + + // some invalid versions + index("test", "" + (docId++), builder -> { + String versionVal = "foo"; + builder.array("version", versionVal); + builder.array("name", "version " + versionVal); + }); + for (int i = 0; i < randomInt(10); i++) { + index("test", "" + (docId++), builder -> { + String versionVal = "foo" + randomInt(1000); + builder.array("version", versionVal); + builder.array("name", "version " + versionVal); + }); + } + } + /** * Creates test data for all numeric get* methods. All values random and different from the other numeric fields already generated. * It returns a map containing the field name and its randomly generated value to be later used in checking the returned values. diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java index 0811b4d9a448e..88fbf4112c611 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java @@ -116,6 +116,11 @@ public void testIPs() throws IOException { ); } + public void testVersionFields() throws IOException { + assertQuery("SELECT CAST('1.2.3' AS VERSION)", "CAST('1.2.3' AS VERSION)", "version", "1.2.3", 2147483647); + assertQuery("SELECT CAST('bad' AS VERSION)", "CAST('bad' AS VERSION)", "version", "bad", 2147483647); + } + public void testDateTimeIntervals() throws IOException { assertQuery("SELECT INTERVAL '326' YEAR", "INTERVAL '326' YEAR", "interval_year", "P326Y", "+326-0", 7); assertQuery("SELECT INTERVAL '50' MONTH", "INTERVAL '50' MONTH", "interval_month", "P50M", "+0-50", 7); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java index 2efd00f0e111d..36215014f0d5d 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java @@ -30,7 +30,9 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSUPPORTED; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.sql.plan.logical.command.sys.SysColumnsTests.UNSIGNED_LONG_TEST_VERSIONS; +import static org.elasticsearch.xpack.sql.plan.logical.command.sys.SysColumnsTests.VERSION_FIELD_TEST_VERSIONS; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_POINT; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_SHAPE; import static org.elasticsearch.xpack.sql.types.SqlTypesTests.loadMapping; @@ -68,7 +70,8 @@ public void testShowColumns() { asList("point", JDBC_TYPE_GEOMETRY, GEO_POINT.typeName()), asList("shape", JDBC_TYPE_GEOMETRY, GEO_SHAPE.typeName()), asList("nested", JDBCType.STRUCT.getName(), NESTED.typeName()), - asList("nested.point", JDBC_TYPE_GEOMETRY, GEO_POINT.typeName()) + asList("nested.point", JDBC_TYPE_GEOMETRY, GEO_POINT.typeName()), + asList("version", JDBCType.VARCHAR.getName(), VERSION.typeName()) ); assertEquals(expect.size(), rows.size()); @@ -94,4 +97,16 @@ public void testUnsignedLongFiltering() { assertTrue((supportsUnsignedLong(Version.fromId(version.id)) && rows.contains(rowSupported)) || rows.contains(rowUnsupported)); } } + + public void testVersionFieldFiltering() { + List rowSupported = List.of("version", "VARCHAR", "version"); + List rowUnsupported = List.of("version", "OTHER", "unsupported"); + for (SqlVersion version : VERSION_FIELD_TEST_VERSIONS) { + List> rows = new ArrayList<>(); + // mapping's mutated by IndexCompatibility.compatible, needs to stay in the loop + Map mapping = loadMapping("mapping-multi-field-variation.json", true); + ShowColumns.fillInRows(IndexCompatibility.compatible(mapping, Version.fromId(version.id)), null, rows); + assertTrue((supportsUnsignedLong(Version.fromId(version.id)) && rows.contains(rowSupported)) || rows.contains(rowUnsupported)); + } + } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java index a346557bda857..eae134c49f601 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java @@ -46,8 +46,10 @@ import static org.elasticsearch.action.ActionListener.wrap; import static org.elasticsearch.xpack.ql.TestUtils.UTC; import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.INTRODUCING_UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.INTRODUCING_VERSION_FIELD_TYPE; import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.isTypeSupportedInVersion; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.sql.proto.Mode.isDriver; import static org.elasticsearch.xpack.sql.types.SqlTypesTests.loadMapping; import static org.mockito.ArgumentMatchers.any; @@ -65,6 +67,13 @@ public class SysColumnsTests extends ESTestCase { SqlVersion.fromId(Version.CURRENT.id) ); + public static List VERSION_FIELD_TEST_VERSIONS = List.of( + SqlVersion.fromId(INTRODUCING_VERSION_FIELD_TYPE.id - SqlVersion.MINOR_MULTIPLIER), + SqlVersion.fromId(INTRODUCING_VERSION_FIELD_TYPE.id), + SqlVersion.fromId(INTRODUCING_VERSION_FIELD_TYPE.id + SqlVersion.MINOR_MULTIPLIER), + SqlVersion.fromId(Version.CURRENT.id) + ); + private static final String CLUSTER_NAME = "cluster"; private static final Map MAPPING1 = loadMapping("mapping-multi-field-with-nested.json", true); private static final Map MAPPING2 = loadMapping("mapping-multi-field-variation.json", true); @@ -170,6 +179,30 @@ public void testUnsignedLongFiltering() { } } + public void testVersionFieldFiltering() { + for (Mode mode : List.of(Mode.JDBC, Mode.ODBC)) { + for (SqlVersion version : VERSION_FIELD_TEST_VERSIONS) { + List> rows = new ArrayList<>(); + // mapping's mutated by IndexCompatibility.compatible, needs to stay in the loop + Map mapping = loadMapping("mapping-multi-field-variation.json", true); + SysColumns.fillInRows( + "test", + "index", + IndexCompatibility.compatible(mapping, Version.fromId(version.id)), + null, + rows, + null, + mode + ); + List types = rows.stream().map(row -> name(row).toString()).collect(Collectors.toList()); + assertEquals( + isTypeSupportedInVersion(VERSION, Version.fromId(version.id)), + types.contains(VERSION.toString().toLowerCase(Locale.ROOT)) + ); + } + } + } + private static Object name(List list) { return list.get(3); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java index 75c57985253e4..9103663a95641 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java @@ -37,7 +37,9 @@ import static org.elasticsearch.action.ActionListener.wrap; import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.isTypeSupportedInVersion; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.sql.plan.logical.command.sys.SysColumnsTests.UNSIGNED_LONG_TEST_VERSIONS; +import static org.elasticsearch.xpack.sql.plan.logical.command.sys.SysColumnsTests.VERSION_FIELD_TEST_VERSIONS; import static org.mockito.Mockito.mock; public class SysTypesTests extends ESTestCase { @@ -160,6 +162,26 @@ public void testUnsignedLongFiltering() { } } + public void testVersionFieldFiltering() { + Set versions = new HashSet<>(VERSION_FIELD_TEST_VERSIONS); + versions.add(null); + for (SqlVersion version : versions) { + for (Mode mode : Mode.values()) { + Tuple cmd = sql("SYS TYPES", mode, version); + + cmd.v1().execute(cmd.v2(), wrap(p -> { + SchemaRowSet r = (SchemaRowSet) p.rowSet(); + List types = new ArrayList<>(); + r.forEachRow(rv -> types.add((String) rv.column(0))); + assertEquals( + isTypeSupportedInVersion(VERSION, Version.fromId(cmd.v2().configuration().version().id)), + types.contains(VERSION.toString()) + ); + }, ex -> fail(ex.getMessage()))); + } + } + } + public void testSysTypesDefaultFiltering() { Tuple cmd = sql("SYS TYPES 0"); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java index f767938c065c0..88a933efc3d26 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.Converter; import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.VersionFieldValue; import org.elasticsearch.xpack.sql.util.DateUtils; import java.math.BigInteger; @@ -778,19 +779,19 @@ public void testIpToString() { public void testStringToVersion() { Converter conversion = converterFor(KEYWORD, VERSION); assertNull(conversion.convert(null)); - assertEquals("2.1.4", conversion.convert("2.1.4")); - assertEquals("2.1.4-SNAPSHOT", conversion.convert("2.1.4-SNAPSHOT")); + assertEquals(new VersionFieldValue("2.1.4"), conversion.convert("2.1.4")); + assertEquals(new VersionFieldValue("2.1.4-SNAPSHOT"), conversion.convert("2.1.4-SNAPSHOT")); } public void testVersionToString() { Source s = new Source(Location.EMPTY, "2.1.4"); Source s2 = new Source(Location.EMPTY, "2.1.4-SNAPSHOT"); - Converter ipToString = converterFor(VERSION, KEYWORD); - assertEquals("2.1.4", ipToString.convert(new Literal(s, "2.1.4", VERSION))); - assertEquals("2.1.4-SNAPSHOT", ipToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); + Converter stringToString = converterFor(VERSION, KEYWORD); + assertEquals("2.1.4", stringToString.convert(new Literal(s, "2.1.4", VERSION))); + assertEquals("2.1.4-SNAPSHOT", stringToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); Converter stringToIp = converterFor(KEYWORD, VERSION); - assertEquals("2.1.4", ipToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); - assertEquals("2.1.4-SNAPSHOT", ipToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); + assertEquals("2.1.4", stringToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); + assertEquals("2.1.4-SNAPSHOT", stringToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); } private DataType randomInterval() { From 0a0eaf6a457ddb3abc77e03539cb0bcb1a5252e0 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Tue, 12 Apr 2022 13:33:06 +0200 Subject: [PATCH 10/20] Fix test cases --- .../xpack/sql/analysis/analyzer/FieldAttributeTests.java | 2 +- .../xpack/sql/plan/logical/command/sys/SysColumnsTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java index 135b20c2589e1..c87911b0b9f44 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java @@ -182,7 +182,7 @@ public void testDottedFieldPathTypo() { public void testStarExpansionExcludesObjectAndUnsupportedTypes() { LogicalPlan plan = plan("SELECT * FROM test"); List list = ((Project) plan).projections(); - assertThat(list, hasSize(13)); + assertThat(list, hasSize(14)); List names = Expressions.names(list); assertThat(names, not(hasItem("some"))); assertThat(names, not(hasItem("some.dotted"))); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java index eae134c49f601..e0bf9fee430e3 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java @@ -78,7 +78,7 @@ public class SysColumnsTests extends ESTestCase { private static final Map MAPPING1 = loadMapping("mapping-multi-field-with-nested.json", true); private static final Map MAPPING2 = loadMapping("mapping-multi-field-variation.json", true); private static final int FIELD_COUNT1 = 20; - private static final int FIELD_COUNT2 = 18; + private static final int FIELD_COUNT2 = 19; private final SqlParser parser = new SqlParser(); From eae32e12174e39f09d67b9138cef89262f381af9 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Mon, 9 May 2022 15:25:21 +0200 Subject: [PATCH 11/20] Add support for ORDER BY versionValueCalculatedWithScript --- .../eql/execution/search/SourceGenerator.java | 6 +- .../eql/plugin/EqlPainlessExtension.java | 2 + .../whitelist/InternalQlScriptUtils.java | 12 +++ .../ql/expression/gen/script/Scripts.java | 9 ++- .../operator/comparison/Comparisons.java | 10 +-- .../elasticsearch/xpack/ql/type/DataType.java | 11 +++ .../xpack/ql/type/DataTypeConverter.java | 3 +- .../xpack/ql/type/VersionFieldValue.java | 53 ------------- .../ql/type/DataTypeConversionTests.java | 7 +- .../xpack/ql/type/VersionFieldValueTests.java | 76 ------------------- .../src/main/resources/version.csv-spec | 19 +++++ .../xpack/sql/action/SqlQueryResponse.java | 3 +- .../sql/execution/search/SourceGenerator.java | 6 +- .../sql/plugin/SqlPainlessExtension.java | 2 + .../xpack/sql/querydsl/agg/TopHitsAgg.java | 12 +-- .../xpack/sql/plugin/sql_whitelist.txt | 1 + .../sql/type/SqlDataTypeConverterTests.java | 8 +- 17 files changed, 78 insertions(+), 162 deletions(-) delete mode 100644 x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java delete mode 100644 x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java index 539a49a8df580..ab1fda7bcdfd8 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/search/SourceGenerator.java @@ -11,7 +11,6 @@ import org.elasticsearch.search.fetch.subphase.FieldAndFormat; import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.sort.NestedSortBuilder; -import org.elasticsearch.search.sort.ScriptSortBuilder.ScriptSortType; import org.elasticsearch.search.sort.SortBuilder; import org.elasticsearch.xpack.eql.querydsl.container.QueryContainer; import org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder; @@ -133,10 +132,7 @@ private static void sorting(QueryContainer container, SearchSourceBuilder source } } } else if (sortable instanceof ScriptSort ss) { - sortBuilder = scriptSort( - ss.script().toPainless(), - ss.script().outputType().isNumeric() ? ScriptSortType.NUMBER : ScriptSortType.STRING - ); + sortBuilder = scriptSort(ss.script().toPainless(), ss.script().outputType().scriptSortType()); } if (sortBuilder != null) { diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlPainlessExtension.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlPainlessExtension.java index 580e7ff4c07dc..77e3c0aa9ad64 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlPainlessExtension.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlPainlessExtension.java @@ -11,6 +11,7 @@ import org.elasticsearch.painless.spi.WhitelistLoader; import org.elasticsearch.script.AggregationScript; import org.elasticsearch.script.BucketAggregationSelectorScript; +import org.elasticsearch.script.BytesRefSortScript; import org.elasticsearch.script.FieldScript; import org.elasticsearch.script.FilterScript; import org.elasticsearch.script.NumberSortScript; @@ -36,6 +37,7 @@ public Map, List> getContextWhitelists() { whitelist.put(FieldScript.CONTEXT, list); whitelist.put(NumberSortScript.CONTEXT, list); whitelist.put(StringSortScript.CONTEXT, list); + whitelist.put(BytesRefSortScript.CONTEXT, list); whitelist.put(BucketAggregationSelectorScript.CONTEXT, list); return whitelist; } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java index 25a6575dc7411..00c24a0f46774 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java @@ -18,6 +18,7 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.InProcessor; import org.elasticsearch.xpack.ql.expression.predicate.regex.RegexProcessor.RegexOperation; import org.elasticsearch.xpack.ql.util.StringUtils; +import org.elasticsearch.xpack.versionfield.Version; import java.util.List; import java.util.Map; @@ -55,6 +56,17 @@ public static String nullSafeSortString(Object sort) { return sort == null ? StringUtils.EMPTY : sort.toString(); } + public static String nullSafeSortVersion(Object sort) { + if (sort instanceof String) { + return (String) sort; + } + Version version = (Version) sort; + if (version == null) { + return StringUtils.EMPTY; + } + return version.toString(); + } + public static Number nullSafeCastNumeric(Number number, String typeName) { return number == null || Double.isNaN(number.doubleValue()) ? null : (Number) convert(number, fromTypeName(typeName)); } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java index fb8664a733db5..89fcddda9f254 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java @@ -80,7 +80,14 @@ public static ScriptTemplate nullSafeFilter(ScriptTemplate script) { } public static ScriptTemplate nullSafeSort(ScriptTemplate script) { - String methodName = script.outputType().isNumeric() ? "nullSafeSortNumeric" : "nullSafeSortString"; + String methodName; + if (script.outputType().name().equals("VERSION")) { + methodName = "nullSafeSortVersion"; + } else if (script.outputType().isNumeric()) { + methodName = "nullSafeSortNumeric"; + } else { + methodName = "nullSafeSortString"; + } return new ScriptTemplate( formatTemplate(format(Locale.ROOT, "{ql}.%s(%s)", methodName, script.template())), script.params(), diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java index f2ac5af3c049d..a0ad875cc4945 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java @@ -6,7 +6,7 @@ */ package org.elasticsearch.xpack.ql.expression.predicate.operator.comparison; -import org.elasticsearch.xpack.ql.type.VersionFieldValue; +import org.elasticsearch.xpack.versionfield.Version; import java.math.BigInteger; import java.util.Set; @@ -78,11 +78,11 @@ static Integer compare(Object l, Object r) { } // automatic conversion for versions - if (l instanceof VersionFieldValue && r instanceof String) { - return ((VersionFieldValue) l).compareTo(new VersionFieldValue((String) r)); + if (l instanceof Version && r instanceof String) { + return ((Version) l).compareTo(new Version((String) r)); } - if (l instanceof String && r instanceof VersionFieldValue) { - return new VersionFieldValue((String) l).compareTo((VersionFieldValue) r); + if (l instanceof String && r instanceof Version) { + return new Version((String) l).compareTo((Version) r); } if (l instanceof Comparable && r instanceof Comparable) { diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java index 5b18234347fab..bdfad043ff592 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.ql.type; +import org.elasticsearch.search.sort.ScriptSortBuilder; + import java.util.Locale; import java.util.Objects; @@ -62,6 +64,15 @@ public String esType() { return esType; } + public ScriptSortBuilder.ScriptSortType scriptSortType() { + if (isNumeric()) { + return ScriptSortBuilder.ScriptSortType.NUMBER; + } else if ("version".equals(typeName())) { + return ScriptSortBuilder.ScriptSortType.VERSION; + } + return ScriptSortBuilder.ScriptSortType.STRING; + } + public boolean isInteger() { return isInteger; } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java index 57278432b0a5a..02a9f0f05e255 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataTypeConverter.java @@ -11,6 +11,7 @@ import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.core.Booleans; import org.elasticsearch.xpack.ql.QlIllegalArgumentException; +import org.elasticsearch.xpack.versionfield.Version; import java.io.IOException; import java.math.BigDecimal; @@ -549,7 +550,7 @@ public enum DefaultConverter implements Converter { } return o; }), - STRING_TO_VERSION(o -> new VersionFieldValue(o.toString())); + STRING_TO_VERSION(o -> new Version(o.toString())); public static final String NAME = "dtc-def"; diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java deleted file mode 100644 index 9fd8e9516a9ca..0000000000000 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/VersionFieldValue.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.ql.type; - -import org.elasticsearch.xpack.versionfield.VersionEncoder; - -import java.util.Objects; - -public final class VersionFieldValue implements Comparable { - private final String stringValue; - private final VersionEncoder.EncodedVersion encoded; - - public VersionFieldValue(String str) { - if (str == null) { - throw new IllegalArgumentException("Invalid version field value: null"); - } - stringValue = str; - encoded = VersionEncoder.encodeVersion(str); - } - - @Override - public int compareTo(VersionFieldValue o) { - return this.encoded.bytesRef.compareTo(o.encoded.bytesRef); - } - - @Override - public String toString() { - return stringValue; - } - - public boolean isValid() { - return encoded.isLegal; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - VersionFieldValue that = (VersionFieldValue) o; - return Objects.equals(stringValue, that.stringValue); - } - - @Override - public int hashCode() { - return Objects.hash(stringValue); - } - -} diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java index 5c5f2c8fa0de2..48154e1d4388c 100644 --- a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.xpack.ql.expression.Literal; import org.elasticsearch.xpack.ql.tree.Location; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.versionfield.Version; import java.math.BigDecimal; import java.math.BigInteger; @@ -568,8 +569,10 @@ public void testIpToString() { public void testStringToVersion() { Converter conversion = converterFor(KEYWORD, VERSION); assertNull(conversion.convert(null)); - assertEquals(new VersionFieldValue("2.1.4"), conversion.convert("2.1.4")); - assertEquals(new VersionFieldValue("2.1.4-SNAPSHOT"), conversion.convert("2.1.4-SNAPSHOT")); + assertEquals(new Version("2.1.4").toString(), conversion.convert("2.1.4").toString()); + assertEquals(new Version("2.1.4").toBytesRef(), ((Version) conversion.convert("2.1.4")).toBytesRef()); + assertEquals(new Version("2.1.4-SNAPSHOT").toString(), conversion.convert("2.1.4-SNAPSHOT").toString()); + assertEquals(new Version("2.1.4-SNAPSHOT").toBytesRef(), ((Version) conversion.convert("2.1.4-SNAPSHOT")).toBytesRef()); } public void testVersionToString() { diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java deleted file mode 100644 index 4a837bb7daa7f..0000000000000 --- a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/VersionFieldValueTests.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.ql.type; - -import org.elasticsearch.test.ESTestCase; - -public class VersionFieldValueTests extends ESTestCase { - - public void testInvalidInput() { - expectThrows(IllegalArgumentException.class, () -> new VersionFieldValue(null)); - } - - public void testValid() { - assertTrue(new VersionFieldValue("1").isValid()); - assertTrue(new VersionFieldValue("1.0").isValid()); - assertTrue(new VersionFieldValue("1.0.0").isValid()); - assertTrue(new VersionFieldValue("1.0.0.2").isValid()); - assertTrue( - new VersionFieldValue( - "" - + randomIntBetween(0, Integer.MAX_VALUE) - + "." - + randomIntBetween(0, Integer.MAX_VALUE) - + "." - + randomIntBetween(0, Integer.MAX_VALUE) - ).isValid() - ); - assertTrue(new VersionFieldValue("1.4.0-rc1").isValid()); - assertTrue(new VersionFieldValue("1.0.0.0-SNAPSHOT").isValid()); - - } - - public void testInvalid() { - assertFalse(new VersionFieldValue("-SNAPSHOT").isValid()); - assertFalse(new VersionFieldValue("1.foo.0-SNAPSHOT.1").isValid()); - assertFalse(new VersionFieldValue("rc1").isValid()); - assertFalse(new VersionFieldValue("bad").isValid()); - } - - public void testCompare() { - assertEquals(new VersionFieldValue("1.2.4"), new VersionFieldValue("1.2.4")); - - // left < right - String[][] comparisons = { - { "1.0.0", "1.1.0" }, - { "1.0.0", "1.0.1" }, - { "1.0.0", "2.0.0" }, - { "1.0.0-SNAPSHOT", "1.0.0" }, - { "1.0.0", "1.0.1-SNAPSHOT" }, - { "1.2.0", "1.11.0" }, - { "1.1.2", "1.1.11" }, - { "1.0.0-alpha1", "1.0.0-alpha2" }, - { "1.0.0-alpha", "1.0.0-beta" }, - { "1.0.0-beta", "1.0.0-rc1" }, - { "1000.0.0", "bad" }, - { "1000.0.0", "SNAPSHOT" }, - { "bar", "foo" } }; - for (int i = 0; i < comparisons.length; i++) { - String[] item = comparisons[i]; - assertTrue( - "Wrong version comparison " + item[0] + " < " + item[1], - new VersionFieldValue(item[0]).compareTo(new VersionFieldValue(item[1])) < 0 - ); - assertTrue( - "Wrong version comparison " + item[1] + " > " + item[0], - new VersionFieldValue(item[1]).compareTo(new VersionFieldValue(item[0])) > 0 - ); - } - - } -} diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec index 54989a303f798..9b87c8f128a63 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec @@ -86,6 +86,25 @@ SELECT * FROM apps ORDER BY version, id; ; + +orderByVersionScript +SELECT * FROM apps ORDER BY CONCAT('1.', CAST(version AS TEXT))::version, id; + + id:i | name:s | version:s +------+-----------+--------------- +1 |aaaaa |1 +8 |hhhhh |1.2.3.4 +5 |eeeee |1.11.0 +2 |bbbbb |2.1 +3 |ccccc |2.3.4 +4 |ddddd |2.12.0 +7 |ggggg |5.2.9-SNAPSHOT +6 |fffff |5.2.9 +10 |jjjjj |5.2.9 +9 |iiiii |bad + +; + groupByVersion SELECT max(id) as id, version FROM apps GROUP BY version ORDER BY version; diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java index 3ff737007c954..c6ffbbc99f307 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java @@ -17,7 +17,6 @@ import org.elasticsearch.xcontent.ToXContentObject; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xpack.ql.async.QlStatusResponse; -import org.elasticsearch.xpack.ql.type.VersionFieldValue; import org.elasticsearch.xpack.sql.proto.ColumnInfo; import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.SqlVersion; @@ -275,7 +274,7 @@ private static XContentBuilder toXContent(ColumnInfo info, XContentBuilder build * Serializes the provided value in SQL-compatible way based on the client mode */ public static XContentBuilder value(XContentBuilder builder, Mode mode, SqlVersion sqlVersion, Object value) throws IOException { - if (value instanceof VersionFieldValue) { + if (value instanceof org.elasticsearch.xpack.versionfield.Version) { builder.value(value.toString()); } else if (value instanceof ZonedDateTime zdt) { // use the ISO format diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java index ae216a903f5f2..b9ebe75fd2879 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java @@ -13,7 +13,6 @@ import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.sort.NestedSortBuilder; -import org.elasticsearch.search.sort.ScriptSortBuilder.ScriptSortType; import org.elasticsearch.search.sort.SortBuilder; import org.elasticsearch.xpack.ql.execution.search.QlSourceBuilder; import org.elasticsearch.xpack.ql.expression.Attribute; @@ -142,10 +141,7 @@ private static void sorting(QueryContainer container, SearchSourceBuilder source } } } else if (sortable instanceof ScriptSort ss) { - sortBuilder = scriptSort( - ss.script().toPainless(), - ss.script().outputType().isNumeric() ? ScriptSortType.NUMBER : ScriptSortType.STRING - ); + sortBuilder = scriptSort(ss.script().toPainless(), ss.script().outputType().scriptSortType()); } else if (sortable instanceof ScoreSort) { sortBuilder = scoreSort(); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPainlessExtension.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPainlessExtension.java index 33a85e158647b..b17bcbf76f77f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPainlessExtension.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPainlessExtension.java @@ -11,6 +11,7 @@ import org.elasticsearch.painless.spi.WhitelistLoader; import org.elasticsearch.script.AggregationScript; import org.elasticsearch.script.BucketAggregationSelectorScript; +import org.elasticsearch.script.BytesRefSortScript; import org.elasticsearch.script.FieldScript; import org.elasticsearch.script.FilterScript; import org.elasticsearch.script.NumberSortScript; @@ -36,6 +37,7 @@ public Map, List> getContextWhitelists() { whitelist.put(FieldScript.CONTEXT, list); whitelist.put(NumberSortScript.CONTEXT, list); whitelist.put(StringSortScript.CONTEXT, list); + whitelist.put(BytesRefSortScript.CONTEXT, list); whitelist.put(BucketAggregationSelectorScript.CONTEXT, list); return whitelist; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/TopHitsAgg.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/TopHitsAgg.java index 6de81fac8f650..e87cff0c06397 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/TopHitsAgg.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/TopHitsAgg.java @@ -60,9 +60,7 @@ AggregationBuilder toBuilder() { sortBuilderList.add( new ScriptSortBuilder( Scripts.nullSafeSort(sortSource.script()).toPainless(), - sortSource.script().outputType().isNumeric() - ? ScriptSortBuilder.ScriptSortType.NUMBER - : ScriptSortBuilder.ScriptSortType.STRING + source().script().outputType().scriptSortType() ).order(sortOrder) ); } @@ -74,12 +72,8 @@ AggregationBuilder toBuilder() { ); } else { sortBuilderList.add( - new ScriptSortBuilder( - Scripts.nullSafeSort(source().script()).toPainless(), - source().script().outputType().isNumeric() - ? ScriptSortBuilder.ScriptSortType.NUMBER - : ScriptSortBuilder.ScriptSortType.STRING - ).order(sortOrder) + new ScriptSortBuilder(Scripts.nullSafeSort(source().script()).toPainless(), source().script().outputType().scriptSortType()) + .order(sortOrder) ); } diff --git a/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt b/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt index 018d9757e3ceb..0ecec66de978e 100644 --- a/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt +++ b/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt @@ -29,6 +29,7 @@ class org.elasticsearch.xpack.ql.expression.function.scalar.whitelist.InternalQl boolean nullSafeFilter(Boolean) double nullSafeSortNumeric(Number) String nullSafeSortString(Object) + String nullSafeSortVersion(Object) # # ASCII Functions diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java index 88a933efc3d26..5b3ddcd2fb159 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java @@ -14,8 +14,8 @@ import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.Converter; import org.elasticsearch.xpack.ql.type.DataType; -import org.elasticsearch.xpack.ql.type.VersionFieldValue; import org.elasticsearch.xpack.sql.util.DateUtils; +import org.elasticsearch.xpack.versionfield.Version; import java.math.BigInteger; import java.time.OffsetTime; @@ -779,8 +779,10 @@ public void testIpToString() { public void testStringToVersion() { Converter conversion = converterFor(KEYWORD, VERSION); assertNull(conversion.convert(null)); - assertEquals(new VersionFieldValue("2.1.4"), conversion.convert("2.1.4")); - assertEquals(new VersionFieldValue("2.1.4-SNAPSHOT"), conversion.convert("2.1.4-SNAPSHOT")); + assertEquals(new Version("2.1.4").toString(), conversion.convert("2.1.4").toString()); + assertEquals(new Version("2.1.4").toBytesRef(), ((Version) conversion.convert("2.1.4")).toBytesRef()); + assertEquals(new Version("2.1.4-SNAPSHOT").toString(), conversion.convert("2.1.4-SNAPSHOT").toString()); + assertEquals(new Version("2.1.4-SNAPSHOT").toBytesRef(), ((Version) conversion.convert("2.1.4-SNAPSHOT")).toBytesRef()); } public void testVersionToString() { From 401d12e56bcc7f0556b863d8c4b0257efed12f08 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Thu, 12 May 2022 12:34:35 +0200 Subject: [PATCH 12/20] Add further tests and address review comments --- .../common/src/main/resources/test_extra.toml | 11 +++ .../test/resources/querytranslator_tests.txt | 14 +++ .../elasticsearch/xpack/ql/type/DataType.java | 2 +- .../ql/type/DataTypeConversionTests.java | 24 ++++- .../mapping-multi-field-variation.json | 2 +- .../xpack/sql/qa/jdbc/ResultSetTestCase.java | 10 +-- .../sql/qa/server/src/main/resources/apps.csv | 1 + .../src/main/resources/version.csv-spec | 89 +++++++++++++++++++ .../xpack/sql/querydsl/agg/GroupByKey.java | 1 - .../analyzer/FieldAttributeTests.java | 41 +++++++++ .../logical/command/ShowColumnsTests.java | 5 +- .../sql/planner/querytranslator_tests.txt | 14 +++ 12 files changed, 202 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml b/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml index 5f736eb73f13c..a95d9f8593047 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml +++ b/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml @@ -249,3 +249,14 @@ query = ''' ''' expected_event_ids = [20, 22] join_keys = ["0"] + + +[[queries]] +name = "sequenceWithVersionConcat" +query = ''' + sequence by transID + [ file where CONCAT(version, "") == "1.2.4" ] + [ file where version == "bad" ] +''' +expected_event_ids = [20, 22] +join_keys = ["0"] diff --git a/x-pack/plugin/eql/src/test/resources/querytranslator_tests.txt b/x-pack/plugin/eql/src/test/resources/querytranslator_tests.txt index 5c6187b4f4271..e8c01ed2dac2e 100644 --- a/x-pack/plugin/eql/src/test/resources/querytranslator_tests.txt +++ b/x-pack/plugin/eql/src/test/resources/querytranslator_tests.txt @@ -901,3 +901,17 @@ process where pid > 100 and pid < 200 // the "not" has an undefined location of where it should be used in the Painless script // disjunctionInsideFunctionWithNot // process where string(pid > 5 and pid != 10) == \"true\" + + +versionFieldAutomaticConversion +process where version > "2" +; +{"range":{"version":{"gt":"2" +; + +versionFieldCast +process where CONCAT(version, constant_keyword) > "2" +; +{"script":{"source":"InternalEqlScriptUtils.multiValueDocValues(doc,params.v0,X0->InternalEqlScriptUtils.multiValueDocValues(doc,params.v1,X1->InternalQlScriptUtils.nullSafeFilter(InternalQlScriptUtils.gt(InternalEqlScriptUtils.concat([X0,X1]),params.v2))))" +"params":{"v0":"version","v1":"constant_keyword","v2":"2"}} +; diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java index bdfad043ff592..55ee0c5faedad 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java @@ -67,7 +67,7 @@ public String esType() { public ScriptSortBuilder.ScriptSortType scriptSortType() { if (isNumeric()) { return ScriptSortBuilder.ScriptSortType.NUMBER; - } else if ("version".equals(typeName())) { + } else if (this == DataTypes.VERSION) { return ScriptSortBuilder.ScriptSortType.VERSION; } return ScriptSortBuilder.ScriptSortType.STRING; diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java index 48154e1d4388c..32e57a22da823 100644 --- a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java @@ -566,7 +566,7 @@ public void testIpToString() { assertEquals("10.0.0.1", ipToString.convert(stringToIp.convert(new Literal(s, "10.0.0.1", KEYWORD)))); } - public void testStringToVersion() { + public void testKeywordToVersion() { Converter conversion = converterFor(KEYWORD, VERSION); assertNull(conversion.convert(null)); assertEquals(new Version("2.1.4").toString(), conversion.convert("2.1.4").toString()); @@ -575,7 +575,16 @@ public void testStringToVersion() { assertEquals(new Version("2.1.4-SNAPSHOT").toBytesRef(), ((Version) conversion.convert("2.1.4-SNAPSHOT")).toBytesRef()); } - public void testVersionToString() { + public void testStringToVersion() { + Converter conversion = converterFor(TEXT, VERSION); + assertNull(conversion.convert(null)); + assertEquals(new Version("2.1.4").toString(), conversion.convert("2.1.4").toString()); + assertEquals(new Version("2.1.4").toBytesRef(), ((Version) conversion.convert("2.1.4")).toBytesRef()); + assertEquals(new Version("2.1.4-SNAPSHOT").toString(), conversion.convert("2.1.4-SNAPSHOT").toString()); + assertEquals(new Version("2.1.4-SNAPSHOT").toBytesRef(), ((Version) conversion.convert("2.1.4-SNAPSHOT")).toBytesRef()); + } + + public void testVersionToKeyword() { Source s = new Source(Location.EMPTY, "2.1.4"); Source s2 = new Source(Location.EMPTY, "2.1.4"); Converter versionToString = converterFor(VERSION, KEYWORD); @@ -585,4 +594,15 @@ public void testVersionToString() { assertEquals("2.1.4", versionToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); assertEquals("2.1.4-SNAPSHOT", versionToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); } + + public void testVersionToString() { + Source s = new Source(Location.EMPTY, "2.1.4"); + Source s2 = new Source(Location.EMPTY, "2.1.4"); + Converter versionToString = converterFor(VERSION, TEXT); + assertEquals("2.1.4", versionToString.convert(new Literal(s, "2.1.4", VERSION))); + assertEquals("2.1.4-SNAPSHOT", versionToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); + Converter stringToIp = converterFor(KEYWORD, TEXT); + assertEquals("2.1.4", versionToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); + assertEquals("2.1.4-SNAPSHOT", versionToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); + } } diff --git a/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json b/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json index c9a41ba7eee96..b5b3d42816502 100644 --- a/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json +++ b/x-pack/plugin/ql/src/test/resources/mapping-multi-field-variation.json @@ -56,6 +56,6 @@ "point": {"type" : "geo_point"} } }, - "version": {"type" : "version"} + "version": {"type" : "version"} } } diff --git a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java index 4d9b2ec8a6887..d66230642a485 100644 --- a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java +++ b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java @@ -2254,15 +2254,13 @@ public void testSingleVersionFieldValue() throws SQLException, IOException { results.next(); assertEquals("version 1.3.0", results.getString("name")); assertEquals("1.3.0", results.getString("version")); + SQLException sqle = expectThrows(SQLException.class, () -> results.getByte("version")); + assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VERSION] to [Byte]", "1.3.0"), sqle.getMessage()); assertFalse(results.next()); }); - } - - public void testBadVersionFieldValue() throws SQLException, IOException { - assumeTrue("Driver version [" + JDBC_DRIVER_VERSION + "] doesn't support VERSION fields", isVersionFieldTypeSupported()); - createTestDataForVersionType(); - String query = "SELECT name, version from test where version = 'foo'"; + // bad version value + query = "SELECT name, version from test where version = 'foo'"; doWithQuery(query, results -> { results.next(); assertEquals("version foo", results.getString("name")); diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv index 62c3c1e17d7e6..2f97d416e3735 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv +++ b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv @@ -9,3 +9,4 @@ id,version,name 8,1.2.3.4,hhhhh 9,bad,iiiii 10,5.2.9,jjjjj +11,,kkkkk diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec index 9b87c8f128a63..16099c9bbca72 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec @@ -19,6 +19,7 @@ SELECT * FROM apps ORDER BY id; 8 |hhhhh |1.2.3.4 9 |iiiii |bad 10 |jjjjj |5.2.9 +11 |kkkkk |null ; @@ -68,6 +69,31 @@ SELECT * FROM apps WHERE version >= '2.3.4' ORDER BY id; ; +between +SELECT * FROM apps WHERE version BETWEEN '1.10' AND '5.2.9' ORDER BY id; + + id:i | name:s | version:s +--------+-------------+--------------- +2 |bbbbb | 2.1 +3 |ccccc | 2.3.4 +4 |ddddd | 2.12.0 +5 |eeeee | 1.11.0 +6 |fffff | 5.2.9 +7 |ggggg | 5.2.9-SNAPSHOT +10 |jjjjj | 5.2.9 + +; + +rlike +SELECT * FROM apps WHERE version::string RLIKE '2.*.4'; + + id:i | name:s | version:s +--------+-------------+--------------- +3 |ccccc | 2.3.4 + +; + + orderByVersion SELECT * FROM apps ORDER BY version, id; @@ -83,6 +109,26 @@ SELECT * FROM apps ORDER BY version, id; 6 |fffff |5.2.9 10 |jjjjj |5.2.9 9 |iiiii |bad +11 |kkkkk |null + +; + +orderByVersionDesc +SELECT * FROM apps ORDER BY version DESC, id ASC; + + id:i | name:s | version:s +------+-----------+--------------- +11 |kkkkk |null +9 |iiiii |bad +6 |fffff |5.2.9 +10 |jjjjj |5.2.9 +7 |ggggg |5.2.9-SNAPSHOT +4 |ddddd |2.12.0 +3 |ccccc |2.3.4 +2 |bbbbb |2.1 +5 |eeeee |1.11.0 +8 |hhhhh |1.2.3.4 +1 |aaaaa |1 ; @@ -101,6 +147,7 @@ SELECT * FROM apps ORDER BY CONCAT('1.', CAST(version AS TEXT))::version, id; 7 |ggggg |5.2.9-SNAPSHOT 6 |fffff |5.2.9 10 |jjjjj |5.2.9 +11 |kkkkk |null 9 |iiiii |bad ; @@ -110,6 +157,7 @@ SELECT max(id) as id, version FROM apps GROUP BY version ORDER BY version; id:i | version:s ------+----------- +11 |null 1 |1 8 |1.2.3.4 5 |1.11.0 @@ -122,6 +170,17 @@ SELECT max(id) as id, version FROM apps GROUP BY version ORDER BY version; ; +groupOrderLimit +SELECT version, version as v, version as v2 FROM apps WHERE version IS NOT NULL GROUP BY 1 ORDER BY 2 DESC LIMIT 3; + + version:s | v:s | v2:s +------------------+------------------+------------------ + bad | bad | bad + 5.2.9 | 5.2.9 | 5.2.9 + 5.2.9-SNAPSHOT | 5.2.9-SNAPSHOT | 5.2.9-SNAPSHOT + +; + groupByVersionScript SELECT max(id) as id FROM apps GROUP BY CONCAT('1.', CAST(version AS TEXT))::version ORDER BY id; @@ -136,6 +195,7 @@ SELECT max(id) as id FROM apps GROUP BY CONCAT('1.', CAST(version AS TEXT))::ver 8 9 10 +11 ; castToVersion @@ -153,6 +213,7 @@ SELECT id, name, CONCAT('1.', CAST(version AS TEXT))::version version FROM apps 8 |hhhhh |1.1.2.3.4 9 |iiiii |1.bad 10 |jjjjj |1.5.2.9 +11 |kkkkk |1. ; @@ -204,3 +265,31 @@ SELECT * FROM apps WHERE '1.2.0' < '1.11.0'::version ORDER BY id LIMIT 1; 1 |aaaaa |1 ; + +groupByVersionHaving +SELECT max(id) as idx, version FROM apps GROUP BY version HAVING idx = 10; + + idx:i | version:s +-------+----------- +10 | 5.2.9 + +; + +scriptsOperatorsOrderBy +SELECT CONCAT('123',CAST(version AS TEXT))::version v, version, CAST(version AS TEXT) version_text, id, +IIF(version > '1.1', 1, 0) m, GREATEST(version, '1.3.0'::version) g, IFNULL(version, '0.1'::version) i, +CASE WHEN version > '1.1' THEN 'high' + WHEN version IS NULL THEN 'none' + ELSE 'low' +END AS c +FROM apps WHERE version IS NULL OR version_text LIKE '1%' ORDER BY version DESC NULLS LAST, id DESC; + + v:s | version:s | version_text:s | id:i | m:i | g:s | i:s | c:s +------------+-----------+-----------------+-------+-------+--------+---------+--------- + 1231.11.0 | 1.11.0 | 1.11.0 | 5 | 1 | 1.11.0 | 1.11.0 | high + 1231.2.3.4 | 1.2.3.4 | 1.2.3.4 | 8 | 1 | 1.3.0 | 1.2.3.4 | high + 1231 | 1 | 1 | 1 | 0 | 1.3.0 | 1 | low + 123 | null | null | 11 | 0 | 1.3.0 | 0.1 | none + +; + diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java index b72335f9146de..3754082a7fcd9 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/GroupByKey.java @@ -62,7 +62,6 @@ public final CompositeValuesSourceBuilder asValueSource() { } else if (script.outputType() == IP) { builder.userValuetypeHint(ValueType.IP); } - // TODO hint for VERSION } // field based else { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java index c87911b0b9f44..c1e6249a6e1c4 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java @@ -42,11 +42,13 @@ import java.util.stream.Collectors; import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.INTRODUCING_UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.INTRODUCING_VERSION_FIELD_TYPE; import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.isTypeSupportedInVersion; import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN; import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD; import static org.elasticsearch.xpack.ql.type.DataTypes.TEXT; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.sql.types.SqlTypesTests.loadMapping; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.contains; @@ -349,6 +351,45 @@ public void testUnsignedLongVersionCompatibility() { } } + public void testVersionFieldVersionCompatibility() { + String query = "SELECT version_number FROM test"; + String queryWithCastLiteral = "SELECT '1.2.3'::version AS version_number"; + String queryWithAlias = "SELECT version_number AS version_number FROM test"; + String queryWithCast = "SELECT CONCAT(version_number::string, '-SNAPSHOT')::version AS version_number FROM test"; + + Version preVersion = Version.fromId(INTRODUCING_VERSION_FIELD_TYPE.id - SqlVersion.MINOR_MULTIPLIER); + Version postVersion = Version.fromId(INTRODUCING_VERSION_FIELD_TYPE.id + SqlVersion.MINOR_MULTIPLIER); + SqlConfiguration sqlConfig = SqlTestUtils.randomConfiguration(SqlVersion.fromId(preVersion.id)); + + for (String sql : List.of(query, queryWithCastLiteral, queryWithAlias, queryWithCast)) { + analyzer = new Analyzer( + sqlConfig, + functionRegistry, + loadCompatibleIndexResolution("mapping-version.json", preVersion), + new Verifier(new Metrics()) + ); + VerificationException ex = expectThrows(VerificationException.class, () -> plan(sql)); + assertThat(ex.getMessage(), containsString("Cannot use field [version_number]")); + + for (Version v : List.of(INTRODUCING_VERSION_FIELD_TYPE, postVersion)) { + analyzer = new Analyzer( + SqlTestUtils.randomConfiguration(SqlVersion.fromId(v.id)), + functionRegistry, + loadCompatibleIndexResolution("mapping-version.json", v), + verifier + ); + LogicalPlan plan = plan(sql); + assertThat(plan, instanceOf(Project.class)); + Project p = (Project) plan; + List projections = p.projections(); + assertThat(projections, hasSize(1)); + Attribute attribute = projections.get(0).toAttribute(); + assertThat(attribute.dataType(), is(VERSION)); + assertThat(attribute.name(), is("version_number")); + } + } + } + public void testNonProjectedUnsignedLongVersionCompatibility() { Version preUnsignedLong = Version.fromId(INTRODUCING_UNSIGNED_LONG.id - SqlVersion.MINOR_MULTIPLIER); SqlConfiguration sqlConfig = SqlTestUtils.randomConfiguration(SqlVersion.fromId(preUnsignedLong.id)); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java index 36215014f0d5d..1f4eb1d800121 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java @@ -20,6 +20,7 @@ import static java.util.Arrays.asList; import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.supportsUnsignedLong; +import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.supportsVersionFieldType; import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.FLOAT; @@ -106,7 +107,9 @@ public void testVersionFieldFiltering() { // mapping's mutated by IndexCompatibility.compatible, needs to stay in the loop Map mapping = loadMapping("mapping-multi-field-variation.json", true); ShowColumns.fillInRows(IndexCompatibility.compatible(mapping, Version.fromId(version.id)), null, rows); - assertTrue((supportsUnsignedLong(Version.fromId(version.id)) && rows.contains(rowSupported)) || rows.contains(rowUnsupported)); + assertTrue( + (supportsVersionFieldType(Version.fromId(version.id)) && rows.contains(rowSupported)) || rows.contains(rowUnsupported) + ); } } } diff --git a/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt b/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt index 1dbe95eba2144..20c13465ec07c 100644 --- a/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt +++ b/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt @@ -102,6 +102,20 @@ SELECT date + 1 * INTERVAL '1' DAY FROM test GROUP BY 1; "missing_bucket":true,"value_type":"long","order":"asc"}}}]}}} ; +GroupByVersion +SELECT version, count(*) FROM test GROUP BY version; +{"terms":{"field":"version" +; + +GroupByVersionScript +SELECT CONCAT('1.', version::string)::version v, count(*) FROM test GROUP BY v; +{"terms":{"script":{"source":"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.concat(params.v0,InternalSqlScriptUtils.cast(InternalQlScriptUtils.docValue(doc,params.v1),params.v2)),params.v3)","lang":"painless","params":{"v0":"1.","v1":"version","v2":"KEYWORD","v3":"VERSION"}} +; + +OrderByVersionScript +SELECT CONCAT('1.', version::string)::version v FROM test ORDER BY v; +{"script":{"source":"InternalQlScriptUtils.nullSafeSortVersion(InternalSqlScriptUtils.cast(InternalSqlScriptUtils.concat(params.v0,InternalSqlScriptUtils.cast(InternalQlScriptUtils.docValue(doc,params.v1),params.v2)),params.v3))","lang":"painless","params":{"v0":"1.","v1":"version","v2":"KEYWORD","v3":"VERSION"}} +; // Having ///////// From 9e52c2f5c0f24f997e862e9213154979360169d7 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Tue, 17 May 2022 16:06:25 +0200 Subject: [PATCH 13/20] Fix based on review suggestions --- .../ql/expression/gen/script/Scripts.java | 2 +- .../operator/comparison/Comparisons.java | 16 +-- .../ql/type/DataTypeConversionTests.java | 33 ++---- .../xpack/sql/qa/jdbc/DataLoader.java | 22 +++- .../sql/qa/server/src/main/resources/apps.csv | 3 + .../multi-cluster-command-sys.csv-spec | 4 +- .../single-node-only/command-sys.csv-spec | 4 +- .../src/main/resources/version.csv-spec | 112 ++++++++++++++---- .../sql/type/SqlDataTypeConverterTests.java | 15 +-- 9 files changed, 145 insertions(+), 66 deletions(-) diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java index 89fcddda9f254..2fe9314f7821e 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java @@ -81,7 +81,7 @@ public static ScriptTemplate nullSafeFilter(ScriptTemplate script) { public static ScriptTemplate nullSafeSort(ScriptTemplate script) { String methodName; - if (script.outputType().name().equals("VERSION")) { + if (script.outputType() == DataTypes.VERSION) { methodName = "nullSafeSortVersion"; } else if (script.outputType().isNumeric()) { methodName = "nullSafeSortNumeric"; diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java index a0ad875cc4945..889f073fc6413 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/operator/comparison/Comparisons.java @@ -73,21 +73,21 @@ static Integer compare(Object l, Object r) { return null; } // typical number comparison - if (l instanceof Number && r instanceof Number) { - return compare((Number) l, (Number) r); + if (l instanceof Number lN && r instanceof Number rN) { + return compare(lN, rN); } // automatic conversion for versions - if (l instanceof Version && r instanceof String) { - return ((Version) l).compareTo(new Version((String) r)); + if (l instanceof Version lV && r instanceof String rStr) { + return lV.compareTo(new Version(rStr)); } - if (l instanceof String && r instanceof Version) { - return new Version((String) l).compareTo((Version) r); + if (l instanceof String lStr && r instanceof Version rV) { + return new Version(lStr).compareTo(rV); } - if (l instanceof Comparable && r instanceof Comparable) { + if (l instanceof Comparable lC && r instanceof Comparable) { try { - return Integer.valueOf(((Comparable) l).compareTo(r)); + return Integer.valueOf(lC.compareTo(r)); } catch (ClassCastException cce) { // when types are not compatible, cce is thrown // fall back to null diff --git a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java index 32e57a22da823..3d439acc04776 100644 --- a/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java +++ b/x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/type/DataTypeConversionTests.java @@ -566,17 +566,8 @@ public void testIpToString() { assertEquals("10.0.0.1", ipToString.convert(stringToIp.convert(new Literal(s, "10.0.0.1", KEYWORD)))); } - public void testKeywordToVersion() { - Converter conversion = converterFor(KEYWORD, VERSION); - assertNull(conversion.convert(null)); - assertEquals(new Version("2.1.4").toString(), conversion.convert("2.1.4").toString()); - assertEquals(new Version("2.1.4").toBytesRef(), ((Version) conversion.convert("2.1.4")).toBytesRef()); - assertEquals(new Version("2.1.4-SNAPSHOT").toString(), conversion.convert("2.1.4-SNAPSHOT").toString()); - assertEquals(new Version("2.1.4-SNAPSHOT").toBytesRef(), ((Version) conversion.convert("2.1.4-SNAPSHOT")).toBytesRef()); - } - public void testStringToVersion() { - Converter conversion = converterFor(TEXT, VERSION); + Converter conversion = converterFor(randomFrom(TEXT, KEYWORD), VERSION); assertNull(conversion.convert(null)); assertEquals(new Version("2.1.4").toString(), conversion.convert("2.1.4").toString()); assertEquals(new Version("2.1.4").toBytesRef(), ((Version) conversion.convert("2.1.4")).toBytesRef()); @@ -584,25 +575,15 @@ public void testStringToVersion() { assertEquals(new Version("2.1.4-SNAPSHOT").toBytesRef(), ((Version) conversion.convert("2.1.4-SNAPSHOT")).toBytesRef()); } - public void testVersionToKeyword() { - Source s = new Source(Location.EMPTY, "2.1.4"); - Source s2 = new Source(Location.EMPTY, "2.1.4"); - Converter versionToString = converterFor(VERSION, KEYWORD); - assertEquals("2.1.4", versionToString.convert(new Literal(s, "2.1.4", VERSION))); - assertEquals("2.1.4-SNAPSHOT", versionToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); - Converter stringToIp = converterFor(KEYWORD, VERSION); - assertEquals("2.1.4", versionToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); - assertEquals("2.1.4-SNAPSHOT", versionToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); - } - public void testVersionToString() { Source s = new Source(Location.EMPTY, "2.1.4"); - Source s2 = new Source(Location.EMPTY, "2.1.4"); - Converter versionToString = converterFor(VERSION, TEXT); + Source s2 = new Source(Location.EMPTY, "2.1.4-SNAPSHOT"); + DataType stringType = randomFrom(TEXT, KEYWORD); + Converter versionToString = converterFor(VERSION, stringType); assertEquals("2.1.4", versionToString.convert(new Literal(s, "2.1.4", VERSION))); assertEquals("2.1.4-SNAPSHOT", versionToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); - Converter stringToIp = converterFor(KEYWORD, TEXT); - assertEquals("2.1.4", versionToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); - assertEquals("2.1.4-SNAPSHOT", versionToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); + Converter stringToVersion = converterFor(stringType, VERSION); + assertEquals("2.1.4", versionToString.convert(stringToVersion.convert(new Literal(s, "2.1.4", stringType)))); + assertEquals("2.1.4-SNAPSHOT", versionToString.convert(stringToVersion.convert(new Literal(s2, "2.1.4-SNAPSHOT", stringType)))); } } diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java index a4abc1d64782e..cb0a8692ba34e 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/DataLoader.java @@ -282,8 +282,26 @@ protected static void loadAppsDatasetIntoEs(RestClient client, String index, Str createIndex.startObject("properties"); { createIndex.startObject("id").field("type", "integer").endObject(); - createIndex.startObject("version").field("type", "version").endObject(); - createIndex.startObject("name").field("type", "text").endObject(); + createIndex.startObject("version"); + { + createIndex.field("type", "version"); + createIndex.startObject("fields"); + { + createIndex.startObject("raw").field("type", "keyword").endObject(); + } + createIndex.endObject(); + } + createIndex.endObject(); + createIndex.startObject("name"); + { + createIndex.field("type", "text"); + createIndex.startObject("fields"); + { + createIndex.startObject("raw").field("type", "keyword").endObject(); + } + createIndex.endObject(); + } + createIndex.endObject(); } createIndex.endObject(); } diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv index 2f97d416e3735..9bd4123424f02 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv +++ b/x-pack/plugin/sql/qa/server/src/main/resources/apps.csv @@ -10,3 +10,6 @@ id,version,name 9,bad,iiiii 10,5.2.9,jjjjj 11,,kkkkk +12,1.2.3.4,aaaaa +13,,lllll +14,5.2.9,mmmmm diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec index 58829440f874f..b77b82d198392 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec @@ -123,7 +123,9 @@ SYS COLUMNS CATALOG 'my_remote_cluster' TABLE LIKE '%'; -----------------+---------------+-------------------+------------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+----------------+-----------------+----------------+---------------+---------------+---------------+---------------+----------------+----------------+------------------ my_remote_cluster|null |apps |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |1 |YES |null |null |null |null |NO |NO my_remote_cluster|null |apps |name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |2 |YES |null |null |null |null |NO |NO -my_remote_cluster|null |apps |version |12 |VERSION |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |null |3 |YES |null |null |null |null |NO |NO +my_remote_cluster|null |apps |name.raw |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |3 |YES |null |null |null |null |NO |NO +my_remote_cluster|null |apps |version |12 |VERSION |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |null |4 |YES |null |null |null |null |NO |NO +my_remote_cluster|null |apps |version.raw |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |5 |YES |null |null |null |null |NO |NO my_remote_cluster|null |logs |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO my_remote_cluster|null |logs |bytes_in |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |2 |YES |null |null |null |null |NO |NO my_remote_cluster|null |logs |bytes_out |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec index 5574f6fc17da6..72fc63590bace 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/single-node-only/command-sys.csv-spec @@ -104,7 +104,9 @@ SYS COLUMNS TABLE LIKE '%'; --------------------+---------------+-------------------+------------------+---------------+----------------+--------------+---------------+---------------+---------------+---------------+---------------+---------------+---------------+----------------+-----------------+----------------+---------------+---------------+---------------+---------------+----------------+----------------+------------------ javaRestTest |null |apps |id |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |1 |YES |null |null |null |null |NO |NO javaRestTest |null |apps |name |12 |TEXT |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |2 |YES |null |null |null |null |NO |NO -javaRestTest |null |apps |version |12 |VERSION |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |null |3 |YES |null |null |null |null |NO |NO +javaRestTest |null |apps |name.raw |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |3 |YES |null |null |null |null |NO |NO +javaRestTest |null |apps |version |12 |VERSION |2147483647 |2147483647 |null |null |1 |null |null |12 |0 |null |4 |YES |null |null |null |null |NO |NO +javaRestTest |null |apps |version.raw |12 |KEYWORD |32766 |2147483647 |null |null |1 |null |null |12 |0 |2147483647 |5 |YES |null |null |null |null |NO |NO javaRestTest |null |logs |@timestamp |93 |DATETIME |34 |8 |null |null |1 |null |null |9 |3 |null |1 |YES |null |null |null |null |NO |NO javaRestTest |null |logs |bytes_in |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |2 |YES |null |null |null |null |NO |NO javaRestTest |null |logs |bytes_out |4 |INTEGER |11 |4 |null |10 |1 |null |null |4 |0 |null |3 |YES |null |null |null |null |NO |NO diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec index 16099c9bbca72..a1be1fefd5a8d 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec @@ -20,6 +20,9 @@ SELECT * FROM apps ORDER BY id; 9 |iiiii |bad 10 |jjjjj |5.2.9 11 |kkkkk |null +12 |aaaaa |1.2.3.4 +13 |lllll |null +14 |mmmmm |5.2.9 ; @@ -31,6 +34,15 @@ SELECT * FROM apps WHERE version = '2.12.0'; 4 |ddddd |2.12.0 ; +keywordSubfield +SELECT version.raw as k FROM apps WHERE version = '2.12.0'; + + k +--------- + 2.12.0 +; + + projectionVersion SELECT version FROM apps WHERE id = 3; @@ -51,6 +63,7 @@ SELECT * FROM apps WHERE version > '2.2' ORDER BY id; 7 |ggggg |5.2.9-SNAPSHOT 9 |iiiii |bad 10 |jjjjj |5.2.9 +14 |mmmmm |5.2.9 ; @@ -65,6 +78,7 @@ SELECT * FROM apps WHERE version >= '2.3.4' ORDER BY id; 7 |ggggg |5.2.9-SNAPSHOT 9 |iiiii |bad 10 |jjjjj |5.2.9 +14 |mmmmm |5.2.9 ; @@ -81,6 +95,7 @@ SELECT * FROM apps WHERE version BETWEEN '1.10' AND '5.2.9' ORDER BY id; 6 |fffff | 5.2.9 7 |ggggg | 5.2.9-SNAPSHOT 10 |jjjjj | 5.2.9 +14 |mmmmm | 5.2.9 ; @@ -93,7 +108,6 @@ SELECT * FROM apps WHERE version::string RLIKE '2.*.4'; ; - orderByVersion SELECT * FROM apps ORDER BY version, id; @@ -101,6 +115,7 @@ SELECT * FROM apps ORDER BY version, id; ------+-----------+--------------- 1 |aaaaa |1 8 |hhhhh |1.2.3.4 +12 |aaaaa |1.2.3.4 5 |eeeee |1.11.0 2 |bbbbb |2.1 3 |ccccc |2.3.4 @@ -108,8 +123,10 @@ SELECT * FROM apps ORDER BY version, id; 7 |ggggg |5.2.9-SNAPSHOT 6 |fffff |5.2.9 10 |jjjjj |5.2.9 +14 |mmmmm |5.2.9 9 |iiiii |bad 11 |kkkkk |null +13 |lllll |null ; @@ -119,20 +136,46 @@ SELECT * FROM apps ORDER BY version DESC, id ASC; id:i | name:s | version:s ------+-----------+--------------- 11 |kkkkk |null +13 |lllll |null 9 |iiiii |bad 6 |fffff |5.2.9 10 |jjjjj |5.2.9 +14 |mmmmm |5.2.9 7 |ggggg |5.2.9-SNAPSHOT 4 |ddddd |2.12.0 3 |ccccc |2.3.4 2 |bbbbb |2.1 5 |eeeee |1.11.0 8 |hhhhh |1.2.3.4 +12 |aaaaa |1.2.3.4 1 |aaaaa |1 ; +orderByVersionNullsFirst +SELECT * FROM apps ORDER BY version NULLS FIRST, id; + + id:i | name:s | version:s +------+-----------+--------------- +11 |kkkkk |null +13 |lllll |null +1 |aaaaa |1 +8 |hhhhh |1.2.3.4 +12 |aaaaa |1.2.3.4 +5 |eeeee |1.11.0 +2 |bbbbb |2.1 +3 |ccccc |2.3.4 +4 |ddddd |2.12.0 +7 |ggggg |5.2.9-SNAPSHOT +6 |fffff |5.2.9 +10 |jjjjj |5.2.9 +14 |mmmmm |5.2.9 +9 |iiiii |bad + +; + + orderByVersionScript SELECT * FROM apps ORDER BY CONCAT('1.', CAST(version AS TEXT))::version, id; @@ -140,6 +183,7 @@ SELECT * FROM apps ORDER BY CONCAT('1.', CAST(version AS TEXT))::version, id; ------+-----------+--------------- 1 |aaaaa |1 8 |hhhhh |1.2.3.4 +12 |aaaaa |1.2.3.4 5 |eeeee |1.11.0 2 |bbbbb |2.1 3 |ccccc |2.3.4 @@ -147,26 +191,49 @@ SELECT * FROM apps ORDER BY CONCAT('1.', CAST(version AS TEXT))::version, id; 7 |ggggg |5.2.9-SNAPSHOT 6 |fffff |5.2.9 10 |jjjjj |5.2.9 +14 |mmmmm |5.2.9 11 |kkkkk |null +13 |lllll |null 9 |iiiii |bad ; +countVersion +SELECT count(version) as v, name.raw as k FROM apps GROUP by k ORDER BY k; + +v:l | k:s +--------+-------- +2 | aaaaa +1 | bbbbb +1 | ccccc +1 | ddddd +1 | eeeee +1 | fffff +1 | ggggg +1 | hhhhh +1 | iiiii +1 | jjjjj +0 | kkkkk +0 | lllll +1 | mmmmm + +; + groupByVersion -SELECT max(id) as id, version FROM apps GROUP BY version ORDER BY version; - - id:i | version:s -------+----------- -11 |null -1 |1 -8 |1.2.3.4 -5 |1.11.0 -2 |2.1 -3 |2.3.4 -4 |2.12.0 -7 |5.2.9-SNAPSHOT -10 |5.2.9 -9 |bad +SELECT count(*) as c, max(id) as maxid, version FROM apps GROUP BY version ORDER BY version; + +c:l | maxid:i | version:s +----+---------+-----------------+ +2 |13 |null +1 |1 |1 +2 |12 |1.2.3.4 +1 |5 |1.11.0 +1 |2 |2.1 +1 |3 |2.3.4 +1 |4 |2.12.0 +1 |7 |5.2.9-SNAPSHOT +3 |14 |5.2.9 +1 |9 |bad ; @@ -192,10 +259,10 @@ SELECT max(id) as id FROM apps GROUP BY CONCAT('1.', CAST(version AS TEXT))::ver 4 5 7 -8 9 -10 -11 +12 +13 +14 ; castToVersion @@ -214,6 +281,9 @@ SELECT id, name, CONCAT('1.', CAST(version AS TEXT))::version version FROM apps 9 |iiiii |1.bad 10 |jjjjj |1.5.2.9 11 |kkkkk |1. +12 |aaaaa |1.1.2.3.4 +13 |lllll |1. +14 |mmmmm |1.5.2.9 ; @@ -267,11 +337,11 @@ SELECT * FROM apps WHERE '1.2.0' < '1.11.0'::version ORDER BY id LIMIT 1; ; groupByVersionHaving -SELECT max(id) as idx, version FROM apps GROUP BY version HAVING idx = 10; +SELECT max(id) as idx, version FROM apps GROUP BY version HAVING idx = 14; idx:i | version:s -------+----------- -10 | 5.2.9 +14 | 5.2.9 ; @@ -287,8 +357,10 @@ FROM apps WHERE version IS NULL OR version_text LIKE '1%' ORDER BY version DESC v:s | version:s | version_text:s | id:i | m:i | g:s | i:s | c:s ------------+-----------+-----------------+-------+-------+--------+---------+--------- 1231.11.0 | 1.11.0 | 1.11.0 | 5 | 1 | 1.11.0 | 1.11.0 | high + 1231.2.3.4 | 1.2.3.4 | 1.2.3.4 | 12 | 1 | 1.3.0 | 1.2.3.4 | high 1231.2.3.4 | 1.2.3.4 | 1.2.3.4 | 8 | 1 | 1.3.0 | 1.2.3.4 | high 1231 | 1 | 1 | 1 | 0 | 1.3.0 | 1 | low + 123 | null | null | 13 | 0 | 1.3.0 | 0.1 | none 123 | null | null | 11 | 0 | 1.3.0 | 0.1 | none ; diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java index 5b3ddcd2fb159..405ddbf4779fb 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/SqlDataTypeConverterTests.java @@ -777,7 +777,7 @@ public void testIpToString() { } public void testStringToVersion() { - Converter conversion = converterFor(KEYWORD, VERSION); + Converter conversion = converterFor(randomFrom(KEYWORD, TEXT), VERSION); assertNull(conversion.convert(null)); assertEquals(new Version("2.1.4").toString(), conversion.convert("2.1.4").toString()); assertEquals(new Version("2.1.4").toBytesRef(), ((Version) conversion.convert("2.1.4")).toBytesRef()); @@ -788,12 +788,13 @@ public void testStringToVersion() { public void testVersionToString() { Source s = new Source(Location.EMPTY, "2.1.4"); Source s2 = new Source(Location.EMPTY, "2.1.4-SNAPSHOT"); - Converter stringToString = converterFor(VERSION, KEYWORD); - assertEquals("2.1.4", stringToString.convert(new Literal(s, "2.1.4", VERSION))); - assertEquals("2.1.4-SNAPSHOT", stringToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); - Converter stringToIp = converterFor(KEYWORD, VERSION); - assertEquals("2.1.4", stringToString.convert(stringToIp.convert(new Literal(s, "2.1.4", KEYWORD)))); - assertEquals("2.1.4-SNAPSHOT", stringToString.convert(stringToIp.convert(new Literal(s2, "2.1.4-SNAPSHOT", KEYWORD)))); + final DataType stringType = randomFrom(KEYWORD, TEXT); + Converter versionToString = converterFor(VERSION, stringType); + assertEquals("2.1.4", versionToString.convert(new Literal(s, "2.1.4", VERSION))); + assertEquals("2.1.4-SNAPSHOT", versionToString.convert(new Literal(s2, "2.1.4-SNAPSHOT", VERSION))); + Converter stringToVersion = converterFor(stringType, VERSION); + assertEquals("2.1.4", versionToString.convert(stringToVersion.convert(new Literal(s, "2.1.4", stringType)))); + assertEquals("2.1.4-SNAPSHOT", versionToString.convert(stringToVersion.convert(new Literal(s2, "2.1.4-SNAPSHOT", stringType)))); } private DataType randomInterval() { From 4384010037cb79c9efc769fc1f32c9e9fbead1be Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Fri, 20 May 2022 11:16:43 +0200 Subject: [PATCH 14/20] Add further fixes based on reviews - better type checks (avoid unpredictable automatic casts) - support multiple casts - add further test cases --- .../whitelist/InternalQlScriptUtils.java | 12 ----- .../ql/expression/gen/script/Scripts.java | 4 +- .../src/main/resources/version.csv-spec | 54 +++++++++++++++++++ .../search/extractor/FieldHitExtractor.java | 4 ++ .../xpack/sql/type/SqlDataTypes.java | 4 ++ .../xpack/sql/plugin/sql_whitelist.txt | 1 - .../extractor/FieldHitExtractorTests.java | 15 ++++++ .../sql/planner/querytranslator_tests.txt | 2 +- 8 files changed, 79 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java index 00c24a0f46774..25a6575dc7411 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/function/scalar/whitelist/InternalQlScriptUtils.java @@ -18,7 +18,6 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.InProcessor; import org.elasticsearch.xpack.ql.expression.predicate.regex.RegexProcessor.RegexOperation; import org.elasticsearch.xpack.ql.util.StringUtils; -import org.elasticsearch.xpack.versionfield.Version; import java.util.List; import java.util.Map; @@ -56,17 +55,6 @@ public static String nullSafeSortString(Object sort) { return sort == null ? StringUtils.EMPTY : sort.toString(); } - public static String nullSafeSortVersion(Object sort) { - if (sort instanceof String) { - return (String) sort; - } - Version version = (Version) sort; - if (version == null) { - return StringUtils.EMPTY; - } - return version.toString(); - } - public static Number nullSafeCastNumeric(Number number, String typeName) { return number == null || Double.isNaN(number.doubleValue()) ? null : (Number) convert(number, fromTypeName(typeName)); } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java index 2fe9314f7821e..81cc5426ce597 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java @@ -81,9 +81,7 @@ public static ScriptTemplate nullSafeFilter(ScriptTemplate script) { public static ScriptTemplate nullSafeSort(ScriptTemplate script) { String methodName; - if (script.outputType() == DataTypes.VERSION) { - methodName = "nullSafeSortVersion"; - } else if (script.outputType().isNumeric()) { + if (script.outputType().isNumeric()) { methodName = "nullSafeSortNumeric"; } else { methodName = "nullSafeSortString"; diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec index a1be1fefd5a8d..0276fd3a2b185 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/version.csv-spec @@ -298,6 +298,52 @@ SELECT '1.2.3'::version as v; ; +castConstantToVersion2 +select '1.2.3'::version v from apps; + + v +------ +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 + +; + + +multipleCast +select '1.2.3'::version::string v from apps; + + v +------ +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 +1.2.3 + +; + + compareVersions SELECT '1.2.3'::version < '1.11.4'::version as v; @@ -365,3 +411,11 @@ FROM apps WHERE version IS NULL OR version_text LIKE '1%' ORDER BY version DESC ; +selectFirstLastVersion +select first(version) as first, last(version) as last from apps; + +first:s | last:s +---------+------------ +1 | bad + +; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index 54187aa0ffde2..e95a814db4feb 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -27,6 +27,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.sql.type.SqlDataTypeConverter.convert; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_POINT; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_SHAPE; @@ -133,6 +134,9 @@ protected Object unwrapCustomValue(Object values) { // since its later processing will be type dependent. (ex.: negation of UL is only "safe" for 0 values) return convert(values, UNSIGNED_LONG); } + if (dataType == VERSION && values instanceof String) { + return convert(values, VERSION); + } return null; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java index 25eb1c09a5aaf..061cdc41a17ae 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/SqlDataTypes.java @@ -12,6 +12,7 @@ import org.elasticsearch.xpack.sql.expression.literal.geo.GeoShape; import org.elasticsearch.xpack.sql.expression.literal.interval.Interval; import org.elasticsearch.xpack.sql.expression.literal.interval.Intervals; +import org.elasticsearch.xpack.versionfield.Version; import java.sql.JDBCType; import java.sql.SQLType; @@ -230,6 +231,9 @@ public static DataType fromJava(Object value) { if (value instanceof GeoShape) { return GEO_SHAPE; } + if (value instanceof Version) { + return VERSION; + } return null; } diff --git a/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt b/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt index 0ecec66de978e..018d9757e3ceb 100644 --- a/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt +++ b/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt @@ -29,7 +29,6 @@ class org.elasticsearch.xpack.ql.expression.function.scalar.whitelist.InternalQl boolean nullSafeFilter(Boolean) double nullSafeSortNumeric(Number) String nullSafeSortString(Object) - String nullSafeSortVersion(Object) # # ASCII Functions diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java index 9edb79211cec9..15615c70b0442 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.xpack.sql.proto.StringUtils; import org.elasticsearch.xpack.sql.type.SqlDataTypes; import org.elasticsearch.xpack.sql.util.DateUtils; +import org.elasticsearch.xpack.versionfield.Version; import java.math.BigDecimal; import java.math.BigInteger; @@ -35,6 +36,7 @@ import static org.elasticsearch.common.time.DateUtils.toMilliSeconds; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; +import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.GEO_SHAPE; import static org.elasticsearch.xpack.sql.type.SqlDataTypes.SHAPE; import static org.elasticsearch.xpack.sql.util.DateUtils.UTC; @@ -220,6 +222,19 @@ public void testUnsignedLongExtraction() { assertEquals(bi, fe.extract(hit)); } + public void testVersionExtraction() { + Version version = new Version(randomAlphaOfLength(10)); + + Object value = randomBoolean() ? version.toString() : version; + + String fieldName = randomAlphaOfLength(10); + DocumentField field = new DocumentField(fieldName, singletonList(value)); + SearchHit hit = new SearchHit(1, null, singletonMap(fieldName, field), null); + FieldHitExtractor fe = new FieldHitExtractor(fieldName, VERSION, randomZone(), randomBoolean()); + + assertEquals(version.toString(), fe.extract(hit).toString()); + } + private FieldHitExtractor getFieldHitExtractor(String fieldName) { return new FieldHitExtractor(fieldName, null, UTC); } diff --git a/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt b/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt index 20c13465ec07c..6f122ec9d443a 100644 --- a/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt +++ b/x-pack/plugin/sql/src/test/resources/org/elasticsearch/xpack/sql/planner/querytranslator_tests.txt @@ -114,7 +114,7 @@ SELECT CONCAT('1.', version::string)::version v, count(*) FROM test GROUP BY v; OrderByVersionScript SELECT CONCAT('1.', version::string)::version v FROM test ORDER BY v; -{"script":{"source":"InternalQlScriptUtils.nullSafeSortVersion(InternalSqlScriptUtils.cast(InternalSqlScriptUtils.concat(params.v0,InternalSqlScriptUtils.cast(InternalQlScriptUtils.docValue(doc,params.v1),params.v2)),params.v3))","lang":"painless","params":{"v0":"1.","v1":"version","v2":"KEYWORD","v3":"VERSION"}} +{"script":{"source":"InternalQlScriptUtils.nullSafeSortString(InternalSqlScriptUtils.cast(InternalSqlScriptUtils.concat(params.v0,InternalSqlScriptUtils.cast(InternalQlScriptUtils.docValue(doc,params.v1),params.v2)),params.v3))","lang":"painless","params":{"v0":"1.","v1":"version","v2":"KEYWORD","v3":"VERSION"}} ; // Having From 4ddef7fa0af29ce97ea5b931ea93bf1ac60c9296 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Thu, 26 May 2022 12:48:31 +0200 Subject: [PATCH 15/20] Update version for bwc checks (to v 8.4.0) --- .../xpack/ql/index/VersionCompatibilityChecks.java | 4 ++-- .../org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java | 4 ++-- .../elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java index fcd414784af3a..e65087c224e2e 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java @@ -12,14 +12,14 @@ import org.elasticsearch.xpack.ql.type.DataType; import static org.elasticsearch.Version.V_8_2_0; -import static org.elasticsearch.Version.V_8_3_0; +import static org.elasticsearch.Version.V_8_4_0; import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; import static org.elasticsearch.xpack.ql.type.DataTypes.VERSION; public final class VersionCompatibilityChecks { public static final Version INTRODUCING_UNSIGNED_LONG = V_8_2_0; - public static final Version INTRODUCING_VERSION_FIELD_TYPE = V_8_3_0; + public static final Version INTRODUCING_VERSION_FIELD_TYPE = V_8_4_0; private VersionCompatibilityChecks() {} diff --git a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java index a4c0b0e1dcbb1..fb5b69a053f13 100644 --- a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java +++ b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java @@ -27,7 +27,7 @@ import java.util.Map; import static org.elasticsearch.Version.V_8_2_0; -import static org.elasticsearch.Version.V_8_3_0; +import static org.elasticsearch.Version.V_8_4_0; import static org.elasticsearch.common.time.DateUtils.toMilliSeconds; import static org.elasticsearch.test.ESTestCase.randomLongBetween; @@ -158,7 +158,7 @@ public static boolean isUnsignedLongSupported() { } public static boolean isVersionFieldTypeSupported() { - return JDBC_DRIVER_VERSION.onOrAfter(V_8_3_0); + return JDBC_DRIVER_VERSION.onOrAfter(V_8_4_0); } } diff --git a/x-pack/plugin/sql/qa/mixed-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java b/x-pack/plugin/sql/qa/mixed-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java index 0d1d9a083cd5b..f09875a660b98 100644 --- a/x-pack/plugin/sql/qa/mixed-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java +++ b/x-pack/plugin/sql/qa/mixed-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/mixed_node/SqlSearchIT.java @@ -40,7 +40,7 @@ import static org.elasticsearch.xpack.ql.TestUtils.readResource; public class SqlSearchIT extends ESRestTestCase { - private static final Version VERSION_FIELD_QL_INTRODUCTION = Version.V_8_3_0; + private static final Version VERSION_FIELD_QL_INTRODUCTION = Version.V_8_4_0; private static final String index = "test_sql_mixed_versions"; private static int numShards; private static int numReplicas = 1; From d8b5473ba92bff6479d4f78653c70d32dd7e0935 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 1 Jun 2022 15:59:23 +0200 Subject: [PATCH 16/20] Implement review suggestions --- .../qa/common/src/main/resources/data/extra.data | 2 ++ .../qa/common/src/main/resources/test_extra.toml | 11 +++++++++++ .../xpack/ql/expression/gen/script/Scripts.java | 7 +------ .../xpack/ql/index/VersionCompatibilityChecks.java | 4 ++-- .../org/elasticsearch/xpack/sql/jdbc/EsType.java | 4 ++-- .../xpack/sql/qa/jdbc/ResultSetTestCase.java | 13 ++++--------- .../search/extractor/FieldHitExtractor.java | 2 +- .../sql/analysis/analyzer/FieldAttributeTests.java | 2 +- .../sql/plan/logical/command/ShowColumnsTests.java | 6 ++---- .../plan/logical/command/sys/SysColumnsTests.java | 2 +- .../sql/plan/logical/command/sys/SysTypesTests.java | 2 +- 11 files changed, 28 insertions(+), 27 deletions(-) diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data b/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data index 36f979336be06..d87ed34992cd8 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data +++ b/x-pack/plugin/eql/qa/common/src/main/resources/data/extra.data @@ -112,6 +112,7 @@ "transID": 2, "process.entity_id": 512, "process.pid": 123, + "version": "1.5.0", "sequence": 18 }, { @@ -120,6 +121,7 @@ "transID": 0, "process.entity_id": 512, "process.pid": 123, + "version": "1.5.0", "sequence": 19 }, { diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml b/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml index a95d9f8593047..14352f067b43d 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml +++ b/x-pack/plugin/eql/qa/common/src/main/resources/test_extra.toml @@ -260,3 +260,14 @@ query = ''' ''' expected_event_ids = [20, 22] join_keys = ["0"] + + +[[queries]] +name = "sequenceWithVersionJoinKey" +query = ''' + sequence by version + [ process where true ] + [ file where true ] +''' +expected_event_ids = [18, 19] +join_keys = ["1.5.0"] diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java index 81cc5426ce597..fb8664a733db5 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/gen/script/Scripts.java @@ -80,12 +80,7 @@ public static ScriptTemplate nullSafeFilter(ScriptTemplate script) { } public static ScriptTemplate nullSafeSort(ScriptTemplate script) { - String methodName; - if (script.outputType().isNumeric()) { - methodName = "nullSafeSortNumeric"; - } else { - methodName = "nullSafeSortString"; - } + String methodName = script.outputType().isNumeric() ? "nullSafeSortNumeric" : "nullSafeSortString"; return new ScriptTemplate( formatTemplate(format(Locale.ROOT, "{ql}.%s(%s)", methodName, script.template())), script.params(), diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java index e65087c224e2e..0a285b9e43d3d 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/VersionCompatibilityChecks.java @@ -28,7 +28,7 @@ public static boolean isTypeSupportedInVersion(DataType dataType, Version versio return supportsUnsignedLong(version); } if (dataType == VERSION) { - return supportsVersionFieldType(version); + return supportsVersionType(version); } return true; } @@ -43,7 +43,7 @@ public static boolean supportsUnsignedLong(Version version) { /** * Does the provided {@code version} support the version type (PR#85502)? */ - public static boolean supportsVersionFieldType(Version version) { + public static boolean supportsVersionType(Version version) { return INTRODUCING_VERSION_FIELD_TYPE.compareTo(version) <= 0; } diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java index cb31cbb4956b8..810d4a3ced8b4 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/EsType.java @@ -32,7 +32,6 @@ public enum EsType implements SQLType { TIME(Types.TIME), DATETIME(Types.TIMESTAMP), IP(Types.VARCHAR), - VERSION(Types.VARCHAR), INTERVAL_YEAR(ExtraTypes.INTERVAL_YEAR), INTERVAL_MONTH(ExtraTypes.INTERVAL_MONTH), INTERVAL_YEAR_TO_MONTH(ExtraTypes.INTERVAL_YEAR_MONTH), @@ -49,7 +48,8 @@ public enum EsType implements SQLType { GEO_POINT(ExtraTypes.GEOMETRY), GEO_SHAPE(ExtraTypes.GEOMETRY), SHAPE(ExtraTypes.GEOMETRY), - UNSIGNED_LONG(Types.NUMERIC); + UNSIGNED_LONG(Types.NUMERIC), + VERSION(Types.VARCHAR); private final Integer type; diff --git a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java index d66230642a485..fae53e6d214a6 100644 --- a/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java +++ b/x-pack/plugin/sql/qa/jdbc/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java @@ -2249,24 +2249,19 @@ public void testSingleVersionFieldValue() throws SQLException, IOException { assumeTrue("Driver version [" + JDBC_DRIVER_VERSION + "] doesn't support VERSION fields", isVersionFieldTypeSupported()); createTestDataForVersionType(); - String query = "SELECT name, version from test where version = '1.3.0'"; + String query = "SELECT name, version FROM test WHERE version = '1.3.0' OR version = 'foo' ORDER BY version ASC"; doWithQuery(query, results -> { - results.next(); + assertTrue(results.next()); assertEquals("version 1.3.0", results.getString("name")); assertEquals("1.3.0", results.getString("version")); SQLException sqle = expectThrows(SQLException.class, () -> results.getByte("version")); assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VERSION] to [Byte]", "1.3.0"), sqle.getMessage()); - assertFalse(results.next()); - }); - - // bad version value - query = "SELECT name, version from test where version = 'foo'"; - doWithQuery(query, results -> { - results.next(); + assertTrue(results.next()); assertEquals("version foo", results.getString("name")); assertEquals("foo", results.getString("version")); assertFalse(results.next()); }); + } private void doWithQuery(String query, CheckedConsumer consumer) throws SQLException { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index e95a814db4feb..67b430d5ba141 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -134,7 +134,7 @@ protected Object unwrapCustomValue(Object values) { // since its later processing will be type dependent. (ex.: negation of UL is only "safe" for 0 values) return convert(values, UNSIGNED_LONG); } - if (dataType == VERSION && values instanceof String) { + if (dataType == VERSION) { return convert(values, VERSION); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java index c1e6249a6e1c4..fa7151e1f8751 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/FieldAttributeTests.java @@ -351,7 +351,7 @@ public void testUnsignedLongVersionCompatibility() { } } - public void testVersionFieldVersionCompatibility() { + public void testVersionTypeVersionCompatibility() { String query = "SELECT version_number FROM test"; String queryWithCastLiteral = "SELECT '1.2.3'::version AS version_number"; String queryWithAlias = "SELECT version_number AS version_number FROM test"; diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java index 1f4eb1d800121..8e0edc54e6fbf 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumnsTests.java @@ -20,7 +20,7 @@ import static java.util.Arrays.asList; import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.supportsUnsignedLong; -import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.supportsVersionFieldType; +import static org.elasticsearch.xpack.ql.index.VersionCompatibilityChecks.supportsVersionType; import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN; import static org.elasticsearch.xpack.ql.type.DataTypes.DATETIME; import static org.elasticsearch.xpack.ql.type.DataTypes.FLOAT; @@ -107,9 +107,7 @@ public void testVersionFieldFiltering() { // mapping's mutated by IndexCompatibility.compatible, needs to stay in the loop Map mapping = loadMapping("mapping-multi-field-variation.json", true); ShowColumns.fillInRows(IndexCompatibility.compatible(mapping, Version.fromId(version.id)), null, rows); - assertTrue( - (supportsVersionFieldType(Version.fromId(version.id)) && rows.contains(rowSupported)) || rows.contains(rowUnsupported) - ); + assertTrue((supportsVersionType(Version.fromId(version.id)) && rows.contains(rowSupported)) || rows.contains(rowUnsupported)); } } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java index 5b4ea97e4ec11..fe903ff455710 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java @@ -179,7 +179,7 @@ public void testUnsignedLongFiltering() { } } - public void testVersionFieldFiltering() { + public void testVersionTypeFiltering() { for (Mode mode : List.of(Mode.JDBC, Mode.ODBC)) { for (SqlVersion version : VERSION_FIELD_TEST_VERSIONS) { List> rows = new ArrayList<>(); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java index 61b804e036af0..7a6481983615b 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypesTests.java @@ -163,7 +163,7 @@ public void testUnsignedLongFiltering() { } } - public void testVersionFieldFiltering() { + public void testVersionTypeFiltering() { Set versions = new HashSet<>(VERSION_FIELD_TEST_VERSIONS); versions.add(null); for (SqlVersion version : versions) { From e31a21f3fae0cef4f6ec0131b12efe4ca1e52fff Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 8 Jun 2022 12:41:22 +0200 Subject: [PATCH 17/20] Revert unnescessary changes to mapper-version --- .../org/elasticsearch/xpack/versionfield/VersionEncoder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java b/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java index cffbd04ef1da5..56558ac3b608f 100644 --- a/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java +++ b/x-pack/plugin/mapper-version/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java @@ -35,7 +35,7 @@ * lexically in ASCII sort order. Numeric identifiers always have lower precedence than non-numeric identifiers. * */ -public class VersionEncoder { +class VersionEncoder { public static final byte NUMERIC_MARKER_BYTE = (byte) 0x01; public static final byte PRERELEASE_SEPARATOR_BYTE = (byte) 0x02; @@ -175,7 +175,7 @@ static boolean legalVersionString(VersionParts versionParts) { return legalMainVersion && legalPreRelease && legalBuildSuffix; } - public static class EncodedVersion { + static class EncodedVersion { public final boolean isLegal; public final boolean isPreRelease; From a92defd2915eebc9f0daf99e6e356d18454a5a0d Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 8 Jun 2022 13:07:26 +0200 Subject: [PATCH 18/20] Implement review suggestions --- .../java/org/elasticsearch/xpack/ql/type/DataType.java | 9 +++------ .../elasticsearch/xpack/sql/action/SqlQueryResponse.java | 6 +++--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java index 55ee0c5faedad..833035fcf49ed 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/type/DataType.java @@ -65,12 +65,9 @@ public String esType() { } public ScriptSortBuilder.ScriptSortType scriptSortType() { - if (isNumeric()) { - return ScriptSortBuilder.ScriptSortType.NUMBER; - } else if (this == DataTypes.VERSION) { - return ScriptSortBuilder.ScriptSortType.VERSION; - } - return ScriptSortBuilder.ScriptSortType.STRING; + return isNumeric() ? ScriptSortBuilder.ScriptSortType.NUMBER + : this == DataTypes.VERSION ? ScriptSortBuilder.ScriptSortType.VERSION + : ScriptSortBuilder.ScriptSortType.STRING; } public boolean isInteger() { diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java index c6ffbbc99f307..7119953829368 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java @@ -274,9 +274,7 @@ private static XContentBuilder toXContent(ColumnInfo info, XContentBuilder build * Serializes the provided value in SQL-compatible way based on the client mode */ public static XContentBuilder value(XContentBuilder builder, Mode mode, SqlVersion sqlVersion, Object value) throws IOException { - if (value instanceof org.elasticsearch.xpack.versionfield.Version) { - builder.value(value.toString()); - } else if (value instanceof ZonedDateTime zdt) { + if (value instanceof ZonedDateTime zdt) { // use the ISO format if (mode == JDBC && isClientCompatible(SqlVersion.fromId(CURRENT.id), sqlVersion)) { builder.value(StringUtils.toString(zdt, sqlVersion)); @@ -287,6 +285,8 @@ public static XContentBuilder value(XContentBuilder builder, Mode mode, SqlVersi // use the SQL format for intervals when sending back the response for CLI // all other clients will receive ISO 8601 formatted intervals builder.value(value.toString()); + } else if (value instanceof org.elasticsearch.xpack.versionfield.Version) { + builder.value(value.toString()); } else { builder.value(value); } From 89aa38175b2d89a7c33fa5f5abae62bfe8f5dca8 Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 8 Jun 2022 16:24:15 +0200 Subject: [PATCH 19/20] Fix compile problems after merge from master --- .../xpack/sql/qa/FieldExtractorTestCase.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java index 5880ee83404c4..12dfb173ca37e 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java @@ -366,24 +366,15 @@ public void testIpField() throws IOException { public void testVersionField() throws IOException { String query = "SELECT version_field FROM test"; String actualValue = "2.11.4"; - boolean explicitSourceSetting = randomBoolean(); // default (no _source setting) or explicit setting - boolean enableSource = randomBoolean(); // enable _source at index level - - Map indexProps = Maps.newMapWithExpectedSize(1); - indexProps.put("_source", enableSource); Map> fieldProps = null; - createIndexWithFieldTypeAndProperties("version", fieldProps, explicitSourceSetting ? indexProps : null); + createIndexWithFieldTypeAndProperties("version", fieldProps, getIndexProps()); index("{\"version_field\":\"" + actualValue + "\"}"); - if (explicitSourceSetting == false || enableSource) { - Map expected = new HashMap<>(); - expected.put("columns", asList(columnInfo("plain", "version_field", "version", JDBCType.VARCHAR, Integer.MAX_VALUE))); - expected.put("rows", singletonList(singletonList(actualValue))); - assertResponse(expected, runSql(query)); - } else { - expectSourceDisabledError(query); - } + Map expected = new HashMap<>(); + expected.put("columns", asList(columnInfo("plain", "version_field", "version", JDBCType.VARCHAR, Integer.MAX_VALUE))); + expected.put("rows", singletonList(singletonList(actualValue))); + assertResponse(expected, runSql(query)); } /* From dd12a672860a20c96481a118b90bef98749576dd Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Wed, 8 Jun 2022 18:46:53 +0200 Subject: [PATCH 20/20] Fix failing test case (due to refactoring on master) --- .../org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java index 12dfb173ca37e..d3841c79b46cc 100644 --- a/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java +++ b/x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/FieldExtractorTestCase.java @@ -373,7 +373,7 @@ public void testVersionField() throws IOException { Map expected = new HashMap<>(); expected.put("columns", asList(columnInfo("plain", "version_field", "version", JDBCType.VARCHAR, Integer.MAX_VALUE))); - expected.put("rows", singletonList(singletonList(actualValue))); + expected.put("rows", singletonList(singletonList(getExpectedValueFromSource(actualValue)))); assertResponse(expected, runSql(query)); }