From 3430deb57514a4c593813e8735d62484756c840d Mon Sep 17 00:00:00 2001 From: Taylor Curran Date: Thu, 19 Dec 2024 09:19:20 -0800 Subject: [PATCH] #3145 Add IP Address Data Type (#3175) * Add `ExprIpValue` and `IP` data type Signed-off-by: currantw * Add support for casting (`cast(field_name to ip)`) and remove existing unused sorting syntax. Signed-off-by: currantw * Update comparison logic to compare in IPv6 Signed-off-by: currantw * Fix bug casting to IP Signed-off-by: currantw * Fix failing tests Signed-off-by: currantw * Assert that comparison only valid if same type, update tests accordingly Signed-off-by: currantw * Add additional tests to increase code coverage Signed-off-by: currantw * Integrate `cidrmatch` changes Signed-off-by: currantw * Remove `OpenSearchIPType` data type Signed-off-by: currantw * Fix more failing tests Signed-off-by: currantw * Minor cleanup Signed-off-by: currantw * Add new tests for IP data type to `SortCommandIT`, and update `weblogs` test data. Signed-off-by: currantw * Fixing IT test failure. Signed-off-by: currantw * Spotless and update test to sort in SQL Signed-off-by: currantw * Fix broken link Signed-off-by: currantw * Fix failing code coverage Signed-off-by: currantw * Fix failing doctest Signed-off-by: currantw * Fix failing `ip.rst` doctest Signed-off-by: currantw * Fix test failure due to merge. Signed-off-by: currantw * Fix spotless Signed-off-by: currantw * Add missing `url` field Signed-off-by: currantw * Address minor review comments. Signed-off-by: currantw * Revert sort syntax changes Signed-off-by: currantw * Minor doc update Signed-off-by: currantw * FIx failing `ip.rst` doctest Signed-off-by: currantw * Add `IPComparisonIT` tests for comparison operators, rename modules and weblogs test index to make plural for consistency. Signed-off-by: currantw --------- Signed-off-by: currantw --- DEVELOPER_GUIDE.rst | 2 +- .../opensearch/sql/ast/expression/Cast.java | 2 + .../sql/data/model/ExprIpValue.java | 50 ++++++ .../opensearch/sql/data/model/ExprValue.java | 6 + .../sql/data/model/ExprValueUtils.java | 9 ++ .../sql/data/type/ExprCoreType.java | 3 + .../org/opensearch/sql/expression/DSL.java | 4 + .../function/BuiltinFunctionName.java | 1 + .../sql/expression/ip/IPFunctions.java | 69 ++------- .../operator/convert/TypeCastOperators.java | 10 ++ .../org/opensearch/sql/utils/IPUtils.java | 97 ++++++++++++ .../opensearch/sql/analysis/AnalyzerTest.java | 2 +- .../sql/data/model/ExprIpValueTest.java | 138 +++++++++++++++++ .../sql/data/model/ExprValueUtilsTest.java | 7 +- .../function/WideningTypeRuleTest.java | 2 + .../sql/expression/ip/IPFunctionTest.java | 120 --------------- .../sql/expression/ip/IPFunctionsTest.java | 93 +++++++++++ .../convert/TypeCastOperatorTest.java | 64 +++++++- docs/user/ppl/cmd/trendline.rst | 2 +- docs/user/ppl/functions/ip.rst | 24 +-- doctest/test_data/weblogs.json | 12 +- doctest/test_mapping/weblogs.json | 21 +++ .../org/opensearch/sql/legacy/JdbcTestIT.java | 5 +- .../sql/legacy/RestIntegTestCase.java | 4 +- .../sql/legacy/SQLIntegTestCase.java | 4 +- .../opensearch/sql/legacy/TestsConstants.java | 2 +- .../opensearch/sql/ppl/IPComparisonIT.java | 145 ++++++++++++++++++ .../{IPFunctionIT.java => IPFunctionsIT.java} | 27 ++-- .../org/opensearch/sql/ppl/SortCommandIT.java | 16 ++ .../weblogs_index_mapping.json | 5 +- integ-test/src/test/resources/weblogs.json | 12 +- .../data/type/OpenSearchDataType.java | 4 +- .../data/type/OpenSearchIpType.java | 34 ---- .../data/value/OpenSearchExprIpValue.java | 48 ------ .../value/OpenSearchExprValueFactory.java | 25 ++- .../script/filter/lucene/LuceneQuery.java | 6 + .../OpenSearchDataTypeRecognitionTest.java | 2 - .../data/type/OpenSearchDataTypeTest.java | 7 +- .../data/value/OpenSearchExprIpValueTest.java | 44 ------ .../value/OpenSearchExprValueFactoryTest.java | 40 +++-- .../script/filter/FilterQueryBuilderTest.java | 24 +++ ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 5 +- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 4 +- .../sql/ppl/parser/AstExpressionBuilder.java | 2 + 44 files changed, 803 insertions(+), 400 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java create mode 100644 core/src/main/java/org/opensearch/sql/utils/IPUtils.java create mode 100644 core/src/test/java/org/opensearch/sql/data/model/ExprIpValueTest.java delete mode 100644 core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java create mode 100644 core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionsTest.java create mode 100644 doctest/test_mapping/weblogs.json create mode 100644 integ-test/src/test/java/org/opensearch/sql/ppl/IPComparisonIT.java rename integ-test/src/test/java/org/opensearch/sql/ppl/{IPFunctionIT.java => IPFunctionsIT.java} (53%) delete mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchIpType.java delete mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValue.java diff --git a/DEVELOPER_GUIDE.rst b/DEVELOPER_GUIDE.rst index c0d2f85668..ec00c587a6 100644 --- a/DEVELOPER_GUIDE.rst +++ b/DEVELOPER_GUIDE.rst @@ -405,7 +405,7 @@ Sample test class: Doctest >>>>>>> -Python doctest library makes our document executable which keeps it up-to-date to source code. The doc generator aforementioned served as scaffolding and generated many docs in short time. Now the examples inside is changed to doctest gradually. For more details please read `Doctest <./dev/Doctest.md>`_. +Python doctest library makes our document executable which keeps it up-to-date to source code. The doc generator aforementioned served as scaffolding and generated many docs in short time. Now the examples inside is changed to doctest gradually. For more details please read `testing-doctest <./docs/dev/testing-doctest.md>`_. Backports diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/Cast.java b/core/src/main/java/org/opensearch/sql/ast/expression/Cast.java index 2019346fb5..541dbedead 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/Cast.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/Cast.java @@ -12,6 +12,7 @@ import static org.opensearch.sql.expression.function.BuiltinFunctionName.CAST_TO_DOUBLE; import static org.opensearch.sql.expression.function.BuiltinFunctionName.CAST_TO_FLOAT; import static org.opensearch.sql.expression.function.BuiltinFunctionName.CAST_TO_INT; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.CAST_TO_IP; import static org.opensearch.sql.expression.function.BuiltinFunctionName.CAST_TO_LONG; import static org.opensearch.sql.expression.function.BuiltinFunctionName.CAST_TO_SHORT; import static org.opensearch.sql.expression.function.BuiltinFunctionName.CAST_TO_STRING; @@ -54,6 +55,7 @@ public class Cast extends UnresolvedExpression { .put("time", CAST_TO_TIME.getName()) .put("timestamp", CAST_TO_TIMESTAMP.getName()) .put("datetime", CAST_TO_DATETIME.getName()) + .put("ip", CAST_TO_IP.getName()) .build(); /** The source expression cast from. */ diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java new file mode 100644 index 0000000000..8bdbec4bb5 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java @@ -0,0 +1,50 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.data.model; + +import inet.ipaddr.IPAddress; +import org.opensearch.sql.data.type.ExprCoreType; +import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.utils.IPUtils; + +/** Expression IP Address Value. */ +public class ExprIpValue extends AbstractExprValue { + private final IPAddress value; + + public ExprIpValue(String addressString) { + value = IPUtils.toAddress(addressString); + } + + @Override + public String value() { + return value.toCanonicalString(); + } + + @Override + public ExprType type() { + return ExprCoreType.IP; + } + + @Override + public int compare(ExprValue other) { + return IPUtils.compare(value, ((ExprIpValue) other).value); + } + + @Override + public boolean equal(ExprValue other) { + return compare(other) == 0; + } + + @Override + public String toString() { + return String.format("IP %s", value()); + } + + @Override + public IPAddress ipValue() { + return value; + } +} diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprValue.java index 034ed22a75..da9c329f93 100644 --- a/core/src/main/java/org/opensearch/sql/data/model/ExprValue.java +++ b/core/src/main/java/org/opensearch/sql/data/model/ExprValue.java @@ -5,6 +5,7 @@ package org.opensearch.sql.data.model; +import inet.ipaddr.IPAddress; import java.io.Serializable; import java.time.Instant; import java.time.LocalDate; @@ -102,6 +103,11 @@ default Double doubleValue() { "invalid to get doubleValue from value of type " + type()); } + /** Get IP address value. */ + default IPAddress ipValue() { + throw new ExpressionEvaluationException("invalid to get ipValue from value of type " + type()); + } + /** Get string value. */ default String stringValue() { throw new ExpressionEvaluationException( diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java b/core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java index 20813045f2..890e0ef8d5 100644 --- a/core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java +++ b/core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java @@ -5,6 +5,7 @@ package org.opensearch.sql.data.model; +import inet.ipaddr.IPAddress; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; @@ -75,6 +76,10 @@ public static ExprValue timestampValue(Instant value) { return new ExprTimestampValue(value); } + public static ExprValue ipValue(String value) { + return new ExprIpValue(value); + } + /** {@link ExprTupleValue} constructor. */ public static ExprValue tupleValue(Map map) { LinkedHashMap valueMap = new LinkedHashMap<>(); @@ -188,6 +193,10 @@ public static Map getTupleValue(ExprValue exprValue) { return exprValue.tupleValue(); } + public static IPAddress getIpValue(ExprValue exprValue) { + return exprValue.ipValue(); + } + public static Boolean getBooleanValue(ExprValue exprValue) { return exprValue.booleanValue(); } diff --git a/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java b/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java index cbc0c98255..6df2ba6390 100644 --- a/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java +++ b/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java @@ -45,6 +45,9 @@ public enum ExprCoreType implements ExprType { TIMESTAMP(STRING, DATE, TIME), INTERVAL(UNDEFINED), + /** IP Address. */ + IP(STRING), + /** Struct. */ STRUCT(UNDEFINED), diff --git a/core/src/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java index 54bd35e70f..44ecc2bc86 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -835,6 +835,10 @@ public static FunctionExpression castTimestamp(Expression value) { return compile(FunctionProperties.None, BuiltinFunctionName.CAST_TO_TIMESTAMP, value); } + public static FunctionExpression castIp(Expression value) { + return compile(FunctionProperties.None, BuiltinFunctionName.CAST_TO_IP, value); + } + public static FunctionExpression typeof(Expression value) { return compile(FunctionProperties.None, BuiltinFunctionName.TYPEOF, value); } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java index a67308c96a..f8e9cf7c5f 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java @@ -231,6 +231,7 @@ public enum BuiltinFunctionName { CAST_TO_TIME(FunctionName.of("cast_to_time")), CAST_TO_TIMESTAMP(FunctionName.of("cast_to_timestamp")), CAST_TO_DATETIME(FunctionName.of("cast_to_datetime")), + CAST_TO_IP(FunctionName.of("cast_to_ip")), TYPEOF(FunctionName.of("typeof")), /** Relevance Function. */ diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java index b3e7fad211..8b3ee23014 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java @@ -6,14 +6,13 @@ package org.opensearch.sql.expression.ip; import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.expression.function.FunctionDSL.define; import static org.opensearch.sql.expression.function.FunctionDSL.impl; import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling; -import inet.ipaddr.AddressStringException; -import inet.ipaddr.IPAddressString; -import inet.ipaddr.IPAddressStringParameters; +import inet.ipaddr.IPAddress; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; @@ -21,6 +20,7 @@ import org.opensearch.sql.expression.function.BuiltinFunctionName; import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.DefaultFunctionResolver; +import org.opensearch.sql.utils.IPUtils; /** Utility class that defines and registers IP functions. */ @UtilityClass @@ -31,20 +31,17 @@ public void register(BuiltinFunctionRepository repository) { } private DefaultFunctionResolver cidrmatch() { - - // TODO #3145: Add support for IP address data type. return define( BuiltinFunctionName.CIDRMATCH.getName(), - impl(nullMissingHandling(IPFunctions::exprCidrMatch), BOOLEAN, STRING, STRING)); + impl(nullMissingHandling(IPFunctions::exprCidrMatch), BOOLEAN, IP, STRING)); } /** * Returns whether the given IP address is within the specified inclusive CIDR IP address range. * Supports both IPv4 and IPv6 addresses. * - * @param addressExprValue IP address as a string (e.g. "198.51.100.14" or - * "2001:0db8::ff00:42:8329"). - * @param rangeExprValue IP address range in CIDR notation as a string (e.g. "198.51.100.0/24" or + * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @param rangeExprValue IP address range string in CIDR notation (e.g. "198.51.100.0/24" or * "2001:0db8::/32") * @return true if the address is in the range; otherwise false. * @throws SemanticCheckException if the address or range is not valid, or if they do not use the @@ -52,54 +49,12 @@ private DefaultFunctionResolver cidrmatch() { */ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprValue) { - // TODO #3145: Update to support IP address data type. - String addressString = addressExprValue.stringValue(); - String rangeString = rangeExprValue.stringValue(); - - final IPAddressStringParameters validationOptions = - new IPAddressStringParameters.Builder() - .allowEmpty(false) - .setEmptyAsLoopback(false) - .allow_inet_aton(false) - .allowSingleSegment(false) - .toParams(); - - // Get and validate IP address. - IPAddressString address = - new IPAddressString(addressExprValue.stringValue(), validationOptions); - - try { - address.validate(); - } catch (AddressStringException e) { - String msg = - String.format( - "IP address '%s' is not valid. Error details: %s", addressString, e.getMessage()); - throw new SemanticCheckException(msg, e); - } - - // Get and validate CIDR IP address range. - IPAddressString range = new IPAddressString(rangeExprValue.stringValue(), validationOptions); - - try { - range.validate(); - } catch (AddressStringException e) { - String msg = - String.format( - "CIDR IP address range '%s' is not valid. Error details: %s", - rangeString, e.getMessage()); - throw new SemanticCheckException(msg, e); - } - - // Address and range must use the same IP version (IPv4 or IPv6). - if (address.isIPv4() ^ range.isIPv4()) { - String msg = - String.format( - "IP address '%s' and CIDR IP address range '%s' are not compatible. Both must be" - + " either IPv4 or IPv6.", - addressString, rangeString); - throw new SemanticCheckException(msg); - } + IPAddress address = addressExprValue.ipValue(); + IPAddress range = IPUtils.toRange(rangeExprValue.stringValue()); - return ExprValueUtils.booleanValue(range.contains(address)); + return (IPUtils.compare(address, range.getLower()) < 0) + || (IPUtils.compare(address, range.getUpper()) > 0) + ? ExprValueUtils.LITERAL_FALSE + : ExprValueUtils.LITERAL_TRUE; } } diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java b/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java index 55e223d94c..b388f7d89a 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java @@ -11,6 +11,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.SHORT; import static org.opensearch.sql.data.type.ExprCoreType.STRING; @@ -31,6 +32,7 @@ import org.opensearch.sql.data.model.ExprDoubleValue; import org.opensearch.sql.data.model.ExprFloatValue; import org.opensearch.sql.data.model.ExprIntegerValue; +import org.opensearch.sql.data.model.ExprIpValue; import org.opensearch.sql.data.model.ExprLongValue; import org.opensearch.sql.data.model.ExprShortValue; import org.opensearch.sql.data.model.ExprStringValue; @@ -54,6 +56,7 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(castToFloat()); repository.register(castToDouble()); repository.register(castToBoolean()); + repository.register(castToIp()); repository.register(castToDate()); repository.register(castToTime()); repository.register(castToTimestamp()); @@ -173,6 +176,13 @@ private static DefaultFunctionResolver castToBoolean() { impl(nullMissingHandling((v) -> v), BOOLEAN, BOOLEAN)); } + private static DefaultFunctionResolver castToIp() { + return FunctionDSL.define( + BuiltinFunctionName.CAST_TO_IP.getName(), + impl(nullMissingHandling((v) -> new ExprIpValue(v.stringValue())), IP, STRING), + impl(nullMissingHandling((v) -> v), IP, IP)); + } + private static DefaultFunctionResolver castToDate() { return FunctionDSL.define( BuiltinFunctionName.CAST_TO_DATE.getName(), diff --git a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java new file mode 100644 index 0000000000..8874823a03 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java @@ -0,0 +1,97 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.utils; + +import inet.ipaddr.AddressStringException; +import inet.ipaddr.IPAddress; +import inet.ipaddr.IPAddressString; +import inet.ipaddr.IPAddressStringParameters; +import inet.ipaddr.ipv4.IPv4Address; +import inet.ipaddr.ipv6.IPv6Address; +import lombok.experimental.UtilityClass; +import org.opensearch.sql.exception.SemanticCheckException; + +@UtilityClass +public class IPUtils { + + // Parameters for IP address strings. + private static final IPAddressStringParameters.Builder commonValidationOptions = + new IPAddressStringParameters.Builder() + .allowEmpty(false) + .allowMask(false) + .setEmptyAsLoopback(false) + .allowPrefixOnly(false) + .allow_inet_aton(false) + .allowSingleSegment(false); + + private static final IPAddressStringParameters ipAddressStringParameters = + commonValidationOptions.allowPrefix(false).toParams(); + private static final IPAddressStringParameters ipAddressRangeStringParameters = + commonValidationOptions.allowPrefix(true).toParams(); + + /** + * Builds and returns the {@link IPAddress} represented by the given IP address range string in + * CIDR (classless inter-domain routing) notation. Throws {@link SemanticCheckException} if it + * does not represent a valid IP address range. Supports both IPv4 and IPv6 address ranges. + */ + public static IPAddress toRange(String s) throws SemanticCheckException { + try { + IPAddress range = new IPAddressString(s, ipAddressRangeStringParameters).toAddress(); + + // Convert IPv6 mapped address range to IPv4. + if (range.isIPv4Convertible()) { + final int prefixLength = range.getPrefixLength(); + range = range.toIPv4().setPrefixLength(prefixLength, false); + } + + return range; + + } catch (AddressStringException e) { + final String errorFormat = "IP address range string '%s' is not valid. Error details: %s"; + throw new SemanticCheckException(String.format(errorFormat, s, e.getMessage()), e); + } + } + + /** + * Builds and returns the {@link IPAddress} represented to the given IP address string. Throws + * {@link SemanticCheckException} if it does not represent a valid IP address. Supports both IPv4 + * and IPv6 addresses. + */ + public static IPAddress toAddress(String s) throws SemanticCheckException { + try { + IPAddress address = new IPAddressString(s, ipAddressStringParameters).toAddress(); + + // Convert IPv6 mapped address to IPv4. + if (address.isIPv4Convertible()) { + address = address.toIPv4(); + } + + return address; + } catch (AddressStringException e) { + final String errorFormat = "IP address string '%s' is not valid. Error details: %s"; + throw new SemanticCheckException(String.format(errorFormat, s, e.getMessage()), e); + } + } + + /** + * Compares the given {@link IPAddress} objects for order. Returns a negative integer, zero, or a + * positive integer if the first {@link IPAddress} object is less than, equal to, or greater than + * the second one. IPv4 addresses are mapped to IPv6 for comparison. + */ + public static int compare(IPAddress a, IPAddress b) { + final IPv6Address ipv6A = toIPv6Address(a); + final IPv6Address ipv6B = toIPv6Address(b); + + return ipv6A.compareTo(ipv6B); + } + + /** Returns the {@link IPv6Address} corresponding to the given {@link IPAddress}. */ + private static IPv6Address toIPv6Address(IPAddress ipAddress) { + return ipAddress instanceof IPv4Address iPv4Address + ? iPv4Address.toIPv6() + : (IPv6Address) ipAddress; + } +} diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java index d6cb0544d8..3f4752aa2e 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -163,7 +163,7 @@ public void filter_relation_with_invalid_qualifiedName_ExpressionEvaluationExcep assertEquals( "= function expected {[BYTE,BYTE],[SHORT,SHORT],[INTEGER,INTEGER],[LONG,LONG]," + "[FLOAT,FLOAT],[DOUBLE,DOUBLE],[STRING,STRING],[BOOLEAN,BOOLEAN],[DATE,DATE]," - + "[TIME,TIME],[TIMESTAMP,TIMESTAMP],[INTERVAL,INTERVAL]," + + "[TIME,TIME],[TIMESTAMP,TIMESTAMP],[INTERVAL,INTERVAL],[IP,IP]," + "[STRUCT,STRUCT],[ARRAY,ARRAY]}, but got [STRING,INTEGER]", exception.getMessage()); } diff --git a/core/src/test/java/org/opensearch/sql/data/model/ExprIpValueTest.java b/core/src/test/java/org/opensearch/sql/data/model/ExprIpValueTest.java new file mode 100644 index 0000000000..b0ef598a5a --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/data/model/ExprIpValueTest.java @@ -0,0 +1,138 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.data.model; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.data.type.ExprCoreType; +import org.opensearch.sql.exception.ExpressionEvaluationException; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.utils.IPUtils; + +public class ExprIpValueTest { + + private static final String ipv4String = "1.2.3.4"; + private static final String ipv6String = "2001:db7::ff00:42:8329"; + private static final String ipInvalidString = "INVALID"; + + private static final ExprValue exprIpv4Value = ExprValueUtils.ipValue(ipv4String); + private static final ExprValue exprIpv6Value = ExprValueUtils.ipValue(ipv6String); + + private static final List ipv4LesserStrings = + List.of("1.2.3.3", "01.2.3.3", "::ffff:1.2.3.3", "::ffff:102:303"); + private static final List ipv4EqualStrings = + List.of("1.2.3.4", "01.2.3.4", "::ffff:1.2.3.4", "::ffff:102:304"); + private static final List ipv4GreaterStrings = + List.of("1.2.3.5", "01.2.3.5", "::ffff:1.2.3.5", "::ffff:102:305"); + + private static final List ipv6LesserStrings = + List.of( + "2001:db7::ff00:42:8328", + "2001:0db7::ff00:0042:8328", + "2001:DB7::FF00:42:8328", + "2001:0db7:0000:0000:0000:ff00:0042:8328"); + private static final List ipv6EqualStrings = + List.of( + "2001:db7::ff00:42:8329", + "2001:0db7::ff00:0042:8329", + "2001:DB7::FF00:42:8329", + "2001:0db7:0000:0000:0000:ff00:0042:8329"); + private static final List ipv6GreaterStrings = + List.of( + "2001:db7::ff00:42:8330", + "2001:0db7::ff00:0042:8330", + "2001:DB7::FF00:42:8330", + "2001:0db7:0000:0000:0000:ff00:0042:8330"); + + @Test + public void testInvalid() { + assertThrows( + SemanticCheckException.class, + () -> ExprValueUtils.ipValue(ipInvalidString), + String.format("IP address string '%s' is not valid. Error details: .*", ipInvalidString)); + } + + @Test + public void testValue() { + ipv4EqualStrings.forEach((s) -> assertEquals(ipv4String, ExprValueUtils.ipValue(s).value())); + ipv6EqualStrings.forEach((s) -> assertEquals(ipv6String, ExprValueUtils.ipValue(s).value())); + } + + @Test + public void testType() { + assertEquals(ExprCoreType.IP, exprIpv4Value.type()); + assertEquals(ExprCoreType.IP, exprIpv6Value.type()); + } + + @Test + public void testCompare() { + + // Compare to IP address. + ipv4LesserStrings.forEach( + (s) -> assertTrue(exprIpv4Value.compareTo(ExprValueUtils.ipValue(s)) > 0)); + ipv4EqualStrings.forEach( + (s) -> assertEquals(0, exprIpv4Value.compareTo(ExprValueUtils.ipValue(s)))); + ipv4GreaterStrings.forEach( + (s) -> assertTrue(exprIpv4Value.compareTo(ExprValueUtils.ipValue(s)) < 0)); + ipv6LesserStrings.forEach( + (s) -> assertTrue(exprIpv6Value.compareTo(ExprValueUtils.ipValue(s)) > 0)); + ipv6EqualStrings.forEach( + (s) -> assertEquals(0, exprIpv6Value.compareTo(ExprValueUtils.ipValue(s)))); + ipv6GreaterStrings.forEach( + (s) -> assertTrue(exprIpv6Value.compareTo(ExprValueUtils.ipValue(s)) < 0)); + + // Compare to null/missing value. + assertThrows( + IllegalStateException.class, + () -> exprIpv4Value.compareTo(ExprValueUtils.LITERAL_NULL), + "[BUG] Unreachable, Comparing with NULL or MISSING is undefined"); + assertThrows( + IllegalStateException.class, + () -> exprIpv4Value.compareTo(ExprValueUtils.LITERAL_MISSING), + "[BUG] Unreachable, Comparing with NULL or MISSING is undefined"); + + // Compare to other data type. + assertThrows( + ExpressionEvaluationException.class, + () -> exprIpv4Value.compareTo(ExprValueUtils.LITERAL_TRUE), + "compare expected value have same type, but with [IP, BOOLEAN]"); + } + + @Test + public void testEquals() { + assertEquals(exprIpv4Value, exprIpv4Value); + assertNotEquals(exprIpv4Value, new Object()); + assertNotEquals(exprIpv4Value, ExprValueUtils.LITERAL_NULL); + assertNotEquals(exprIpv4Value, ExprValueUtils.LITERAL_MISSING); + + ipv4EqualStrings.forEach((s) -> assertEquals(exprIpv4Value, ExprValueUtils.ipValue(s))); + ipv6EqualStrings.forEach((s) -> assertEquals(exprIpv6Value, ExprValueUtils.ipValue(s))); + + ipv4LesserStrings.forEach((s) -> assertNotEquals(exprIpv4Value, ExprValueUtils.ipValue(s))); + ipv6GreaterStrings.forEach((s) -> assertNotEquals(exprIpv6Value, ExprValueUtils.ipValue(s))); + } + + @Test + public void testToString() { + ipv4EqualStrings.forEach( + (s) -> + assertEquals(String.format("IP %s", ipv4String), ExprValueUtils.ipValue(s).toString())); + ipv6EqualStrings.forEach( + (s) -> + assertEquals(String.format("IP %s", ipv6String), ExprValueUtils.ipValue(s).toString())); + } + + @Test + public void testIpValue() { + ipv4EqualStrings.forEach((s) -> assertEquals(IPUtils.toAddress(s), exprIpv4Value.ipValue())); + ipv6EqualStrings.forEach((s) -> assertEquals(IPUtils.toAddress(s), exprIpv6Value.ipValue())); + } +} diff --git a/core/src/test/java/org/opensearch/sql/data/model/ExprValueUtilsTest.java b/core/src/test/java/org/opensearch/sql/data/model/ExprValueUtilsTest.java index 9fe6347102..48db530a94 100644 --- a/core/src/test/java/org/opensearch/sql/data/model/ExprValueUtilsTest.java +++ b/core/src/test/java/org/opensearch/sql/data/model/ExprValueUtilsTest.java @@ -14,6 +14,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.INTERVAL; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.data.type.ExprCoreType.STRUCT; import static org.opensearch.sql.data.type.ExprCoreType.TIME; @@ -47,6 +48,7 @@ import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.storage.bindingtuple.BindingTuple; +import org.opensearch.sql.utils.IPUtils; @DisplayName("Test Expression Value Utils") public class ExprValueUtilsTest { @@ -63,6 +65,7 @@ public class ExprValueUtilsTest { private static final List nonNumberValues = Arrays.asList( + new ExprIpValue("1.2.3.4"), new ExprStringValue("1"), ExprBooleanValue.of(true), new ExprCollectionValue(ImmutableList.of(new ExprIntegerValue(1))), @@ -85,6 +88,7 @@ public class ExprValueUtilsTest { ExprValueUtils::getDoubleValue); private static final List> nonNumberValueExtractor = Arrays.asList( + ExprValueUtils::getIpValue, ExprValueUtils::getStringValue, ExprValueUtils::getBooleanValue, ExprValueUtils::getCollectionValue, @@ -109,7 +113,7 @@ public class ExprValueUtilsTest { ExprCoreType.FLOAT, ExprCoreType.DOUBLE); private static final List nonNumberTypes = - Arrays.asList(STRING, BOOLEAN, ARRAY, STRUCT); + Arrays.asList(IP, STRING, BOOLEAN, ARRAY, STRUCT); private static final List dateAndTimeTypes = Arrays.asList(DATE, TIME, TIMESTAMP, INTERVAL); private static final List allTypes = @@ -124,6 +128,7 @@ private static Stream getValueTestArgumentStream() { 1L, 1f, 1D, + IPUtils.toAddress("1.2.3.4"), "1", true, Arrays.asList(integerValue(1)), diff --git a/core/src/test/java/org/opensearch/sql/expression/function/WideningTypeRuleTest.java b/core/src/test/java/org/opensearch/sql/expression/function/WideningTypeRuleTest.java index 3064ffcdee..d38be4c958 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/WideningTypeRuleTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/WideningTypeRuleTest.java @@ -13,6 +13,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.SHORT; import static org.opensearch.sql.data.type.ExprCoreType.STRING; @@ -57,6 +58,7 @@ class WideningTypeRuleTest { .put(STRING, TIMESTAMP, 1) .put(STRING, DATE, 1) .put(STRING, TIME, 1) + .put(STRING, IP, 1) .put(DATE, TIMESTAMP, 1) .put(TIME, TIMESTAMP, 1) .put(UNDEFINED, BYTE, 1) diff --git a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java deleted file mode 100644 index b50bf9fd1f..0000000000 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ /dev/null @@ -1,120 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.expression.ip; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.when; -import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; -import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.model.ExprValueUtils; -import org.opensearch.sql.exception.SemanticCheckException; -import org.opensearch.sql.expression.DSL; -import org.opensearch.sql.expression.Expression; -import org.opensearch.sql.expression.FunctionExpression; -import org.opensearch.sql.expression.env.Environment; - -@ExtendWith(MockitoExtension.class) -public class IPFunctionTest { - - // IP range and address constants for testing. - private static final ExprValue IPv4Range = ExprValueUtils.stringValue("198.51.100.0/24"); - private static final ExprValue IPv6Range = ExprValueUtils.stringValue("2001:0db8::/32"); - - // TODO #3145: Add tests for IP address data type. - private static final ExprValue IPv4AddressBelow = ExprValueUtils.stringValue("198.51.99.1"); - private static final ExprValue IPv4AddressWithin = ExprValueUtils.stringValue("198.51.100.1"); - private static final ExprValue IPv4AddressAbove = ExprValueUtils.stringValue("198.51.101.2"); - - private static final ExprValue IPv6AddressBelow = - ExprValueUtils.stringValue("2001:0db7::ff00:42:8329"); - private static final ExprValue IPv6AddressWithin = - ExprValueUtils.stringValue("2001:0db8::ff00:42:8329"); - private static final ExprValue IPv6AddressAbove = - ExprValueUtils.stringValue("2001:0db9::ff00:42:8329"); - - // Mock value environment for testing. - @Mock private Environment env; - - @Test - public void cidrmatch_invalid_address() { - SemanticCheckException exception = - assertThrows( - SemanticCheckException.class, - () -> execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); - assertTrue( - exception.getMessage().matches("IP address 'INVALID' is not valid. Error details: .*")); - } - - @Test - public void cidrmatch_invalid_range() { - SemanticCheckException exception = - assertThrows( - SemanticCheckException.class, - () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); - assertTrue( - exception - .getMessage() - .matches("CIDR IP address range 'INVALID' is not valid. Error details: .*")); - } - - @Test - public void cidrmatch_different_versions() { - SemanticCheckException exception; - - exception = - assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, IPv6Range)); - assertEquals( - "IP address '198.51.100.1' and CIDR IP address range '2001:0db8::/32' are not compatible." - + " Both must be either IPv4 or IPv6.", - exception.getMessage()); - - exception = - assertThrows(SemanticCheckException.class, () -> execute(IPv6AddressWithin, IPv4Range)); - assertEquals( - "IP address '2001:0db8::ff00:42:8329' and CIDR IP address range '198.51.100.0/24' are not" - + " compatible. Both must be either IPv4 or IPv6.", - exception.getMessage()); - } - - @Test - public void cidrmatch_valid_ipv4() { - assertEquals(LITERAL_FALSE, execute(IPv4AddressBelow, IPv4Range)); - assertEquals(LITERAL_TRUE, execute(IPv4AddressWithin, IPv4Range)); - assertEquals(LITERAL_FALSE, execute(IPv4AddressAbove, IPv4Range)); - } - - @Test - public void cidrmatch_valid_ipv6() { - assertEquals(LITERAL_FALSE, execute(IPv6AddressBelow, IPv6Range)); - assertEquals(LITERAL_TRUE, execute(IPv6AddressWithin, IPv6Range)); - assertEquals(LITERAL_FALSE, execute(IPv6AddressAbove, IPv6Range)); - } - - /** - * Builds and evaluates a CIDR function expression with the given field and range expression - * values, and returns the resulting value. - */ - private ExprValue execute(ExprValue field, ExprValue range) { - - final String fieldName = "ip_address"; - FunctionExpression exp = DSL.cidrmatch(DSL.ref(fieldName, STRING), DSL.literal(range)); - - // Mock the value environment to return the specified field - // expression as the value for the "ip_address" field. - when(DSL.ref(fieldName, STRING).valueOf(env)).thenReturn(field); - - return exp.valueOf(env); - } -} diff --git a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionsTest.java new file mode 100644 index 0000000000..a74bbda3a1 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionsTest.java @@ -0,0 +1,93 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.expression.ip; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.when; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; +import static org.opensearch.sql.data.type.ExprCoreType.IP; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.env.Environment; + +@ExtendWith(MockitoExtension.class) +public class IPFunctionsTest { + + // IP range and address constants for testing. + private static final ExprValue IPv4Range = ExprValueUtils.stringValue("198.51.100.0/24"); + private static final ExprValue IPv4RangeMapped = + ExprValueUtils.stringValue("::ffff:198.51.100.0/24"); + private static final ExprValue IPv6Range = ExprValueUtils.stringValue("2001:0db8::/32"); + + private static final ExprValue IPv4AddressBelow = ExprValueUtils.ipValue("198.51.99.1"); + private static final ExprValue IPv4AddressWithin = ExprValueUtils.ipValue("198.51.100.1"); + private static final ExprValue IPv4AddressAbove = ExprValueUtils.ipValue("198.51.101.2"); + + private static final ExprValue IPv6AddressBelow = + ExprValueUtils.ipValue("2001:0db7::ff00:42:8329"); + private static final ExprValue IPv6AddressWithin = + ExprValueUtils.ipValue("2001:0db8::ff00:42:8329"); + private static final ExprValue IPv6AddressAbove = + ExprValueUtils.ipValue("2001:0db9::ff00:42:8329"); + + // Mock value environment for testing. + @Mock private Environment env; + + @Test + public void cidrmatch_invalid_arguments() { + assertThrows( + SemanticCheckException.class, + () -> execute(ExprValueUtils.ipValue("INVALID"), IPv4Range), + "IP address string 'INVALID' is not valid. Error details: .*"); + assertThrows( + SemanticCheckException.class, + () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID")), + "IP address range string 'INVALID' is not valid. Error details: .*"); + } + + @Test + public void cidrmatch_valid_arguments() { + + assertEquals(LITERAL_FALSE, execute(IPv4AddressBelow, IPv4Range)); + assertEquals(LITERAL_TRUE, execute(IPv4AddressWithin, IPv4Range)); + assertEquals(LITERAL_FALSE, execute(IPv4AddressAbove, IPv4Range)); + + assertEquals(LITERAL_FALSE, execute(IPv4AddressBelow, IPv4RangeMapped)); + assertEquals(LITERAL_TRUE, execute(IPv4AddressWithin, IPv4RangeMapped)); + assertEquals(LITERAL_FALSE, execute(IPv4AddressAbove, IPv4RangeMapped)); + + assertEquals(LITERAL_FALSE, execute(IPv6AddressBelow, IPv6Range)); + assertEquals(LITERAL_TRUE, execute(IPv6AddressWithin, IPv6Range)); + assertEquals(LITERAL_FALSE, execute(IPv6AddressAbove, IPv6Range)); + } + + /** + * Builds and evaluates a {@code cidrmatch} function expression with the given address and range + * expression values, and returns the resulting value. + */ + private ExprValue execute(ExprValue address, ExprValue range) { + + final String fieldName = "ip_address"; + FunctionExpression exp = DSL.cidrmatch(DSL.ref(fieldName, IP), DSL.literal(range)); + + // Mock the value environment to return the specified field + // expression as the value for the "ip_address" field. + when(DSL.ref(fieldName, IP).valueOf(env)).thenReturn(address); + + return exp.valueOf(env); + } +} diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/convert/TypeCastOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/convert/TypeCastOperatorTest.java index 44a3ccabbd..fd579dfb47 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/convert/TypeCastOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/convert/TypeCastOperatorTest.java @@ -7,12 +7,14 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.opensearch.sql.data.type.ExprCoreType.BYTE; import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.SHORT; import static org.opensearch.sql.data.type.ExprCoreType.STRING; @@ -29,12 +31,17 @@ import org.opensearch.sql.data.model.ExprDoubleValue; import org.opensearch.sql.data.model.ExprFloatValue; import org.opensearch.sql.data.model.ExprIntegerValue; +import org.opensearch.sql.data.model.ExprIpValue; import org.opensearch.sql.data.model.ExprLongValue; +import org.opensearch.sql.data.model.ExprMissingValue; +import org.opensearch.sql.data.model.ExprNullValue; import org.opensearch.sql.data.model.ExprShortValue; import org.opensearch.sql.data.model.ExprStringValue; import org.opensearch.sql.data.model.ExprTimeValue; import org.opensearch.sql.data.model.ExprTimestampValue; import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.exception.ExpressionEvaluationException; +import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.FunctionExpression; @@ -316,10 +323,6 @@ void castToTime() { assertEquals(TIME, expression.type()); assertEquals(new ExprTimeValue("01:01:01"), expression.valueOf()); - expression = DSL.castTime(DSL.literal(new ExprTimestampValue("2012-08-07 01:01:01"))); - assertEquals(TIME, expression.type()); - assertEquals(new ExprTimeValue("01:01:01"), expression.valueOf()); - expression = DSL.castTime(DSL.literal(new ExprTimeValue("01:01:01"))); assertEquals(TIME, expression.type()); assertEquals(new ExprTimeValue("01:01:01"), expression.valueOf()); @@ -334,9 +337,56 @@ void castToTimestamp() { expression = DSL.castTimestamp(DSL.literal(new ExprTimestampValue("2012-08-07 01:01:01"))); assertEquals(TIMESTAMP, expression.type()); assertEquals(new ExprTimestampValue("2012-08-07 01:01:01"), expression.valueOf()); + } - expression = DSL.castTimestamp(DSL.literal(new ExprTimestampValue("2012-08-07 01:01:01"))); - assertEquals(TIMESTAMP, expression.type()); - assertEquals(new ExprTimestampValue("2012-08-07 01:01:01"), expression.valueOf()); + @Test + void castToIp() { + FunctionExpression exp; + + final String ipv4String = "1.2.3.4"; + final String ipv6String = "2001:db7::ff00:42:8329"; + final String ipInvalidString = "INVALID"; + + final ExprValue exprIpv4Value = new ExprIpValue(ipv4String); + final ExprValue exprIpv6Value = new ExprIpValue(ipv6String); + + // From string + exp = DSL.castIp(DSL.literal(ipv4String)); + assertEquals(IP, exp.type()); + assertEquals(exprIpv4Value, exp.valueOf()); + + exp = DSL.castIp(DSL.literal(ipv6String)); + assertEquals(IP, exp.type()); + assertEquals(exprIpv6Value, exp.valueOf()); + + exp = DSL.castIp(DSL.literal(ipInvalidString)); + assertThrows( + SemanticCheckException.class, + exp::valueOf, + String.format("IP address string '%s' is not valid. Error details: .*", ipInvalidString)); + + // From IP address + exp = DSL.castIp(DSL.literal(exprIpv4Value)); + assertEquals(IP, exp.type()); + assertEquals(exprIpv4Value, exp.valueOf()); + + exp = DSL.castIp(DSL.literal(exprIpv6Value)); + assertEquals(IP, exp.type()); + assertEquals(exprIpv6Value, exp.valueOf()); + + // From invalid type + assertThrows( + ExpressionEvaluationException.class, + () -> DSL.castIp(DSL.literal(0)), + "cast_to_ip function expected {[IP],[STRING]}, but got [INTEGER]"); + + // From null or missing value + exp = DSL.castIp(DSL.literal(ExprNullValue.of())); + assertEquals(IP, exp.type()); + assertTrue(exp.valueOf().isNull()); + + exp = DSL.castIp(DSL.literal(ExprMissingValue.of())); + assertEquals(IP, exp.type()); + assertTrue(exp.valueOf().isMissing()); } } diff --git a/docs/user/ppl/cmd/trendline.rst b/docs/user/ppl/cmd/trendline.rst index 166a3c056f..e6df0d7a2c 100644 --- a/docs/user/ppl/cmd/trendline.rst +++ b/docs/user/ppl/cmd/trendline.rst @@ -23,7 +23,7 @@ Syntax * field: mandatory. The name of the field the moving average should be calculated for. * alias: optional. The name of the resulting column containing the moving average (defaults to the field name with "_trendline"). -And the moment only the Simple Moving Average (SMA) type is supported. +At the moment only the Simple Moving Average (SMA) type is supported. It is calculated like diff --git a/docs/user/ppl/functions/ip.rst b/docs/user/ppl/functions/ip.rst index 3387974af5..30cb9020b0 100644 --- a/docs/user/ppl/functions/ip.rst +++ b/docs/user/ppl/functions/ip.rst @@ -20,19 +20,19 @@ Argument type: STRING, STRING Return type: BOOLEAN -Example: +Example:: - os> source=weblogs | where cidrmatch(host, '199.120.110.0/24') | fields host - fetched rows / total rows = 1/1 - +----------------+ - | host | - |----------------| - | 199.120.110.21 | - +----------------+ + > source=weblogs | where cidrmatch(host, '1.2.3.0/24') | fields host, url + fetched rows / total rows = 2/2 + +---------+--------------------+ + | host | url | + |---------|--------------------| + | 1.2.3.4 | /history/voyager1/ | + | 1.2.3.5 | /history/voyager2/ | + +---------+--------------------+ Note: - - `ip` can be an IPv4 or an IPv6 address - - `cidr` can be an IPv4 or an IPv6 block - - `ip` and `cidr` must be either both IPv4 or both IPv6 - - `ip` and `cidr` must both be valid and non-empty/non-null + - `ip` can be an IPv4 or IPv6 address + - `cidr` can be an IPv4 or IPv6 block + - `ip` and `cidr` must both be valid and non-missing/non-null diff --git a/doctest/test_data/weblogs.json b/doctest/test_data/weblogs.json index 4228e9c4d2..afb1679e22 100644 --- a/doctest/test_data/weblogs.json +++ b/doctest/test_data/weblogs.json @@ -1,6 +1,6 @@ -{"index":{}} -{"host": "199.72.81.55", "method": "GET", "url": "/history/apollo/", "response": "200", "bytes": "6245"} -{"index":{}} -{"host": "199.120.110.21", "method": "GET", "url": "/shuttle/missions/sts-73/mission-sts-73.html", "response": "200", "bytes": "4085"} -{"index":{}} -{"host": "205.212.115.106", "method": "GET", "url": "/shuttle/countdown/countdown.html", "response": "200", "bytes": "3985"} +{"host":"::1","method":"GET","url":"/history/apollo/","response":"200","bytes":"6245"} +{"host":"0.0.0.2","method":"GET","url":"/shuttle/missions/sts-73/mission-sts-73.html","response":"200","bytes":"4085"} +{"host":"::3","method":"GET","url":"/shuttle/countdown/countdown.html","response":"200","bytes":"3985"} +{"host":"::FFFF:1.2.3.4","method":"GET","url":"/history/voyager1/","response":"200","bytes":"1234"} +{"host":"1.2.3.5","method":"GET","url":"/history/voyager2/","response": "200","bytes":"4321"} +{"host":"::FFFF:1234","method":"GET","url":"/history/artemis/","response":"200","bytes": "9876"} diff --git a/doctest/test_mapping/weblogs.json b/doctest/test_mapping/weblogs.json new file mode 100644 index 0000000000..05b9784313 --- /dev/null +++ b/doctest/test_mapping/weblogs.json @@ -0,0 +1,21 @@ +{ + "mappings": { + "properties": { + "host": { + "type": "ip" + }, + "method": { + "type": "text" + }, + "url": { + "type": "text" + }, + "response": { + "type": "text" + }, + "bytes": { + "type": "text" + } + } + } +} diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java index 005119a9bc..4ad88c632b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java @@ -155,10 +155,7 @@ public void dateFunctionNameCaseInsensitiveTest() { public void ipTypeShouldPassJdbcFormatter() { assertThat( executeQuery( - "SELECT host_ip AS hostIP FROM " - + TestsConstants.TEST_INDEX_WEBLOG - + " ORDER BY hostIP", - "jdbc"), + "SELECT host FROM " + TestsConstants.TEST_INDEX_WEBLOGS + " ORDER BY host", "jdbc"), containsString("\"type\": \"ip\"")); } diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/RestIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/RestIntegTestCase.java index a94047c1e4..3d53b96668 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/RestIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/RestIntegTestCase.java @@ -273,8 +273,8 @@ public enum Index { getOrderIndexMapping(), "src/test/resources/order.json"), WEBLOG( - TestsConstants.TEST_INDEX_WEBLOG, - "weblog", + TestsConstants.TEST_INDEX_WEBLOGS, + "weblogs", getWeblogsIndexMapping(), "src/test/resources/weblogs.json"), DATE( diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 5b956fb5d3..1728be74e6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -661,8 +661,8 @@ public enum Index { getOrderIndexMapping(), "src/test/resources/order.json"), WEBLOG( - TestsConstants.TEST_INDEX_WEBLOG, - "weblog", + TestsConstants.TEST_INDEX_WEBLOGS, + "weblogs", getWeblogsIndexMapping(), "src/test/resources/weblogs.json"), DATE( diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index 73838feb4f..1e336f544e 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -43,7 +43,7 @@ public class TestsConstants { public static final String TEST_INDEX_BANK_CSV_SANITIZE = TEST_INDEX_BANK + "_csv_sanitize"; public static final String TEST_INDEX_BANK_RAW_SANITIZE = TEST_INDEX_BANK + "_raw_sanitize"; public static final String TEST_INDEX_ORDER = TEST_INDEX + "_order"; - public static final String TEST_INDEX_WEBLOG = TEST_INDEX + "_weblog"; + public static final String TEST_INDEX_WEBLOGS = TEST_INDEX + "_weblogs"; public static final String TEST_INDEX_DATE = TEST_INDEX + "_date"; public static final String TEST_INDEX_DATE_TIME = TEST_INDEX + "_datetime"; public static final String TEST_INDEX_DEEP_NESTED = TEST_INDEX + "_deep_nested"; diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPComparisonIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPComparisonIT.java new file mode 100644 index 0000000000..a19ea32a68 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPComparisonIT.java @@ -0,0 +1,145 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ppl; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOGS; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.Test; +import org.opensearch.sql.legacy.SQLIntegTestCase; + +public class IPComparisonIT extends PPLIntegTestCase { + + @Override + public void init() throws IOException { + loadIndex(SQLIntegTestCase.Index.WEBLOG); + } + + @Test + public void test_equal() throws IOException { + JSONObject result; + final String operator = "="; + + result = executeComparisonQuery(operator, "1.2.3.4"); + verifyDataRows(result, rows("1.2.3.4")); + + result = executeComparisonQuery(operator, "::ffff:1.2.3.4"); + verifyDataRows(result, rows("1.2.3.4")); + + result = executeComparisonQuery(operator, "::1"); + verifyDataRows(result, rows("::1")); + + result = executeComparisonQuery(operator, "0000:0000:0000:0000:0000:0000:0000:0001"); + verifyDataRows(result, rows("::1")); + } + + @Test + public void test_not_equal() throws IOException { + JSONObject result; + final String operator = "!="; + + result = executeComparisonQuery(operator, "1.2.3.4"); + verifyDataRows( + result, rows("::1"), rows("0.0.0.2"), rows("::3"), rows("1.2.3.5"), rows("::ffff:1234")); + + result = executeComparisonQuery(operator, "::ffff:1.2.3.4"); + verifyDataRows( + result, rows("::1"), rows("0.0.0.2"), rows("::3"), rows("1.2.3.5"), rows("::ffff:1234")); + + result = executeComparisonQuery(operator, "::1"); + verifyDataRows( + result, + rows("0.0.0.2"), + rows("::3"), + rows("1.2.3.4"), + rows("1.2.3.5"), + rows("::ffff:1234")); + + result = executeComparisonQuery(operator, "0000:0000:0000:0000:0000:0000:0000:0001"); + verifyDataRows( + result, + rows("0.0.0.2"), + rows("::3"), + rows("1.2.3.4"), + rows("1.2.3.5"), + rows("::ffff:1234")); + } + + @Test + public void test_greater_than() throws IOException { + JSONObject result; + final String operator = ">"; + + result = executeComparisonQuery(operator, "1.2.3.3"); + verifyDataRows(result, rows("1.2.3.4"), rows("1.2.3.5")); + + result = executeComparisonQuery(operator, "1.2.3.4"); + verifyDataRows(result, rows("1.2.3.5")); + + result = executeComparisonQuery(operator, "1.2.3.5"); + verifyDataRows(result); + } + + @Test + public void test_greater_than_or_equal_to() throws IOException { + JSONObject result; + final String operator = ">="; + + result = executeComparisonQuery(operator, "1.2.3.4"); + verifyDataRows(result, rows("1.2.3.4"), rows("1.2.3.5")); + + result = executeComparisonQuery(operator, "1.2.3.5"); + verifyDataRows(result, rows("1.2.3.5")); + + result = executeComparisonQuery(operator, "1.2.3.6"); + verifyDataRows(result); + } + + @Test + public void test_less_than() throws IOException { + JSONObject result; + final String operator = "<"; + + result = executeComparisonQuery(operator, "::4"); + verifyDataRows(result, rows("::1"), rows("::3")); + + result = executeComparisonQuery(operator, "::3"); + verifyDataRows(result, rows("::1")); + + result = executeComparisonQuery(operator, "::1"); + verifyDataRows(result); + } + + @Test + public void test_less_than_or_equal_to() throws IOException { + JSONObject result; + final String operator = "<="; + + result = executeComparisonQuery(operator, "::3"); + verifyDataRows(result, rows("::1"), rows("::3")); + + result = executeComparisonQuery(operator, "::1"); + verifyDataRows(result, rows("::1")); + + result = executeComparisonQuery(operator, "::0"); + verifyDataRows(result); + } + + /** + * Executes a query comparison on the weblogs test index with the given comparison operator and IP + * address string, and returns the resulting {@link JSONObject}; + */ + private JSONObject executeComparisonQuery(String comparisonOperator, String addressString) + throws IOException { + String formatString = "source=%s | where host %s '%s' | fields host"; + String query = + String.format(formatString, TEST_INDEX_WEBLOGS, comparisonOperator, addressString); + return executeQuery(query); + } +} diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionsIT.java similarity index 53% rename from integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java rename to integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionsIT.java index adb044d0d2..1b0dbf711c 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionsIT.java @@ -5,7 +5,7 @@ package org.opensearch.sql.ppl; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOG; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOGS; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; @@ -15,7 +15,7 @@ import org.json.JSONObject; import org.junit.jupiter.api.Test; -public class IPFunctionIT extends PPLIntegTestCase { +public class IPFunctionsIT extends PPLIntegTestCase { @Override public void init() throws IOException { @@ -25,34 +25,33 @@ public void init() throws IOException { @Test public void test_cidrmatch() throws IOException { - // TODO #3145: Add tests for IP address data type. JSONObject result; // No matches result = executeQuery( String.format( - "source=%s | where cidrmatch(host_string, '199.120.111.0/24') | fields host_string", - TEST_INDEX_WEBLOG)); - verifySchema(result, schema("host_string", null, "string")); + "source=%s | where cidrmatch(host, '250.0.0.0/24') | fields host", + TEST_INDEX_WEBLOGS)); + verifySchema(result, schema("host", null, "ip")); verifyDataRows(result); // One match result = executeQuery( String.format( - "source=%s | where cidrmatch(host_string, '199.120.110.0/24') | fields host_string", - TEST_INDEX_WEBLOG)); - verifySchema(result, schema("host_string", null, "string")); - verifyDataRows(result, rows("199.120.110.21")); + "source=%s | where cidrmatch(host, '0.0.0.0/24') | fields host", + TEST_INDEX_WEBLOGS)); + verifySchema(result, schema("host", null, "ip")); + verifyDataRows(result, rows("0.0.0.2")); // Multiple matches result = executeQuery( String.format( - "source=%s | where cidrmatch(host_string, '199.0.0.0/8') | fields host_string", - TEST_INDEX_WEBLOG)); - verifySchema(result, schema("host_string", null, "string")); - verifyDataRows(result, rows("199.72.81.55"), rows("199.120.110.21")); + "source=%s | where cidrmatch(host, '1.2.3.0/24') | fields host", + TEST_INDEX_WEBLOGS)); + verifySchema(result, schema("host", null, "ip")); + verifyDataRows(result, rows("1.2.3.4"), rows("1.2.3.5")); } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/SortCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/SortCommandIT.java index 1061f0bd9d..b234dd032d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/SortCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/SortCommandIT.java @@ -8,6 +8,7 @@ import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DOG; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOGS; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.verifyOrder; @@ -28,6 +29,7 @@ public void init() throws IOException { loadIndex(Index.BANK); loadIndex(Index.BANK_WITH_NULL_VALUES); loadIndex(Index.DOG); + loadIndex(Index.WEBLOG); } @Test @@ -130,6 +132,20 @@ public void testSortStringField() throws IOException { rows("Ratliff")); } + @Test + public void testSortIpField() throws IOException { + final JSONObject result = + executeQuery(String.format("source=%s | fields host | sort host", TEST_INDEX_WEBLOGS)); + verifyOrder( + result, + rows("::1"), + rows("::3"), + rows("::ffff:1234"), + rows("0.0.0.2"), + rows("1.2.3.4"), + rows("1.2.3.5")); + } + @Test public void testSortMultipleFields() throws IOException { JSONObject result = diff --git a/integ-test/src/test/resources/indexDefinitions/weblogs_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/weblogs_index_mapping.json index bff3e20bb9..05b9784313 100644 --- a/integ-test/src/test/resources/indexDefinitions/weblogs_index_mapping.json +++ b/integ-test/src/test/resources/indexDefinitions/weblogs_index_mapping.json @@ -1,12 +1,9 @@ { "mappings": { "properties": { - "host_ip": { + "host": { "type": "ip" }, - "host_string": { - "type": "keyword" - }, "method": { "type": "text" }, diff --git a/integ-test/src/test/resources/weblogs.json b/integ-test/src/test/resources/weblogs.json index d2e9a968f8..27d39b83be 100644 --- a/integ-test/src/test/resources/weblogs.json +++ b/integ-test/src/test/resources/weblogs.json @@ -1,6 +1,12 @@ {"index":{}} -{"host_ip": "199.72.81.55", "host_string": "199.72.81.55", "method": "GET", "url": "/history/apollo/", "response": "200", "bytes": "6245"} +{"host": "::1", "method": "GET", "url": "/history/apollo/", "response": "200", "bytes": "6245"} {"index":{}} -{"host_ip": "199.120.110.21", "host_string": "199.120.110.21", "method": "GET", "url": "/shuttle/missions/sts-73/mission-sts-73.html", "response": "200", "bytes": "4085"} +{"host": "0.0.0.2", "method": "GET", "url": "/shuttle/missions/sts-73/mission-sts-73.html", "response": "200", "bytes": "4085"} {"index":{}} -{"host_ip": "205.212.115.106", "host_string": "205.212.115.106", "method": "GET", "url": "/shuttle/countdown/countdown.html", "response": "200", "bytes": "3985"} +{"host": "::3", "method": "GET", "url": "/shuttle/countdown/countdown.html", "response": "200", "bytes": "3985"} +{"index":{}} +{"host": "::FFFF:1.2.3.4", "method": "GET", "url": "/history/voyager1/", "response": "200", "bytes": "1234"} +{"index":{}} +{"host": "1.2.3.5", "method": "GET", "url": "/history/voyager2/", "response": "200", "bytes": "4321"} +{"index":{}} +{"host": "::FFFF:1234", "method": "GET", "url": "/history/artemis/", "response": "200", "bytes": "9876"} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java index c35eacfc72..6c8912be86 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java @@ -26,7 +26,7 @@ public enum MappingType { Invalid(null, ExprCoreType.UNKNOWN), Text("text", ExprCoreType.UNKNOWN), Keyword("keyword", ExprCoreType.STRING), - Ip("ip", ExprCoreType.UNKNOWN), + Ip("ip", ExprCoreType.IP), GeoPoint("geo_point", ExprCoreType.UNKNOWN), Binary("binary", ExprCoreType.UNKNOWN), Date("date", ExprCoreType.TIMESTAMP), @@ -160,8 +160,6 @@ public static OpenSearchDataType of(MappingType mappingType, Map return OpenSearchGeoPointType.of(); case Binary: return OpenSearchBinaryType.of(); - case Ip: - return OpenSearchIpType.of(); case Date: case DateNanos: // Default date formatter is used when "" is passed as the second parameter diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchIpType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchIpType.java deleted file mode 100644 index 22581ec28c..0000000000 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchIpType.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.opensearch.data.type; - -import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; - -import lombok.EqualsAndHashCode; - -/** - * The type of an ip value. See doc - */ -@EqualsAndHashCode(callSuper = false) -public class OpenSearchIpType extends OpenSearchDataType { - - private static final OpenSearchIpType instance = new OpenSearchIpType(); - - private OpenSearchIpType() { - super(MappingType.Ip); - exprCoreType = UNKNOWN; - } - - public static OpenSearchIpType of() { - return OpenSearchIpType.instance; - } - - @Override - protected OpenSearchDataType cloneEmpty() { - return instance; - } -} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValue.java deleted file mode 100644 index 30b3784bfc..0000000000 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValue.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.opensearch.data.value; - -import java.util.Objects; -import lombok.RequiredArgsConstructor; -import org.opensearch.sql.data.model.AbstractExprValue; -import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.type.ExprType; -import org.opensearch.sql.opensearch.data.type.OpenSearchIpType; - -/** - * OpenSearch IP ExprValue
- * Todo, add this to avoid the unknown value type exception, the implementation will be changed. - */ -@RequiredArgsConstructor -public class OpenSearchExprIpValue extends AbstractExprValue { - - private final String ip; - - @Override - public Object value() { - return ip; - } - - @Override - public ExprType type() { - return OpenSearchIpType.of(); - } - - @Override - public int compare(ExprValue other) { - return ip.compareTo(((OpenSearchExprIpValue) other).ip); - } - - @Override - public boolean equal(ExprValue other) { - return ip.equals(((OpenSearchExprIpValue) other).ip); - } - - @Override - public int hashCode() { - return Objects.hashCode(ip); - } -} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 41d6667ded..68c6fda617 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -11,6 +11,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.data.type.ExprCoreType.STRUCT; @@ -50,6 +51,7 @@ import org.opensearch.sql.data.model.ExprDoubleValue; import org.opensearch.sql.data.model.ExprFloatValue; import org.opensearch.sql.data.model.ExprIntegerValue; +import org.opensearch.sql.data.model.ExprIpValue; import org.opensearch.sql.data.model.ExprLongValue; import org.opensearch.sql.data.model.ExprNullValue; import org.opensearch.sql.data.model.ExprShortValue; @@ -63,7 +65,6 @@ import org.opensearch.sql.opensearch.data.type.OpenSearchBinaryType; import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.data.type.OpenSearchDateType; -import org.opensearch.sql.opensearch.data.type.OpenSearchIpType; import org.opensearch.sql.opensearch.data.utils.Content; import org.opensearch.sql.opensearch.data.utils.ObjectContent; import org.opensearch.sql.opensearch.data.utils.OpenSearchJsonContent; @@ -133,8 +134,8 @@ public void extendTypeMapping(Map typeMapping) { OpenSearchDateType.of(TIMESTAMP), OpenSearchExprValueFactory::createOpenSearchDateType) .put( - OpenSearchDataType.of(OpenSearchDataType.MappingType.Ip), - (c, dt) -> new OpenSearchExprIpValue(c.stringValue())) + OpenSearchDateType.of(OpenSearchDataType.MappingType.Ip), + (c, dt) -> new ExprIpValue(c.stringValue())) .put( OpenSearchDataType.of(OpenSearchDataType.MappingType.Binary), (c, dt) -> new OpenSearchExprBinaryValue(c.stringValue())) @@ -202,14 +203,12 @@ private ExprValue parse( } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Object)) || type == STRUCT) { return parseStruct(content, field, supportArrays); + } else if (typeActionMap.containsKey(type)) { + return typeActionMap.get(type).apply(content, type); } else { - if (typeActionMap.containsKey(type)) { - return typeActionMap.get(type).apply(content, type); - } else { - throw new IllegalStateException( - String.format( - "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); - } + throw new IllegalStateException( + String.format( + "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); } } @@ -418,10 +417,10 @@ private ExprValue parseGeoPoint(Content content, boolean supportArrays) { */ private ExprValue parseInnerArrayValue( Content content, String prefix, ExprType type, boolean supportArrays) { - if (type instanceof OpenSearchIpType - || type instanceof OpenSearchBinaryType - || type instanceof OpenSearchDateType) { + if (type instanceof OpenSearchBinaryType || type instanceof OpenSearchDateType) { return parse(content, prefix, Optional.of(type), supportArrays); + } else if (content.isString() && type.equals(OpenSearchDataType.of(IP))) { + return parse(content, prefix, Optional.of(OpenSearchDataType.of(IP)), supportArrays); } else if (content.isString()) { return parse(content, prefix, Optional.of(OpenSearchDataType.of(STRING)), supportArrays); } else if (content.isLong()) { diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java index c9ef5bcca5..26ef56e576 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/LuceneQuery.java @@ -18,6 +18,7 @@ import org.opensearch.sql.data.model.ExprDoubleValue; import org.opensearch.sql.data.model.ExprFloatValue; import org.opensearch.sql.data.model.ExprIntegerValue; +import org.opensearch.sql.data.model.ExprIpValue; import org.opensearch.sql.data.model.ExprLongValue; import org.opensearch.sql.data.model.ExprShortValue; import org.opensearch.sql.data.model.ExprStringValue; @@ -211,6 +212,11 @@ private ExprValue cast(FunctionExpression castFunction, ReferenceExpression ref) return expr.valueOf(); } }) + .put( + BuiltinFunctionName.CAST_TO_IP.getName(), + (expr, ref) -> { + return new ExprIpValue(expr.valueOf().stringValue()); + }) .put( BuiltinFunctionName.CAST_TO_DATE.getName(), (expr, ref) -> { diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeRecognitionTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeRecognitionTest.java index 35ad6b7ea6..2e90004571 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeRecognitionTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeRecognitionTest.java @@ -17,7 +17,6 @@ import org.opensearch.sql.expression.DSL; import org.opensearch.sql.opensearch.data.value.OpenSearchExprBinaryValue; import org.opensearch.sql.opensearch.data.value.OpenSearchExprGeoPointValue; -import org.opensearch.sql.opensearch.data.value.OpenSearchExprIpValue; import org.opensearch.sql.opensearch.data.value.OpenSearchExprTextValue; public class OpenSearchDataTypeRecognitionTest { @@ -33,7 +32,6 @@ private static Stream types() { return Stream.of( Arguments.of("TEXT", new OpenSearchExprTextValue("A"), "text without fields"), Arguments.of("BINARY", new OpenSearchExprBinaryValue("A"), "binary"), - Arguments.of("IP", new OpenSearchExprIpValue("A"), "ip"), Arguments.of("TEXT", new TestTextWithFieldValue("Hello World"), "text with fields"), Arguments.of("GEO_POINT", new OpenSearchExprGeoPointValue(0d, 0d), "geo point")); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java index 76fbbd6e65..77b905e228 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java @@ -22,6 +22,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.SHORT; import static org.opensearch.sql.data.type.ExprCoreType.STRING; @@ -108,9 +109,9 @@ private static Stream getTestDataWithType() { Arguments.of(MappingType.DateNanos, "timestamp", TIMESTAMP), Arguments.of(MappingType.Object, "object", STRUCT), Arguments.of(MappingType.Nested, "nested", ARRAY), + Arguments.of(MappingType.Ip, "ip", IP), Arguments.of(MappingType.GeoPoint, "geo_point", OpenSearchGeoPointType.of()), - Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of()), - Arguments.of(MappingType.Ip, "ip", OpenSearchIpType.of())); + Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of())); } @ParameterizedTest(name = "{1}") @@ -188,13 +189,13 @@ public void types_but_clones_are_singletons_and_cached() { () -> assertSame(OpenSearchDataType.of(MappingType.Text), OpenSearchTextType.of()), () -> assertSame(OpenSearchDataType.of(MappingType.Binary), OpenSearchBinaryType.of()), () -> assertSame(OpenSearchDataType.of(MappingType.GeoPoint), OpenSearchGeoPointType.of()), - () -> assertSame(OpenSearchDataType.of(MappingType.Ip), OpenSearchIpType.of()), () -> assertNotSame( OpenSearchTextType.of(), OpenSearchTextType.of(Map.of("properties", OpenSearchDataType.of(INTEGER)))), () -> assertSame(OpenSearchDataType.of(INTEGER), OpenSearchDataType.of(INTEGER)), () -> assertSame(OpenSearchDataType.of(STRING), OpenSearchDataType.of(STRING)), + () -> assertSame(OpenSearchDataType.of(IP), OpenSearchDataType.of(IP)), () -> assertSame(OpenSearchDataType.of(STRUCT), OpenSearchDataType.of(STRUCT)), () -> assertNotSame( diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValueTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValueTest.java index 5ee175f304..8b13789179 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValueTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprIpValueTest.java @@ -1,45 +1 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ -package org.opensearch.sql.opensearch.data.value; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import org.junit.jupiter.api.Test; -import org.opensearch.sql.opensearch.data.type.OpenSearchIpType; - -public class OpenSearchExprIpValueTest { - - private final String ipString = "192.168.0.1"; - private final OpenSearchExprIpValue ipValue = new OpenSearchExprIpValue(ipString); - - @Test - void testValue() { - assertEquals(ipString, ipValue.value()); - } - - @Test - void testType() { - assertEquals(OpenSearchIpType.of(), ipValue.type()); - } - - @Test - void testCompare() { - assertEquals(0, ipValue.compareTo(new OpenSearchExprIpValue(ipString))); - assertEquals(ipValue, new OpenSearchExprIpValue(ipString)); - } - - @Test - void testEqual() { - assertTrue(ipValue.equal(new OpenSearchExprIpValue(ipString))); - } - - @Test - void testHashCode() { - assertNotNull(ipValue.hashCode()); - } -} diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index d82926077e..89dfd4dbdb 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -16,6 +16,7 @@ import static org.opensearch.sql.data.model.ExprValueUtils.doubleValue; import static org.opensearch.sql.data.model.ExprValueUtils.floatValue; import static org.opensearch.sql.data.model.ExprValueUtils.integerValue; +import static org.opensearch.sql.data.model.ExprValueUtils.ipValue; import static org.opensearch.sql.data.model.ExprValueUtils.longValue; import static org.opensearch.sql.data.model.ExprValueUtils.nullValue; import static org.opensearch.sql.data.model.ExprValueUtils.shortValue; @@ -51,6 +52,7 @@ import org.opensearch.geometry.utils.Geohash; import org.opensearch.sql.data.model.ExprCollectionValue; import org.opensearch.sql.data.model.ExprDateValue; +import org.opensearch.sql.data.model.ExprIpValue; import org.opensearch.sql.data.model.ExprTimeValue; import org.opensearch.sql.data.model.ExprTimestampValue; import org.opensearch.sql.data.model.ExprTupleValue; @@ -215,6 +217,16 @@ public void constructString() { () -> assertEquals(stringValue("text"), constructFromObject("stringV", "text"))); } + @Test + public void constructIp() { + assertAll( + () -> assertEquals(ipValue("1.2.3.4"), tupleValue("{\"ipV\":\"1.2.3.4\"}").get("ipV")), + () -> + assertEquals( + ipValue("2001:db7::ff00:42:8329"), + constructFromObject("ipV", "2001:db7::ff00:42:8329"))); + } + @Test public void constructBoolean() { assertAll( @@ -659,17 +671,6 @@ public void constructArrayOfGeoPointsReturnsAll() { .get("geoV")); } - @Test - public void constructArrayOfIPsReturnsAll() { - final String ip1 = "192.168.0.1"; - final String ip2 = "192.168.0.2"; - - assertEquals( - new ExprCollectionValue( - List.of(new OpenSearchExprIpValue(ip1), new OpenSearchExprIpValue(ip2))), - tupleValue(String.format("{\"%s\":[\"%s\",\"%s\"]}", fieldIp, ip1, ip2)).get(fieldIp)); - } - @Test public void constructBinaryArrayReturnsAll() { assertEquals( @@ -681,6 +682,17 @@ public void constructBinaryArrayReturnsAll() { .get("binaryV")); } + @Test + public void constructArrayOfIPsReturnsAll() { + final String ipv4String = "1.2.3.4"; + final String ipv6String = "2001:db7::ff00:42:8329"; + + assertEquals( + new ExprCollectionValue(List.of(ipValue(ipv4String), ipValue(ipv6String))), + tupleValue(String.format("{\"%s\":[\"%s\",\"%s\"]}", fieldIp, ipv4String, ipv6String)) + .get(fieldIp)); + } + @Test public void constructArrayOfCustomEpochMillisReturnsAll() { assertEquals( @@ -743,10 +755,10 @@ public void constructStruct() { @Test public void constructIP() { - final String valueIp = "192.168.0.1"; + final String ipString = "192.168.0.1"; assertEquals( - new OpenSearchExprIpValue(valueIp), - tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, valueIp)).get(fieldIp)); + new ExprIpValue(ipString), + tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, ipString)).get(fieldIp)); } @Test diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java index bd2a9901ed..f8c43743ab 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java @@ -16,6 +16,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; +import static org.opensearch.sql.data.type.ExprCoreType.IP; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.SHORT; import static org.opensearch.sql.data.type.ExprCoreType.STRING; @@ -59,6 +60,10 @@ @ExtendWith(MockitoExtension.class) class FilterQueryBuilderTest { + private static Stream ipCastSource() { + return Stream.of(literal("1.2.3.4"), literal("2001:db7::ff00:42:8329")); + } + private static Stream numericCastSource() { return Stream.of( literal((byte) 1), @@ -1715,6 +1720,25 @@ void cast_to_boolean_false_in_filter(LiteralExpression expr) { json, buildQuery(DSL.equal(ref("boolean_value", BOOLEAN), DSL.castBoolean(expr)))); } + @ParameterizedTest(name = "castIp({0})") + @MethodSource({"ipCastSource"}) + void cast_to_ip_in_filter(LiteralExpression expr) { + String json = + String.format( + """ + { + "term" : { + "ip_value" : { + "value" : "%s", + "boost" : 1.0 + } + } + }""", + expr.valueOf().stringValue()); + + assertJsonEquals(json, buildQuery(DSL.equal(ref("ip_value", IP), DSL.castIp(expr)))); + } + @Test void cast_from_boolean() { Expression booleanExpr = literal(false); diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index 4a883fa656..053ec530db 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -52,10 +52,10 @@ WITH: 'WITH'; // CLAUSE KEYWORDS SORTBY: 'SORTBY'; -// FIELD KEYWORDS +// SORT FIELD KEYWORDS +// TODO #3180: Fix broken sort functionality AUTO: 'AUTO'; STR: 'STR'; -IP: 'IP'; NUM: 'NUM'; // TRENDLINE KEYWORDS @@ -142,6 +142,7 @@ LONG: 'LONG'; FLOAT: 'FLOAT'; STRING: 'STRING'; BOOLEAN: 'BOOLEAN'; +IP: 'IP'; // SPECIAL CHARACTERS AND OPERATORS PIPE: '|'; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index c9d0f2e110..27f7e4014b 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -408,6 +408,7 @@ convertedDataType | typeName = FLOAT | typeName = STRING | typeName = BOOLEAN + | typeName = IP ; evalFunctionName @@ -897,7 +898,8 @@ keywordsCanBeId | DATASOURCES // CLAUSEKEYWORDS | SORTBY - // FIELDKEYWORDSAUTO + // SORT FIELD KEYWORDS + | AUTO | STR | IP | NUM diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index 8bc98c8eee..5a7522683a 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -161,6 +161,8 @@ public UnresolvedExpression visitWcFieldExpression(WcFieldExpressionContext ctx) @Override public UnresolvedExpression visitSortField(SortFieldContext ctx) { + + // TODO #3180: Fix broken sort functionality return new Field( visit(ctx.sortFieldExpression().fieldExpression().qualifiedName()), ArgumentFactory.getArgumentList(ctx));