From bd264a0f6652711bc24776aa369be1437bf6721d Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 22 Oct 2024 10:03:00 -0700 Subject: [PATCH 01/40] Add stub implementation of `cidr`: - Update Lexer and Parser. - Add `DSL::cidr` and BuiltinFunctionName.CIDR`. - Add `BinaryPredicateOperator::cidr`. - Add `IPUtils` with stub implementation of `isAddressInRange`. Signed-off-by: currantw --- .../org/opensearch/sql/expression/DSL.java | 4 ++++ .../function/BuiltinFunctionName.java | 3 +++ .../predicate/BinaryPredicateOperator.java | 14 ++++++++++--- .../predicate/UnaryPredicateOperator.java | 2 +- .../org/opensearch/sql/utils/IPUtils.java | 21 +++++++++++++++++++ ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 1 + ppl/src/main/antlr/OpenSearchPPLParser.g4 | 1 + 7 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/utils/IPUtils.java 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 9975afac7f..f987211355 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -563,6 +563,10 @@ public static FunctionExpression regexp(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.REGEXP, expressions); } + public static FunctionExpression cidr(Expression... expressions) { + return compile(FunctionProperties.None, BuiltinFunctionName.CIDR, expressions); + } + public static FunctionExpression concat(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.CONCAT, expressions); } 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 fd5ea14a2e..cc4e5b6d8d 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 @@ -132,6 +132,9 @@ public enum BuiltinFunctionName { /** Text Functions. */ TOSTRING(FunctionName.of("tostring")), + /** IP Functions. */ + CIDR(FunctionName.of("cidr")), + /** Arithmetic Operators. */ ADD(FunctionName.of("+")), ADDFUNCTION(FunctionName.of("add")), diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java index bf6b3c22f5..9801017f2d 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java @@ -26,6 +26,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; import org.opensearch.sql.utils.OperatorUtils; /** @@ -55,6 +56,7 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(like()); repository.register(notLike()); repository.register(regexp()); + repository.register(cidr()); } /** @@ -118,7 +120,7 @@ public static void register(BuiltinFunctionRepository repository) { * * */ - private static Table andTable = + private static final Table andTable = new ImmutableTable.Builder() .put(LITERAL_TRUE, LITERAL_TRUE, LITERAL_TRUE) .put(LITERAL_TRUE, LITERAL_FALSE, LITERAL_FALSE) @@ -193,7 +195,7 @@ public static void register(BuiltinFunctionRepository repository) { * * */ - private static Table orTable = + private static final Table orTable = new ImmutableTable.Builder() .put(LITERAL_TRUE, LITERAL_TRUE, LITERAL_TRUE) .put(LITERAL_TRUE, LITERAL_FALSE, LITERAL_TRUE) @@ -268,7 +270,7 @@ public static void register(BuiltinFunctionRepository repository) { * * */ - private static Table xorTable = + private static final Table xorTable = new ImmutableTable.Builder() .put(LITERAL_TRUE, LITERAL_TRUE, LITERAL_FALSE) .put(LITERAL_TRUE, LITERAL_FALSE, LITERAL_TRUE) @@ -396,6 +398,12 @@ private static DefaultFunctionResolver regexp() { impl(nullMissingHandling(OperatorUtils::matchesRegexp), INTEGER, STRING, STRING)); } + private static DefaultFunctionResolver cidr() { + return define( + BuiltinFunctionName.CIDR.getName(), + impl(nullMissingHandling(IPUtils::isAddressInRange), BOOLEAN, STRING, STRING)); + } + private static DefaultFunctionResolver notLike() { return define( BuiltinFunctionName.NOT_LIKE.getName(), diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java index ad9d9ac934..9c097217ce 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java @@ -161,7 +161,7 @@ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { * * @param v1 varable 1 * @param v2 varable 2 - * @return null if v1 equls to v2 + * @return null if v1 equals to v2 */ public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { return v1.equals(v2) ? LITERAL_NULL : v1; 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..bfc26782c6 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java @@ -0,0 +1,21 @@ +package org.opensearch.sql.utils; + +import org.opensearch.sql.data.model.ExprBooleanValue; +import org.opensearch.sql.data.model.ExprValue; + +public class IPUtils { + + /** + * Returns whether the given IP address is within the specified IP address range. + * Supports both IPv4 and IPv6 addresses. + * + * @param address IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @param range IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8:/32") + * @return true value if IP address is in range; else false value. + */ + public static ExprBooleanValue isAddressInRange(ExprValue address, ExprValue range) { + + // TODO - implementation + return ExprBooleanValue.of(true); + } +} diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index 9f707c13cd..7446a6e6e2 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -322,6 +322,7 @@ CAST: 'CAST'; LIKE: 'LIKE'; ISNULL: 'ISNULL'; ISNOTNULL: 'ISNOTNULL'; +CIDR: 'CIDR'; // FLOWCONTROL FUNCTIONS IFNULL: 'IFNULL'; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 4dc223b028..579a933171 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -629,6 +629,7 @@ conditionFunctionName : LIKE | ISNULL | ISNOTNULL + | CIDR ; // flow control function return non-boolean value From c2ecdf72c40faaa276399f2d88675f6e9b8e6ef1 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 23 Oct 2024 09:40:13 -0700 Subject: [PATCH 02/40] Initial implementation of `isAddressInRange` method. Signed-off-by: currantw --- .../org/opensearch/sql/utils/IPUtils.java | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java index bfc26782c6..1b2a41f14a 100644 --- a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java @@ -1,21 +1,43 @@ package org.opensearch.sql.utils; +import com.google.common.net.InetAddresses; import org.opensearch.sql.data.model.ExprBooleanValue; import org.opensearch.sql.data.model.ExprValue; +import java.util.Arrays; + public class IPUtils { /** * Returns whether the given IP address is within the specified IP address range. * Supports both IPv4 and IPv6 addresses. * - * @param address IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @param range IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8:/32") - * @return true value if IP address is in range; else false value. + * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8:/32") + * @return true if IP address is in range; else false */ - public static ExprBooleanValue isAddressInRange(ExprValue address, ExprValue range) { + public static ExprBooleanValue isAddressInRange(ExprValue addressExprValue, ExprValue rangeExprValue) { + + byte[] addressBytes; + try { + addressBytes = InetAddresses.forString(addressExprValue.stringValue()).getAddress(); + } catch (Exception e) { + throw new IllegalArgumentException(String.format("Argument '%s' is not a valid IP address", addressExprValue)); + } + + int prefixLengthBytes; + byte[] rangeBytes; + try { + String[] rangeFields = rangeExprValue.stringValue().split("/"); + prefixLengthBytes = Integer.parseInt(rangeFields[1]) / Byte.SIZE; + rangeBytes = Arrays.copyOfRange(InetAddresses.forString(rangeFields[0]).getAddress(), 0, prefixLengthBytes); + } catch (Exception e) { + throw new IllegalArgumentException(String.format("Argument '%s' is not a valid IP address range in CIDR notation", rangeExprValue)); + } + + if(addressBytes.length < prefixLengthBytes) + return ExprBooleanValue.of(false); - // TODO - implementation - return ExprBooleanValue.of(true); + return ExprBooleanValue.of(Arrays.equals(addressBytes, 0, prefixLengthBytes, rangeBytes, 0, prefixLengthBytes)); } } From bfcbbf5784f1bed5d697a98ec036414f105efbf5 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 23 Oct 2024 10:48:48 -0700 Subject: [PATCH 03/40] Add `BinaryPredicateOperator` tests for `cidr`. Update `isAddressInRange` implementation to return `false` rather than raising an exception. Signed-off-by: currantw --- .../org/opensearch/sql/utils/IPUtils.java | 24 ++++-------- .../BinaryPredicateOperatorTest.java | 38 ++++++++++++++++--- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java index 1b2a41f14a..fcf0deed8e 100644 --- a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java @@ -13,31 +13,21 @@ public class IPUtils { * Supports both IPv4 and IPv6 addresses. * * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8:/32") + * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8:/32") * @return true if IP address is in range; else false */ public static ExprBooleanValue isAddressInRange(ExprValue addressExprValue, ExprValue rangeExprValue) { - byte[] addressBytes; try { - addressBytes = InetAddresses.forString(addressExprValue.stringValue()).getAddress(); - } catch (Exception e) { - throw new IllegalArgumentException(String.format("Argument '%s' is not a valid IP address", addressExprValue)); - } + byte[] addressBytes = InetAddresses.forString(addressExprValue.stringValue()).getAddress(); - int prefixLengthBytes; - byte[] rangeBytes; - try { String[] rangeFields = rangeExprValue.stringValue().split("/"); - prefixLengthBytes = Integer.parseInt(rangeFields[1]) / Byte.SIZE; - rangeBytes = Arrays.copyOfRange(InetAddresses.forString(rangeFields[0]).getAddress(), 0, prefixLengthBytes); - } catch (Exception e) { - throw new IllegalArgumentException(String.format("Argument '%s' is not a valid IP address range in CIDR notation", rangeExprValue)); - } + int prefixLengthBytes = Integer.parseInt(rangeFields[1]) / Byte.SIZE; + byte[] rangeBytes = Arrays.copyOfRange(InetAddresses.forString(rangeFields[0]).getAddress(), 0, prefixLengthBytes); - if(addressBytes.length < prefixLengthBytes) + return ExprBooleanValue.of(Arrays.equals(addressBytes, 0, prefixLengthBytes, rangeBytes, 0, prefixLengthBytes)); + } catch (Exception e) { return ExprBooleanValue.of(false); - - return ExprBooleanValue.of(Arrays.equals(addressBytes, 0, prefixLengthBytes, rangeBytes, 0, prefixLengthBytes)); + } } } diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index 55dfbd35c2..4edb0a42d4 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -57,7 +57,7 @@ class BinaryPredicateOperatorTest extends ExpressionTestBase { - private static List STRING_PATTERN_PAIRS = + private static final List STRING_PATTERN_PAIRS = ImmutableList.of( new StringPatternPair("Michael!", ".*"), new StringPatternPair("new*\\n*line", "new\\\\*.\\\\*line"), @@ -209,13 +209,31 @@ private static Stream testLikeArguments() { return builder.build(); } + private static Stream getCidrArguments() { + return Stream.of( + Arguments.of("10.24.33.5", "10.24.34.0/24", false), // IPv4 less + Arguments.of("10.24.34.5", "10.24.34.0/24", true), // IPv4 between + Arguments.of("10.24.35.5", "10.24.34.0/24", false), // IPv4 greater + Arguments.of("10.24.35.5", "10.24.34.0/33", false), // IPv4 prefix too long + + Arguments.of("2001:0db7::8329", "2001:0db8::/32", false), // IPv6 less + Arguments.of("2001:0db8::8329", "2001:0db8::/32", true), // IPv6 between + Arguments.of("2001:0db9::8329", "2001:0db8::/32", false), // IPv6 greater + Arguments.of("2001:0db9::8329", "2001:0db8::/129", false), // IPv6 prefix too long + + Arguments.of("INVALID", "10.24.34.0/24", false), // Invalid argument + Arguments.of("10.24.34.5", "INVALID", false), // Invalid range + Arguments.of("10.24.34.5", "10.24.34.0/INVALID", false) // Invalid prefix + ); + } + @ParameterizedTest(name = "and({0}, {1})") @MethodSource("binaryPredicateArguments") public void test_and(Boolean v1, Boolean v2) { FunctionExpression and = DSL.and(DSL.literal(booleanValue(v1)), DSL.literal(booleanValue(v2))); assertEquals(BOOLEAN, and.type()); assertEquals(v1 && v2, ExprValueUtils.getBooleanValue(and.valueOf(valueEnv()))); - assertEquals(String.format("and(%s, %s)", v1.toString(), v2.toString()), and.toString()); + assertEquals(String.format("and(%s, %s)", v1, v2.toString()), and.toString()); } @Test @@ -295,7 +313,7 @@ public void test_or(Boolean v1, Boolean v2) { FunctionExpression or = DSL.or(DSL.literal(booleanValue(v1)), DSL.literal(booleanValue(v2))); assertEquals(BOOLEAN, or.type()); assertEquals(v1 || v2, ExprValueUtils.getBooleanValue(or.valueOf(valueEnv()))); - assertEquals(String.format("or(%s, %s)", v1.toString(), v2.toString()), or.toString()); + assertEquals(String.format("or(%s, %s)", v1, v2.toString()), or.toString()); } @Test @@ -375,7 +393,7 @@ public void test_xor(Boolean v1, Boolean v2) { FunctionExpression xor = DSL.xor(DSL.literal(booleanValue(v1)), DSL.literal(booleanValue(v2))); assertEquals(BOOLEAN, xor.type()); assertEquals(v1 ^ v2, ExprValueUtils.getBooleanValue(xor.valueOf(valueEnv()))); - assertEquals(String.format("xor(%s, %s)", v1.toString(), v2.toString()), xor.toString()); + assertEquals(String.format("xor(%s, %s)", v1, v2), xor.toString()); } @Test @@ -555,7 +573,7 @@ public void test_like(ExprValue v1, ExprValue v2) { FunctionExpression like = DSL.like(DSL.literal(v1), DSL.literal(v2)); assertEquals(BOOLEAN, like.type()); assertEquals(matches(v1, v2), like.valueOf(valueEnv())); - assertEquals(String.format("like(%s, %s)", v1.toString(), v2.toString()), like.toString()); + assertEquals(String.format("like(%s, %s)", v1, v2), like.toString()); } @Test @@ -584,7 +602,7 @@ void testRegexpString(StringPatternPair stringPatternPair) { assertEquals(stringPatternPair.regExpTest(), expression.valueOf(valueEnv()).integerValue()); } - /** Todo. remove this test cases after script serilization implemented. */ + /** Todo. remove this test cases after script serialization implemented. */ @Test public void serializationTest() throws Exception { Expression expression = DSL.equal(DSL.literal("v1"), DSL.literal("v2")); @@ -615,4 +633,12 @@ public void compare_int_long() { FunctionExpression equal = DSL.equal(DSL.literal(1), DSL.literal(1L)); assertTrue(equal.valueOf(valueEnv()).booleanValue()); } + + @ParameterizedTest + @MethodSource("getCidrArguments") + public void test_cidr(String address, String range, boolean expected) { + FunctionExpression cidr = DSL.cidr(DSL.literal(address), DSL.literal(range)); + assertEquals(cidr.type(), BOOLEAN); + assertEquals(cidr.valueOf().booleanValue(), expected); + } } From 093696a497f599ce6ca8c5e115fce6d67c3d6833 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 23 Oct 2024 11:40:44 -0700 Subject: [PATCH 04/40] Fix a couple documentation typos. Signed-off-by: currantw --- core/src/main/java/org/opensearch/sql/utils/IPUtils.java | 2 +- .../operator/predicate/BinaryPredicateOperatorTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java index fcf0deed8e..46d8516d6a 100644 --- a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java @@ -13,7 +13,7 @@ public class IPUtils { * Supports both IPv4 and IPv6 addresses. * * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8:/32") + * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8::/32") * @return true if IP address is in range; else false */ public static ExprBooleanValue isAddressInRange(ExprValue addressExprValue, ExprValue rangeExprValue) { diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index 4edb0a42d4..eda7d2756d 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -221,7 +221,7 @@ private static Stream getCidrArguments() { Arguments.of("2001:0db9::8329", "2001:0db8::/32", false), // IPv6 greater Arguments.of("2001:0db9::8329", "2001:0db8::/129", false), // IPv6 prefix too long - Arguments.of("INVALID", "10.24.34.0/24", false), // Invalid argument + Arguments.of("INVALID", "10.24.34.0/24", false), // Invalid address Arguments.of("10.24.34.5", "INVALID", false), // Invalid range Arguments.of("10.24.34.5", "10.24.34.0/INVALID", false) // Invalid prefix ); From f0464d3efb8eecd7a7f402d24eb9b50dd7b48fdc Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 23 Oct 2024 12:18:38 -0700 Subject: [PATCH 05/40] Add documentation for `cidr` function. Signed-off-by: currantw --- .../org/opensearch/sql/utils/IPUtils.java | 2 +- docs/user/ppl/functions/ip.rst | 33 +++++++++++++++++++ docs/user/ppl/index.rst | 2 ++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 docs/user/ppl/functions/ip.rst diff --git a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java index 46d8516d6a..01bb8332c8 100644 --- a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java @@ -14,7 +14,7 @@ public class IPUtils { * * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8::/32") - * @return true if IP address is in range; else false + * @return true if address is in range; else false */ public static ExprBooleanValue isAddressInRange(ExprValue addressExprValue, ExprValue rangeExprValue) { diff --git a/docs/user/ppl/functions/ip.rst b/docs/user/ppl/functions/ip.rst new file mode 100644 index 0000000000..a5366faaa4 --- /dev/null +++ b/docs/user/ppl/functions/ip.rst @@ -0,0 +1,33 @@ +================ +IP Functions +================ + +.. rubric:: Table of contents + +.. contents:: + :local: + :depth: 1 + +CIDR +------ + +Description +>>>>>>>>>>> + +Usage: cidr(address, range) returns true if IP address is within the IP address range in CIDR notation. Supports both IPv4 and IPv6 addresses. + +Argument type: STRING, STRING + +Return type: BOOLEAN + +Example:: + + os> source=devices | where cidr(address, "198.51.100.0/24") + fetched rows / total rows = 2/2 + +----------------+----------------+ + | name | address | + |----------------+----------------+ + | John's Macbook | 198.51.100.2 | + | Iain's PC | 198.51.100.254 | + +----------------+----------------+ + diff --git a/docs/user/ppl/index.rst b/docs/user/ppl/index.rst index 1fa981b1b7..8106e044dd 100644 --- a/docs/user/ppl/index.rst +++ b/docs/user/ppl/index.rst @@ -102,6 +102,8 @@ The query start with search command and then flowing a set of command delimited - `System Functions `_ + - `IP Functions `_ + * **Optimization** - `Optimization <../../user/optimization/optimization.rst>`_ From ce8d1903d0c81e6599c730390d4b0421ae6fb083 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 23 Oct 2024 12:28:00 -0700 Subject: [PATCH 06/40] Add documentation for `cidr` function. Signed-off-by: currantw --- docs/user/ppl/functions/ip.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/ppl/functions/ip.rst b/docs/user/ppl/functions/ip.rst index a5366faaa4..cd213918fd 100644 --- a/docs/user/ppl/functions/ip.rst +++ b/docs/user/ppl/functions/ip.rst @@ -14,7 +14,7 @@ CIDR Description >>>>>>>>>>> -Usage: cidr(address, range) returns true if IP address is within the IP address range in CIDR notation. Supports both IPv4 and IPv6 addresses. +Usage: cidr(address, range) returns whether the given IP address is within the specified IP address range. Supports both IPv4 and IPv6 addresses. Argument type: STRING, STRING From 6d56ac99130432eda70d863329ad8fbac2e62042 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 23 Oct 2024 15:43:06 -0700 Subject: [PATCH 07/40] Add license header to `IPUtils`. Signed-off-by: currantw --- core/src/main/java/org/opensearch/sql/utils/IPUtils.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java index 01bb8332c8..065f158a54 100644 --- a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.utils; import com.google.common.net.InetAddresses; From 96aaa895870b0dc728a2a855415ef19f0665e12d Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 23 Oct 2024 15:53:21 -0700 Subject: [PATCH 08/40] Revert `final` and `toString` cleanup. Signed-off-by: currantw --- .../operator/predicate/BinaryPredicateOperator.java | 6 +++--- .../predicate/BinaryPredicateOperatorTest.java | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java index 9801017f2d..58482fda2c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java @@ -120,7 +120,7 @@ public static void register(BuiltinFunctionRepository repository) { * * */ - private static final Table andTable = + private static Table andTable = new ImmutableTable.Builder() .put(LITERAL_TRUE, LITERAL_TRUE, LITERAL_TRUE) .put(LITERAL_TRUE, LITERAL_FALSE, LITERAL_FALSE) @@ -195,7 +195,7 @@ public static void register(BuiltinFunctionRepository repository) { * * */ - private static final Table orTable = + private static Table orTable = new ImmutableTable.Builder() .put(LITERAL_TRUE, LITERAL_TRUE, LITERAL_TRUE) .put(LITERAL_TRUE, LITERAL_FALSE, LITERAL_TRUE) @@ -270,7 +270,7 @@ public static void register(BuiltinFunctionRepository repository) { * * */ - private static final Table xorTable = + private static Table xorTable = new ImmutableTable.Builder() .put(LITERAL_TRUE, LITERAL_TRUE, LITERAL_FALSE) .put(LITERAL_TRUE, LITERAL_FALSE, LITERAL_TRUE) diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index eda7d2756d..f63d96d2f9 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -57,7 +57,7 @@ class BinaryPredicateOperatorTest extends ExpressionTestBase { - private static final List STRING_PATTERN_PAIRS = + private static List STRING_PATTERN_PAIRS = ImmutableList.of( new StringPatternPair("Michael!", ".*"), new StringPatternPair("new*\\n*line", "new\\\\*.\\\\*line"), @@ -233,7 +233,7 @@ public void test_and(Boolean v1, Boolean v2) { FunctionExpression and = DSL.and(DSL.literal(booleanValue(v1)), DSL.literal(booleanValue(v2))); assertEquals(BOOLEAN, and.type()); assertEquals(v1 && v2, ExprValueUtils.getBooleanValue(and.valueOf(valueEnv()))); - assertEquals(String.format("and(%s, %s)", v1, v2.toString()), and.toString()); + assertEquals(String.format("and(%s, %s)", v1.toString(), v2.toString()), and.toString()); } @Test @@ -313,7 +313,7 @@ public void test_or(Boolean v1, Boolean v2) { FunctionExpression or = DSL.or(DSL.literal(booleanValue(v1)), DSL.literal(booleanValue(v2))); assertEquals(BOOLEAN, or.type()); assertEquals(v1 || v2, ExprValueUtils.getBooleanValue(or.valueOf(valueEnv()))); - assertEquals(String.format("or(%s, %s)", v1, v2.toString()), or.toString()); + assertEquals(String.format("or(%s, %s)", v1.toString(), v2.toString()), or.toString()); } @Test @@ -393,7 +393,7 @@ public void test_xor(Boolean v1, Boolean v2) { FunctionExpression xor = DSL.xor(DSL.literal(booleanValue(v1)), DSL.literal(booleanValue(v2))); assertEquals(BOOLEAN, xor.type()); assertEquals(v1 ^ v2, ExprValueUtils.getBooleanValue(xor.valueOf(valueEnv()))); - assertEquals(String.format("xor(%s, %s)", v1, v2), xor.toString()); + assertEquals(String.format("xor(%s, %s)", v1.toString(), v2.toString()), xor.toString()); } @Test @@ -573,7 +573,7 @@ public void test_like(ExprValue v1, ExprValue v2) { FunctionExpression like = DSL.like(DSL.literal(v1), DSL.literal(v2)); assertEquals(BOOLEAN, like.type()); assertEquals(matches(v1, v2), like.valueOf(valueEnv())); - assertEquals(String.format("like(%s, %s)", v1, v2), like.toString()); + assertEquals(String.format("like(%s, %s)", v1.toString(), v2.toString()), like.toString()); } @Test From 04af98de436ea3b8d3839766f3b7e2984954ae35 Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 28 Oct 2024 17:00:09 -0700 Subject: [PATCH 09/40] Update to use `CidrExpression`: - Add `CidrExpression`, which is a `FunctionImplementation`. - Add `CidrExressionTest` (unit tests). - Remove `IPUtils` (logic moved to `CidrExpression`). - Update documentation in `ip.rst`. - Add `IpFunctions` utility class. - Remove previous `IPUtils` utility class. Signed-off-by: currantw --- .../function/BuiltinFunctionRepository.java | 2 + .../sql/expression/ip/CidrExpression.java | 139 ++++++++++++++++++ .../sql/expression/ip/IpFunctions.java | 32 ++++ .../predicate/BinaryPredicateOperator.java | 8 - .../org/opensearch/sql/utils/IPUtils.java | 38 ----- .../sql/expression/ip/CidrExpressionTest.java | 104 +++++++++++++ docs/user/ppl/functions/ip.rst | 15 +- 7 files changed, 284 insertions(+), 54 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java create mode 100644 core/src/main/java/org/opensearch/sql/expression/ip/IpFunctions.java delete mode 100644 core/src/main/java/org/opensearch/sql/utils/IPUtils.java create mode 100644 core/src/test/java/org/opensearch/sql/expression/ip/CidrExpressionTest.java diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java index 2e16d5f01f..750a012684 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java @@ -27,6 +27,7 @@ import org.opensearch.sql.expression.aggregation.AggregatorFunction; import org.opensearch.sql.expression.datetime.DateTimeFunction; import org.opensearch.sql.expression.datetime.IntervalClause; +import org.opensearch.sql.expression.ip.IpFunctions; import org.opensearch.sql.expression.operator.arthmetic.ArithmeticFunction; import org.opensearch.sql.expression.operator.arthmetic.MathematicalFunction; import org.opensearch.sql.expression.operator.convert.TypeCastOperator; @@ -81,6 +82,7 @@ public static synchronized BuiltinFunctionRepository getInstance() { TypeCastOperator.register(instance); SystemFunctions.register(instance); OpenSearchFunctions.register(instance); + IpFunctions.register(instance); } return instance; } diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java b/core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java new file mode 100644 index 0000000000..7ae74a5af7 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java @@ -0,0 +1,139 @@ +package org.opensearch.sql.expression.ip; + +import com.google.common.net.InetAddresses; +import lombok.EqualsAndHashCode; +import lombok.ToString; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.data.type.ExprCoreType; +import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.exception.ExpressionEvaluationException; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.env.Environment; +import org.opensearch.sql.expression.function.FunctionName; + +import java.io.Serializable; +import java.math.BigInteger; +import java.net.Inet4Address; +import java.net.InetAddress; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +@ToString +@EqualsAndHashCode(callSuper = false) +public class CidrExpression extends FunctionExpression { + + private final Expression addressExpression; + private final InetAddressRange range; + + public CidrExpression(List arguments) { + super(FunctionName.of("cidr"), arguments); + + // Must be exactly two arguments. + if (arguments.size() != 2) { + String msg = String.format("Unexpected number of arguments to function '%s'. Expected %s, but found %s.", FunctionName.of("cidr"), 2, arguments.size()); + throw new ExpressionEvaluationException(msg); + } + + this.addressExpression = arguments.getFirst(); + this.range = new InetAddressRange(arguments.getLast().valueOf().stringValue()); + } + + @Override + public ExprValue valueOf(Environment valueEnv) { + ExprValue addressValue = addressExpression.valueOf(valueEnv); + if (addressValue.isNull() || addressValue.isMissing()) + return ExprValueUtils.nullValue(); + + String addressString = addressValue.stringValue(); + if (!InetAddresses.isInetAddress(addressString)) + return ExprValueUtils.nullValue(); + + InetAddress address = InetAddresses.forString(addressString); + return ExprValueUtils.booleanValue(range.contains(address)); + } + + @Override + public ExprType type() { + return ExprCoreType.BOOLEAN; + } + + /** + * Represents an IP address range. + * Supports both IPv4 and IPv6 addresses. + */ + private class InetAddressRange implements Serializable { + + // Basic CIDR notation pattern. + private static final Pattern cidrPattern = Pattern.compile("(?
.+)[/](?[0-9]+)"); + + // Lower/upper bounds for the IP address range. + private final InetAddress lowerBound; + private final InetAddress upperBound; + + /** + * Builds a new IP address range from the given CIDR notation string. + * + * @param cidr CIDR notation string (e.g. "198.51.100.0/24" or "2001:0db8::/32") + */ + public InetAddressRange(String cidr) { + + // Parse address and network length. + Matcher cidrMatcher = cidrPattern.matcher(cidr); + if (!cidrMatcher.matches()) + throw new SemanticCheckException(String.format("CIDR notation '%s' in not valid", range)); + + String addressString = cidrMatcher.group("address"); + if (!InetAddresses.isInetAddress(addressString)) + throw new SemanticCheckException(String.format("IP address '%s' in not valid", addressString)); + + InetAddress address = InetAddresses.forString(addressString); + + int networkLengthBits = Integer.parseInt(cidrMatcher.group("prefix")); + int addressLengthBits = address.getAddress().length * Byte.SIZE; + + if (networkLengthBits > addressLengthBits) + throw new SemanticCheckException(String.format("Network length of '%s' bits is not valid", networkLengthBits)); + + // Build bounds by converting the address to an integer, setting all the non-significant bits to + // zero for the lower bounds and one for the upper bounds, and then converting back to addresses. + BigInteger lowerBoundInt = InetAddresses.toBigInteger(address); + BigInteger upperBoundInt = InetAddresses.toBigInteger(address); + + int hostLengthBits = addressLengthBits - networkLengthBits; + for (int bit = 0; bit < hostLengthBits; bit++) { + lowerBoundInt = lowerBoundInt.clearBit(bit); + upperBoundInt = upperBoundInt.setBit(bit); + } + + if (address instanceof Inet4Address) { + lowerBound = InetAddresses.fromIPv4BigInteger(lowerBoundInt); + upperBound = InetAddresses.fromIPv4BigInteger(upperBoundInt); + } else { + lowerBound = InetAddresses.fromIPv6BigInteger(lowerBoundInt); + upperBound = InetAddresses.fromIPv6BigInteger(upperBoundInt); + } + } + + /** + * Returns whether the IP address is contained within the range. + * + * @param address IPv4 or IPv6 address, represented as a {@link BigInteger}. + * (see {@link InetAddresses#toBigInteger(InetAddress)}). + */ + public boolean contains(InetAddress address) { + + if ((address instanceof Inet4Address) ^ (lowerBound instanceof Inet4Address)) return false; + + BigInteger addressInt = InetAddresses.toBigInteger(address); + + if (addressInt.compareTo(InetAddresses.toBigInteger(lowerBound)) < 0) return false; + if (addressInt.compareTo(InetAddresses.toBigInteger(upperBound)) <= 0) return false; + + return true; + } + } +} 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 new file mode 100644 index 0000000000..5582ff89ef --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IpFunctions.java @@ -0,0 +1,32 @@ +package org.opensearch.sql.expression.ip; + +import lombok.experimental.UtilityClass; +import org.apache.commons.lang3.tuple.Pair; +import org.opensearch.sql.expression.function.*; + +import java.util.Arrays; + +import static org.opensearch.sql.data.type.ExprCoreType.*; +import static org.opensearch.sql.expression.function.FunctionDSL.define; + +/** + * Utility class that defines and registers IP functions. + */ +@UtilityClass +public class IpFunctions { + + /** + * Registers all IP functions with the given built-in function repository. + */ + public void register(BuiltinFunctionRepository repository) { + repository.register(cidr()); + } + + private DefaultFunctionResolver cidr() { + + FunctionName name = BuiltinFunctionName.CIDR.getName(); + FunctionSignature signature = new FunctionSignature(name, Arrays.asList(STRING, STRING)); + + return define(name, funcName -> Pair.of(signature,(properties, arguments) -> new CidrExpression(arguments))); + } +} diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java index 58482fda2c..bf6b3c22f5 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java @@ -26,7 +26,6 @@ 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; import org.opensearch.sql.utils.OperatorUtils; /** @@ -56,7 +55,6 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(like()); repository.register(notLike()); repository.register(regexp()); - repository.register(cidr()); } /** @@ -398,12 +396,6 @@ private static DefaultFunctionResolver regexp() { impl(nullMissingHandling(OperatorUtils::matchesRegexp), INTEGER, STRING, STRING)); } - private static DefaultFunctionResolver cidr() { - return define( - BuiltinFunctionName.CIDR.getName(), - impl(nullMissingHandling(IPUtils::isAddressInRange), BOOLEAN, STRING, STRING)); - } - private static DefaultFunctionResolver notLike() { return define( BuiltinFunctionName.NOT_LIKE.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 deleted file mode 100644 index 065f158a54..0000000000 --- a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.utils; - -import com.google.common.net.InetAddresses; -import org.opensearch.sql.data.model.ExprBooleanValue; -import org.opensearch.sql.data.model.ExprValue; - -import java.util.Arrays; - -public class IPUtils { - - /** - * Returns whether the given IP address is within the specified IP address range. - * Supports both IPv4 and IPv6 addresses. - * - * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8::/32") - * @return true if address is in range; else false - */ - public static ExprBooleanValue isAddressInRange(ExprValue addressExprValue, ExprValue rangeExprValue) { - - try { - byte[] addressBytes = InetAddresses.forString(addressExprValue.stringValue()).getAddress(); - - String[] rangeFields = rangeExprValue.stringValue().split("/"); - int prefixLengthBytes = Integer.parseInt(rangeFields[1]) / Byte.SIZE; - byte[] rangeBytes = Arrays.copyOfRange(InetAddresses.forString(rangeFields[0]).getAddress(), 0, prefixLengthBytes); - - return ExprBooleanValue.of(Arrays.equals(addressBytes, 0, prefixLengthBytes, rangeBytes, 0, prefixLengthBytes)); - } catch (Exception e) { - return ExprBooleanValue.of(false); - } - } -} diff --git a/core/src/test/java/org/opensearch/sql/expression/ip/CidrExpressionTest.java b/core/src/test/java/org/opensearch/sql/expression/ip/CidrExpressionTest.java new file mode 100644 index 0000000000..d46b916742 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/ip/CidrExpressionTest.java @@ -0,0 +1,104 @@ +package org.opensearch.sql.expression.ip; + +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +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.ExpressionEvaluationException; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.ExpressionTestBase; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.env.Environment; + +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.*; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; + +@ExtendWith(MockitoExtension.class) +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +public class CidrExpressionTest extends ExpressionTestBase { + + // IP range and address constants for testing. + static final ExprValue IPv4Range = ExprValueUtils.stringValue("198.51.100.0/24"); + static final ExprValue IPv6Range = ExprValueUtils.stringValue("2001:0db8::/32"); + + static final ExprValue IPv4AddressBelow = ExprValueUtils.stringValue("198.51.99.1"); + static final ExprValue IPv4AddressWithin = ExprValueUtils.stringValue("198.51.100.1"); + static final ExprValue IPv4AddressAbove = ExprValueUtils.stringValue("198.51.101.2"); + + static final ExprValue IPv6AddressBelow = ExprValueUtils.stringValue("2001:0db7::ff00:42:8329"); + static final ExprValue IPv6AddressWithin = ExprValueUtils.stringValue("2001:0db8::ff00:42:8329"); + static final ExprValue IPv6AddressAbove = ExprValueUtils.stringValue("2001:0db9::ff00:42:8329"); + + // Mock value environment for testing. + @Mock + Environment env; + + @Test + public void test_invalid_num_arguments() { + assertThrows(ExpressionEvaluationException.class, DSL::cidr); + assertThrows(ExpressionEvaluationException.class, () -> DSL.cidr(DSL.literal(0), DSL.literal(0), DSL.literal(0))); + } + + @Test + public void test_null_and_missing() { + assertEquals(LITERAL_NULL, execute(LITERAL_NULL, IPv4Range)); + assertEquals(LITERAL_NULL, execute(LITERAL_MISSING, IPv4Range)); + } + + @Test + public void test_invalid_address() { + assertEquals(LITERAL_NULL, execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); + } + + @Test + public void test_invalid_range() { + assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); + assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID/32"))); + assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("198.51.100.0/33"))); + } + + @Test + public void test_valid_ipv4() { + assertEquals(LITERAL_FALSE, execute(IPv4AddressBelow, IPv4Range)); + assertEquals(LITERAL_TRUE, execute(IPv4AddressWithin, IPv4Range)); + assertEquals(LITERAL_FALSE, execute(IPv4AddressAbove, IPv4Range)); + } + + @Test + public void test_valid_ipv6() { + assertEquals(LITERAL_FALSE, execute(IPv6AddressBelow, IPv6Range)); + assertEquals(LITERAL_TRUE, execute(IPv6AddressWithin, IPv6Range)); + assertEquals(LITERAL_FALSE, execute(IPv6AddressAbove, IPv6Range)); + } + + @Test + public void test_valid_different_versions() { + assertEquals(LITERAL_FALSE, execute(IPv4AddressWithin, IPv6Range)); + assertEquals(LITERAL_FALSE, execute(IPv6AddressWithin, IPv4Range)); + } + + /** + * 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.cidr(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/docs/user/ppl/functions/ip.rst b/docs/user/ppl/functions/ip.rst index cd213918fd..8b9e370198 100644 --- a/docs/user/ppl/functions/ip.rst +++ b/docs/user/ppl/functions/ip.rst @@ -22,12 +22,11 @@ Return type: BOOLEAN Example:: - os> source=devices | where cidr(address, "198.51.100.0/24") - fetched rows / total rows = 2/2 - +----------------+----------------+ - | name | address | - |----------------+----------------+ - | John's Macbook | 198.51.100.2 | - | Iain's PC | 198.51.100.254 | - +----------------+----------------+ + os> source=weblogs | where cidr(address, "199.120.110.0/24") | fields host, method, url + fetched rows / total rows = 1/1 + +----------------+--------+----------------------------------------------+ + | host | method | url | + |----------------+--------+----------------------------------------------+ + | 199.120.110.21 | GET | /shuttle/missions/sts-73/mission-sts-73.html | + +----------------+--------+----------------------------------------------+ From f2ca434b641985b35985587fa0a10ddf7b32863e Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 28 Oct 2024 21:18:39 -0700 Subject: [PATCH 10/40] Refactor to replace `CidrExpression` with `IPFunctions`. Plus preliminary implementation of integration tests (currently failing). Signed-off-by: currantw --- .../function/BuiltinFunctionRepository.java | 4 +- .../function/DefaultFunctionResolver.java | 2 +- .../sql/expression/ip/CidrExpression.java | 139 ------------------ .../sql/expression/ip/IPFunction.java | 94 ++++++++++++ .../sql/expression/ip/IpFunctions.java | 32 ---- ...xpressionTest.java => IPFunctionTest.java} | 30 +--- .../org/opensearch/sql/ppl/IPFunctionIT.java | 40 +++++ 7 files changed, 144 insertions(+), 197 deletions(-) delete mode 100644 core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java create mode 100644 core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java delete mode 100644 core/src/main/java/org/opensearch/sql/expression/ip/IpFunctions.java rename core/src/test/java/org/opensearch/sql/expression/ip/{CidrExpressionTest.java => IPFunctionTest.java} (77%) create mode 100644 integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java index 750a012684..7b6a1d84ee 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java @@ -27,7 +27,7 @@ import org.opensearch.sql.expression.aggregation.AggregatorFunction; import org.opensearch.sql.expression.datetime.DateTimeFunction; import org.opensearch.sql.expression.datetime.IntervalClause; -import org.opensearch.sql.expression.ip.IpFunctions; +import org.opensearch.sql.expression.ip.IPFunction; import org.opensearch.sql.expression.operator.arthmetic.ArithmeticFunction; import org.opensearch.sql.expression.operator.arthmetic.MathematicalFunction; import org.opensearch.sql.expression.operator.convert.TypeCastOperator; @@ -82,7 +82,7 @@ public static synchronized BuiltinFunctionRepository getInstance() { TypeCastOperator.register(instance); SystemFunctions.register(instance); OpenSearchFunctions.register(instance); - IpFunctions.register(instance); + IPFunction.register(instance); } return instance; } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/DefaultFunctionResolver.java b/core/src/main/java/org/opensearch/sql/expression/function/DefaultFunctionResolver.java index 5d0f31594b..e1d0052723 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/DefaultFunctionResolver.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/DefaultFunctionResolver.java @@ -61,7 +61,7 @@ public Pair resolve(FunctionSignature unreso && !FunctionSignature.isVarArgFunction(bestMatchEntry.getValue().getParamTypeList())) { throw new ExpressionEvaluationException( String.format( - "%s function expected %s, but get %s", + "%s function expected %s, but got %s", functionName, formatFunctions(functionBundle.keySet()), unresolvedSignature.formatTypes())); diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java b/core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java deleted file mode 100644 index 7ae74a5af7..0000000000 --- a/core/src/main/java/org/opensearch/sql/expression/ip/CidrExpression.java +++ /dev/null @@ -1,139 +0,0 @@ -package org.opensearch.sql.expression.ip; - -import com.google.common.net.InetAddresses; -import lombok.EqualsAndHashCode; -import lombok.ToString; -import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.model.ExprValueUtils; -import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.data.type.ExprType; -import org.opensearch.sql.exception.ExpressionEvaluationException; -import org.opensearch.sql.exception.SemanticCheckException; -import org.opensearch.sql.expression.Expression; -import org.opensearch.sql.expression.FunctionExpression; -import org.opensearch.sql.expression.env.Environment; -import org.opensearch.sql.expression.function.FunctionName; - -import java.io.Serializable; -import java.math.BigInteger; -import java.net.Inet4Address; -import java.net.InetAddress; -import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -@ToString -@EqualsAndHashCode(callSuper = false) -public class CidrExpression extends FunctionExpression { - - private final Expression addressExpression; - private final InetAddressRange range; - - public CidrExpression(List arguments) { - super(FunctionName.of("cidr"), arguments); - - // Must be exactly two arguments. - if (arguments.size() != 2) { - String msg = String.format("Unexpected number of arguments to function '%s'. Expected %s, but found %s.", FunctionName.of("cidr"), 2, arguments.size()); - throw new ExpressionEvaluationException(msg); - } - - this.addressExpression = arguments.getFirst(); - this.range = new InetAddressRange(arguments.getLast().valueOf().stringValue()); - } - - @Override - public ExprValue valueOf(Environment valueEnv) { - ExprValue addressValue = addressExpression.valueOf(valueEnv); - if (addressValue.isNull() || addressValue.isMissing()) - return ExprValueUtils.nullValue(); - - String addressString = addressValue.stringValue(); - if (!InetAddresses.isInetAddress(addressString)) - return ExprValueUtils.nullValue(); - - InetAddress address = InetAddresses.forString(addressString); - return ExprValueUtils.booleanValue(range.contains(address)); - } - - @Override - public ExprType type() { - return ExprCoreType.BOOLEAN; - } - - /** - * Represents an IP address range. - * Supports both IPv4 and IPv6 addresses. - */ - private class InetAddressRange implements Serializable { - - // Basic CIDR notation pattern. - private static final Pattern cidrPattern = Pattern.compile("(?
.+)[/](?[0-9]+)"); - - // Lower/upper bounds for the IP address range. - private final InetAddress lowerBound; - private final InetAddress upperBound; - - /** - * Builds a new IP address range from the given CIDR notation string. - * - * @param cidr CIDR notation string (e.g. "198.51.100.0/24" or "2001:0db8::/32") - */ - public InetAddressRange(String cidr) { - - // Parse address and network length. - Matcher cidrMatcher = cidrPattern.matcher(cidr); - if (!cidrMatcher.matches()) - throw new SemanticCheckException(String.format("CIDR notation '%s' in not valid", range)); - - String addressString = cidrMatcher.group("address"); - if (!InetAddresses.isInetAddress(addressString)) - throw new SemanticCheckException(String.format("IP address '%s' in not valid", addressString)); - - InetAddress address = InetAddresses.forString(addressString); - - int networkLengthBits = Integer.parseInt(cidrMatcher.group("prefix")); - int addressLengthBits = address.getAddress().length * Byte.SIZE; - - if (networkLengthBits > addressLengthBits) - throw new SemanticCheckException(String.format("Network length of '%s' bits is not valid", networkLengthBits)); - - // Build bounds by converting the address to an integer, setting all the non-significant bits to - // zero for the lower bounds and one for the upper bounds, and then converting back to addresses. - BigInteger lowerBoundInt = InetAddresses.toBigInteger(address); - BigInteger upperBoundInt = InetAddresses.toBigInteger(address); - - int hostLengthBits = addressLengthBits - networkLengthBits; - for (int bit = 0; bit < hostLengthBits; bit++) { - lowerBoundInt = lowerBoundInt.clearBit(bit); - upperBoundInt = upperBoundInt.setBit(bit); - } - - if (address instanceof Inet4Address) { - lowerBound = InetAddresses.fromIPv4BigInteger(lowerBoundInt); - upperBound = InetAddresses.fromIPv4BigInteger(upperBoundInt); - } else { - lowerBound = InetAddresses.fromIPv6BigInteger(lowerBoundInt); - upperBound = InetAddresses.fromIPv6BigInteger(upperBoundInt); - } - } - - /** - * Returns whether the IP address is contained within the range. - * - * @param address IPv4 or IPv6 address, represented as a {@link BigInteger}. - * (see {@link InetAddresses#toBigInteger(InetAddress)}). - */ - public boolean contains(InetAddress address) { - - if ((address instanceof Inet4Address) ^ (lowerBound instanceof Inet4Address)) return false; - - BigInteger addressInt = InetAddresses.toBigInteger(address); - - if (addressInt.compareTo(InetAddresses.toBigInteger(lowerBound)) < 0) return false; - if (addressInt.compareTo(InetAddresses.toBigInteger(upperBound)) <= 0) return false; - - return true; - } - } -} diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java new file mode 100644 index 0000000000..4a6d7625a4 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -0,0 +1,94 @@ +package org.opensearch.sql.expression.ip; + +import com.google.common.net.InetAddresses; +import lombok.experimental.UtilityClass; +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.function.BuiltinFunctionName; +import org.opensearch.sql.expression.function.BuiltinFunctionRepository; +import org.opensearch.sql.expression.function.DefaultFunctionResolver; + +import java.math.BigInteger; +import java.net.Inet4Address; +import java.net.InetAddress; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.opensearch.sql.data.type.ExprCoreType.*; +import static org.opensearch.sql.expression.function.FunctionDSL.*; + +/** + * Utility class that defines and registers IP functions. + */ +@UtilityClass +public class IPFunction { + + private static final Pattern cidrPattern = Pattern.compile("(?
.+)[/](?[0-9]+)"); + + public void register(BuiltinFunctionRepository repository) { + repository.register(cidr()); + } + + private DefaultFunctionResolver cidr() { + return define( + BuiltinFunctionName.CIDR.getName(), + impl(nullMissingHandling(IPFunction::exprCidr), BOOLEAN, STRING, STRING)); + } + + /** + * Returns whether the given IP address is within the specified IP address range. + * Supports both IPv4 and IPv6 addresses. + * + * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8::/32") + * @return null if the address is not valid; true if the address is in the range; otherwise false. + * @throws SemanticCheckException if the range is not valid + */ + private ExprValue exprCidr(ExprValue addressExprValue, ExprValue rangeExprValue) { + + // Get address + String addressString = addressExprValue.stringValue(); + if (!InetAddresses.isInetAddress(addressString)) + return ExprValueUtils.nullValue(); + + InetAddress address = InetAddresses.forString(addressString); + + // Get range and network length + String rangeString = rangeExprValue.stringValue(); + + Matcher cidrMatcher = cidrPattern.matcher(rangeString); + if (!cidrMatcher.matches()) + throw new SemanticCheckException(String.format("CIDR notation '%s' in not valid", rangeString)); + + String rangeAddressString = cidrMatcher.group("address"); + if (!InetAddresses.isInetAddress(rangeAddressString)) + throw new SemanticCheckException(String.format("IP address '%s' in not valid", rangeAddressString)); + + InetAddress rangeAddress = InetAddresses.forString(rangeAddressString); + + if ((address instanceof Inet4Address) ^ (rangeAddress instanceof Inet4Address)) + return ExprValueUtils.booleanValue(false); + + int networkLengthBits = Integer.parseInt(cidrMatcher.group("networkLength")); + int addressLengthBits = address.getAddress().length * Byte.SIZE; + + if (networkLengthBits > addressLengthBits) + throw new SemanticCheckException(String.format("Network length of '%s' bits is not valid", networkLengthBits)); + + // Build bounds by converting the address to an integer, setting all the non-significant bits to + // zero for the lower bounds and one for the upper bounds, and then converting back to addresses. + BigInteger lowerBoundInt = InetAddresses.toBigInteger(rangeAddress); + BigInteger upperBoundInt = InetAddresses.toBigInteger(rangeAddress); + + int hostLengthBits = addressLengthBits - networkLengthBits; + for (int bit = 0; bit < hostLengthBits; bit++) { + lowerBoundInt = lowerBoundInt.clearBit(bit); + upperBoundInt = upperBoundInt.setBit(bit); + } + + // Convert the address to an integer and compare it to the bounds. + BigInteger addressInt = InetAddresses.toBigInteger(address); + return ExprValueUtils.booleanValue((addressInt.compareTo(lowerBoundInt) >= 0) && (addressInt.compareTo(upperBoundInt) <= 0)); + } +} 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 deleted file mode 100644 index 5582ff89ef..0000000000 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IpFunctions.java +++ /dev/null @@ -1,32 +0,0 @@ -package org.opensearch.sql.expression.ip; - -import lombok.experimental.UtilityClass; -import org.apache.commons.lang3.tuple.Pair; -import org.opensearch.sql.expression.function.*; - -import java.util.Arrays; - -import static org.opensearch.sql.data.type.ExprCoreType.*; -import static org.opensearch.sql.expression.function.FunctionDSL.define; - -/** - * Utility class that defines and registers IP functions. - */ -@UtilityClass -public class IpFunctions { - - /** - * Registers all IP functions with the given built-in function repository. - */ - public void register(BuiltinFunctionRepository repository) { - repository.register(cidr()); - } - - private DefaultFunctionResolver cidr() { - - FunctionName name = BuiltinFunctionName.CIDR.getName(); - FunctionSignature signature = new FunctionSignature(name, Arrays.asList(STRING, STRING)); - - return define(name, funcName -> Pair.of(signature,(properties, arguments) -> new CidrExpression(arguments))); - } -} diff --git a/core/src/test/java/org/opensearch/sql/expression/ip/CidrExpressionTest.java b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java similarity index 77% rename from core/src/test/java/org/opensearch/sql/expression/ip/CidrExpressionTest.java rename to core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java index d46b916742..d16e6cfe91 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/CidrExpressionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -1,18 +1,14 @@ package org.opensearch.sql.expression.ip; -import org.junit.jupiter.api.DisplayNameGeneration; -import org.junit.jupiter.api.DisplayNameGenerator; 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.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; -import org.opensearch.sql.expression.ExpressionTestBase; import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.env.Environment; @@ -23,8 +19,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.STRING; @ExtendWith(MockitoExtension.class) -@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) -public class CidrExpressionTest extends ExpressionTestBase { +public class IPFunctionTest { // IP range and address constants for testing. static final ExprValue IPv4Range = ExprValueUtils.stringValue("198.51.100.0/24"); @@ -43,45 +38,33 @@ public class CidrExpressionTest extends ExpressionTestBase { Environment env; @Test - public void test_invalid_num_arguments() { - assertThrows(ExpressionEvaluationException.class, DSL::cidr); - assertThrows(ExpressionEvaluationException.class, () -> DSL.cidr(DSL.literal(0), DSL.literal(0), DSL.literal(0))); - } - - @Test - public void test_null_and_missing() { - assertEquals(LITERAL_NULL, execute(LITERAL_NULL, IPv4Range)); - assertEquals(LITERAL_NULL, execute(LITERAL_MISSING, IPv4Range)); - } - - @Test - public void test_invalid_address() { + public void cidr_invalid_address() { assertEquals(LITERAL_NULL, execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); } @Test - public void test_invalid_range() { + public void cidr_invalid_range() { assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID/32"))); assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("198.51.100.0/33"))); } @Test - public void test_valid_ipv4() { + public void cidr_valid_ipv4() { assertEquals(LITERAL_FALSE, execute(IPv4AddressBelow, IPv4Range)); assertEquals(LITERAL_TRUE, execute(IPv4AddressWithin, IPv4Range)); assertEquals(LITERAL_FALSE, execute(IPv4AddressAbove, IPv4Range)); } @Test - public void test_valid_ipv6() { + public void cidr_valid_ipv6() { assertEquals(LITERAL_FALSE, execute(IPv6AddressBelow, IPv6Range)); assertEquals(LITERAL_TRUE, execute(IPv6AddressWithin, IPv6Range)); assertEquals(LITERAL_FALSE, execute(IPv6AddressAbove, IPv6Range)); } @Test - public void test_valid_different_versions() { + public void cidr_valid_different_versions() { assertEquals(LITERAL_FALSE, execute(IPv4AddressWithin, IPv6Range)); assertEquals(LITERAL_FALSE, execute(IPv6AddressWithin, IPv4Range)); } @@ -101,4 +84,5 @@ private ExprValue execute(ExprValue field, ExprValue range) { return exp.valueOf(env); } + } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java new file mode 100644 index 0000000000..d502643f47 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -0,0 +1,40 @@ +package org.opensearch.sql.ppl; + +import org.json.JSONObject; +import org.junit.jupiter.api.Test; + +import java.io.IOException; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOG; +import static org.opensearch.sql.util.MatcherUtils.*; + +public class IPFunctionIT extends PPLIntegTestCase { + + @Override + public void init() throws IOException { + loadIndex(Index.WEBLOG); + } + + @Test + public void testCidr() throws IOException { + + JSONObject result; + + // No matches + result = executeQuery(String.format("source=%s | where cidr(host, '199.120.111.0/24') | fields url", TEST_INDEX_WEBLOG)); + verifySchema(result, schema("url", null, "boolean")); + verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); + + // One match + result = executeQuery(String.format("source=%s | where cidr(host, '199.120.110.0/24') | fields url", TEST_INDEX_WEBLOG)); + verifySchema(result, schema("url", null, "boolean")); + verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); + + // Multiple matches + result = executeQuery(String.format("source=%s | where cidr(host, '199.0.0.0/8') | fields url", TEST_INDEX_WEBLOG)); + verifySchema(result, schema("url", null, "boolean")); + verifyDataRows(result, + rows("/history/apollo/"), + rows("/shuttle/missions/sts-73/mission-sts-73.html")); + } +} From efe26b819f4f8778fe71df07a5cd078254aa5d6b Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 29 Oct 2024 10:04:02 -0700 Subject: [PATCH 11/40] Address review comments - minor code cleanup. Signed-off-by: currantw --- .../sql/expression/ip/IPFunction.java | 20 +++++++++----- .../sql/expression/ip/IPFunctionTest.java | 23 +++++++++------- .../BinaryPredicateOperatorTest.java | 26 ------------------- .../org/opensearch/sql/ppl/IPFunctionIT.java | 9 ++++--- 4 files changed, 33 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index 4a6d7625a4..b87e9cf795 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.expression.ip; import com.google.common.net.InetAddresses; @@ -10,12 +15,12 @@ import org.opensearch.sql.expression.function.DefaultFunctionResolver; import java.math.BigInteger; -import java.net.Inet4Address; import java.net.InetAddress; import java.util.regex.Matcher; import java.util.regex.Pattern; -import static org.opensearch.sql.data.type.ExprCoreType.*; +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.expression.function.FunctionDSL.*; /** @@ -31,9 +36,7 @@ public void register(BuiltinFunctionRepository repository) { } private DefaultFunctionResolver cidr() { - return define( - BuiltinFunctionName.CIDR.getName(), - impl(nullMissingHandling(IPFunction::exprCidr), BOOLEAN, STRING, STRING)); + return define(BuiltinFunctionName.CIDR.getName(), impl(nullMissingHandling(IPFunction::exprCidr), BOOLEAN, STRING, STRING)); } /** @@ -49,8 +52,9 @@ private ExprValue exprCidr(ExprValue addressExprValue, ExprValue rangeExprValue) // Get address String addressString = addressExprValue.stringValue(); - if (!InetAddresses.isInetAddress(addressString)) + if (!InetAddresses.isInetAddress(addressString)) { return ExprValueUtils.nullValue(); + } InetAddress address = InetAddresses.forString(addressString); @@ -67,8 +71,10 @@ private ExprValue exprCidr(ExprValue addressExprValue, ExprValue rangeExprValue) InetAddress rangeAddress = InetAddresses.forString(rangeAddressString); - if ((address instanceof Inet4Address) ^ (rangeAddress instanceof Inet4Address)) + // Address and range must use the same IP version (IPv4 or IPv6). + if (address.getClass().equals(rangeAddress.getClass())) { return ExprValueUtils.booleanValue(false); + } int networkLengthBits = Integer.parseInt(cidrMatcher.group("networkLength")); int addressLengthBits = address.getAddress().length * Byte.SIZE; 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 index d16e6cfe91..cd1c78287c 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.expression.ip; import org.junit.jupiter.api.Test; @@ -22,20 +27,20 @@ public class IPFunctionTest { // IP range and address constants for testing. - static final ExprValue IPv4Range = ExprValueUtils.stringValue("198.51.100.0/24"); - static final ExprValue IPv6Range = ExprValueUtils.stringValue("2001:0db8::/32"); + private static final ExprValue IPv4Range = ExprValueUtils.stringValue("198.51.100.0/24"); + private static final ExprValue IPv6Range = ExprValueUtils.stringValue("2001:0db8::/32"); - static final ExprValue IPv4AddressBelow = ExprValueUtils.stringValue("198.51.99.1"); - static final ExprValue IPv4AddressWithin = ExprValueUtils.stringValue("198.51.100.1"); - static final ExprValue IPv4AddressAbove = ExprValueUtils.stringValue("198.51.101.2"); + 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"); - static final ExprValue IPv6AddressBelow = ExprValueUtils.stringValue("2001:0db7::ff00:42:8329"); - static final ExprValue IPv6AddressWithin = ExprValueUtils.stringValue("2001:0db8::ff00:42:8329"); - static final ExprValue IPv6AddressAbove = ExprValueUtils.stringValue("2001:0db9::ff00:42:8329"); + 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 - Environment env; + private Environment env; @Test public void cidr_invalid_address() { diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index f63d96d2f9..19cbb4674e 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -209,24 +209,6 @@ private static Stream testLikeArguments() { return builder.build(); } - private static Stream getCidrArguments() { - return Stream.of( - Arguments.of("10.24.33.5", "10.24.34.0/24", false), // IPv4 less - Arguments.of("10.24.34.5", "10.24.34.0/24", true), // IPv4 between - Arguments.of("10.24.35.5", "10.24.34.0/24", false), // IPv4 greater - Arguments.of("10.24.35.5", "10.24.34.0/33", false), // IPv4 prefix too long - - Arguments.of("2001:0db7::8329", "2001:0db8::/32", false), // IPv6 less - Arguments.of("2001:0db8::8329", "2001:0db8::/32", true), // IPv6 between - Arguments.of("2001:0db9::8329", "2001:0db8::/32", false), // IPv6 greater - Arguments.of("2001:0db9::8329", "2001:0db8::/129", false), // IPv6 prefix too long - - Arguments.of("INVALID", "10.24.34.0/24", false), // Invalid address - Arguments.of("10.24.34.5", "INVALID", false), // Invalid range - Arguments.of("10.24.34.5", "10.24.34.0/INVALID", false) // Invalid prefix - ); - } - @ParameterizedTest(name = "and({0}, {1})") @MethodSource("binaryPredicateArguments") public void test_and(Boolean v1, Boolean v2) { @@ -633,12 +615,4 @@ public void compare_int_long() { FunctionExpression equal = DSL.equal(DSL.literal(1), DSL.literal(1L)); assertTrue(equal.valueOf(valueEnv()).booleanValue()); } - - @ParameterizedTest - @MethodSource("getCidrArguments") - public void test_cidr(String address, String range, boolean expected) { - FunctionExpression cidr = DSL.cidr(DSL.literal(address), DSL.literal(range)); - assertEquals(cidr.type(), BOOLEAN); - assertEquals(cidr.valueOf().booleanValue(), expected); - } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java index d502643f47..88b9948404 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.ppl; import org.json.JSONObject; @@ -33,8 +38,6 @@ public void testCidr() throws IOException { // Multiple matches result = executeQuery(String.format("source=%s | where cidr(host, '199.0.0.0/8') | fields url", TEST_INDEX_WEBLOG)); verifySchema(result, schema("url", null, "boolean")); - verifyDataRows(result, - rows("/history/apollo/"), - rows("/shuttle/missions/sts-73/mission-sts-73.html")); + verifyDataRows(result, rows("/history/apollo/"), rows("/shuttle/missions/sts-73/mission-sts-73.html")); } } From 824c4638898fe357b9755a234a8a180c0ba06219 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 29 Oct 2024 12:00:59 -0700 Subject: [PATCH 12/40] Fix failing `IPFunction` unit tests. Signed-off-by: currantw --- .../main/java/org/opensearch/sql/expression/ip/IPFunction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index b87e9cf795..ce5e6b8c92 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -72,7 +72,7 @@ private ExprValue exprCidr(ExprValue addressExprValue, ExprValue rangeExprValue) InetAddress rangeAddress = InetAddresses.forString(rangeAddressString); // Address and range must use the same IP version (IPv4 or IPv6). - if (address.getClass().equals(rangeAddress.getClass())) { + if (!address.getClass().equals(rangeAddress.getClass())) { return ExprValueUtils.booleanValue(false); } From 3ffac2e2ca735b83b585aa2741ec4c18beccfb61 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 29 Oct 2024 12:13:26 -0700 Subject: [PATCH 13/40] Reformat new files with Spotless. Signed-off-by: currantw --- .../sql/expression/ip/IPFunction.java | 156 +++++++++--------- .../sql/expression/ip/IPFunctionTest.java | 147 +++++++++-------- .../org/opensearch/sql/ppl/IPFunctionIT.java | 71 ++++---- 3 files changed, 199 insertions(+), 175 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index ce5e6b8c92..9349ed02db 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -5,7 +5,15 @@ package org.opensearch.sql.expression.ip; +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; +import static org.opensearch.sql.expression.function.FunctionDSL.*; + import com.google.common.net.InetAddresses; +import java.math.BigInteger; +import java.net.InetAddress; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; @@ -14,87 +22,85 @@ import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.DefaultFunctionResolver; -import java.math.BigInteger; -import java.net.InetAddress; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; -import static org.opensearch.sql.expression.function.FunctionDSL.*; - -/** - * Utility class that defines and registers IP functions. - */ +/** Utility class that defines and registers IP functions. */ @UtilityClass public class IPFunction { - private static final Pattern cidrPattern = Pattern.compile("(?
.+)[/](?[0-9]+)"); - - public void register(BuiltinFunctionRepository repository) { - repository.register(cidr()); + private static final Pattern cidrPattern = + Pattern.compile("(?
.+)[/](?[0-9]+)"); + + public void register(BuiltinFunctionRepository repository) { + repository.register(cidr()); + } + + private DefaultFunctionResolver cidr() { + return define( + BuiltinFunctionName.CIDR.getName(), + impl(nullMissingHandling(IPFunction::exprCidr), BOOLEAN, STRING, STRING)); + } + + /** + * Returns whether the given IP address is within the specified IP address range. Supports both + * IPv4 and IPv6 addresses. + * + * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or + * "2001:0db8::/32") + * @return null if the address is not valid; true if the address is in the range; otherwise false. + * @throws SemanticCheckException if the range is not valid + */ + private ExprValue exprCidr(ExprValue addressExprValue, ExprValue rangeExprValue) { + + // Get address + String addressString = addressExprValue.stringValue(); + if (!InetAddresses.isInetAddress(addressString)) { + return ExprValueUtils.nullValue(); } - private DefaultFunctionResolver cidr() { - return define(BuiltinFunctionName.CIDR.getName(), impl(nullMissingHandling(IPFunction::exprCidr), BOOLEAN, STRING, STRING)); + InetAddress address = InetAddresses.forString(addressString); + + // Get range and network length + String rangeString = rangeExprValue.stringValue(); + + Matcher cidrMatcher = cidrPattern.matcher(rangeString); + if (!cidrMatcher.matches()) + throw new SemanticCheckException( + String.format("CIDR notation '%s' in not valid", rangeString)); + + String rangeAddressString = cidrMatcher.group("address"); + if (!InetAddresses.isInetAddress(rangeAddressString)) + throw new SemanticCheckException( + String.format("IP address '%s' in not valid", rangeAddressString)); + + InetAddress rangeAddress = InetAddresses.forString(rangeAddressString); + + // Address and range must use the same IP version (IPv4 or IPv6). + if (!address.getClass().equals(rangeAddress.getClass())) { + return ExprValueUtils.booleanValue(false); } - /** - * Returns whether the given IP address is within the specified IP address range. - * Supports both IPv4 and IPv6 addresses. - * - * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or "2001:0db8::/32") - * @return null if the address is not valid; true if the address is in the range; otherwise false. - * @throws SemanticCheckException if the range is not valid - */ - private ExprValue exprCidr(ExprValue addressExprValue, ExprValue rangeExprValue) { - - // Get address - String addressString = addressExprValue.stringValue(); - if (!InetAddresses.isInetAddress(addressString)) { - return ExprValueUtils.nullValue(); - } - - InetAddress address = InetAddresses.forString(addressString); - - // Get range and network length - String rangeString = rangeExprValue.stringValue(); - - Matcher cidrMatcher = cidrPattern.matcher(rangeString); - if (!cidrMatcher.matches()) - throw new SemanticCheckException(String.format("CIDR notation '%s' in not valid", rangeString)); - - String rangeAddressString = cidrMatcher.group("address"); - if (!InetAddresses.isInetAddress(rangeAddressString)) - throw new SemanticCheckException(String.format("IP address '%s' in not valid", rangeAddressString)); - - InetAddress rangeAddress = InetAddresses.forString(rangeAddressString); - - // Address and range must use the same IP version (IPv4 or IPv6). - if (!address.getClass().equals(rangeAddress.getClass())) { - return ExprValueUtils.booleanValue(false); - } - - int networkLengthBits = Integer.parseInt(cidrMatcher.group("networkLength")); - int addressLengthBits = address.getAddress().length * Byte.SIZE; - - if (networkLengthBits > addressLengthBits) - throw new SemanticCheckException(String.format("Network length of '%s' bits is not valid", networkLengthBits)); - - // Build bounds by converting the address to an integer, setting all the non-significant bits to - // zero for the lower bounds and one for the upper bounds, and then converting back to addresses. - BigInteger lowerBoundInt = InetAddresses.toBigInteger(rangeAddress); - BigInteger upperBoundInt = InetAddresses.toBigInteger(rangeAddress); - - int hostLengthBits = addressLengthBits - networkLengthBits; - for (int bit = 0; bit < hostLengthBits; bit++) { - lowerBoundInt = lowerBoundInt.clearBit(bit); - upperBoundInt = upperBoundInt.setBit(bit); - } - - // Convert the address to an integer and compare it to the bounds. - BigInteger addressInt = InetAddresses.toBigInteger(address); - return ExprValueUtils.booleanValue((addressInt.compareTo(lowerBoundInt) >= 0) && (addressInt.compareTo(upperBoundInt) <= 0)); + int networkLengthBits = Integer.parseInt(cidrMatcher.group("networkLength")); + int addressLengthBits = address.getAddress().length * Byte.SIZE; + + if (networkLengthBits > addressLengthBits) + throw new SemanticCheckException( + String.format("Network length of '%s' bits is not valid", networkLengthBits)); + + // Build bounds by converting the address to an integer, setting all the non-significant bits to + // zero for the lower bounds and one for the upper bounds, and then converting back to + // addresses. + BigInteger lowerBoundInt = InetAddresses.toBigInteger(rangeAddress); + BigInteger upperBoundInt = InetAddresses.toBigInteger(rangeAddress); + + int hostLengthBits = addressLengthBits - networkLengthBits; + for (int bit = 0; bit < hostLengthBits; bit++) { + lowerBoundInt = lowerBoundInt.clearBit(bit); + upperBoundInt = upperBoundInt.setBit(bit); } + + // Convert the address to an integer and compare it to the bounds. + BigInteger addressInt = InetAddresses.toBigInteger(address); + return ExprValueUtils.booleanValue( + (addressInt.compareTo(lowerBoundInt) >= 0) && (addressInt.compareTo(upperBoundInt) <= 0)); + } } 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 index cd1c78287c..149a6b87c1 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -5,6 +5,12 @@ 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.*; +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; @@ -17,77 +23,78 @@ import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.env.Environment; -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.*; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; - @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"); - - 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 cidr_invalid_address() { - assertEquals(LITERAL_NULL, execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); - } - - @Test - public void cidr_invalid_range() { - assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); - assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID/32"))); - assertThrows(SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("198.51.100.0/33"))); - } - - @Test - public void cidr_valid_ipv4() { - assertEquals(LITERAL_FALSE, execute(IPv4AddressBelow, IPv4Range)); - assertEquals(LITERAL_TRUE, execute(IPv4AddressWithin, IPv4Range)); - assertEquals(LITERAL_FALSE, execute(IPv4AddressAbove, IPv4Range)); - } - - @Test - public void cidr_valid_ipv6() { - assertEquals(LITERAL_FALSE, execute(IPv6AddressBelow, IPv6Range)); - assertEquals(LITERAL_TRUE, execute(IPv6AddressWithin, IPv6Range)); - assertEquals(LITERAL_FALSE, execute(IPv6AddressAbove, IPv6Range)); - } - - @Test - public void cidr_valid_different_versions() { - assertEquals(LITERAL_FALSE, execute(IPv4AddressWithin, IPv6Range)); - assertEquals(LITERAL_FALSE, execute(IPv6AddressWithin, IPv4Range)); - } - - /** - * 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.cidr(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); - } - + // 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"); + + 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 cidr_invalid_address() { + assertEquals(LITERAL_NULL, execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); + } + + @Test + public void cidr_invalid_range() { + assertThrows( + SemanticCheckException.class, + () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); + assertThrows( + SemanticCheckException.class, + () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID/32"))); + assertThrows( + SemanticCheckException.class, + () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("198.51.100.0/33"))); + } + + @Test + public void cidr_valid_ipv4() { + assertEquals(LITERAL_FALSE, execute(IPv4AddressBelow, IPv4Range)); + assertEquals(LITERAL_TRUE, execute(IPv4AddressWithin, IPv4Range)); + assertEquals(LITERAL_FALSE, execute(IPv4AddressAbove, IPv4Range)); + } + + @Test + public void cidr_valid_ipv6() { + assertEquals(LITERAL_FALSE, execute(IPv6AddressBelow, IPv6Range)); + assertEquals(LITERAL_TRUE, execute(IPv6AddressWithin, IPv6Range)); + assertEquals(LITERAL_FALSE, execute(IPv6AddressAbove, IPv6Range)); + } + + @Test + public void cidr_valid_different_versions() { + assertEquals(LITERAL_FALSE, execute(IPv4AddressWithin, IPv6Range)); + assertEquals(LITERAL_FALSE, execute(IPv6AddressWithin, IPv4Range)); + } + + /** + * 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.cidr(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/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java index 88b9948404..fee60b23a7 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -5,39 +5,50 @@ package org.opensearch.sql.ppl; -import org.json.JSONObject; -import org.junit.jupiter.api.Test; - -import java.io.IOException; - import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOG; import static org.opensearch.sql.util.MatcherUtils.*; -public class IPFunctionIT extends PPLIntegTestCase { - - @Override - public void init() throws IOException { - loadIndex(Index.WEBLOG); - } - - @Test - public void testCidr() throws IOException { - - JSONObject result; - - // No matches - result = executeQuery(String.format("source=%s | where cidr(host, '199.120.111.0/24') | fields url", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("url", null, "boolean")); - verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; - // One match - result = executeQuery(String.format("source=%s | where cidr(host, '199.120.110.0/24') | fields url", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("url", null, "boolean")); - verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); +public class IPFunctionIT extends PPLIntegTestCase { - // Multiple matches - result = executeQuery(String.format("source=%s | where cidr(host, '199.0.0.0/8') | fields url", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("url", null, "boolean")); - verifyDataRows(result, rows("/history/apollo/"), rows("/shuttle/missions/sts-73/mission-sts-73.html")); - } + @Override + public void init() throws IOException { + loadIndex(Index.WEBLOG); + } + + @Test + public void testCidr() throws IOException { + + JSONObject result; + + // No matches + result = + executeQuery( + String.format( + "source=%s | where cidr(host, '199.120.111.0/24') | fields url", + TEST_INDEX_WEBLOG)); + verifySchema(result, schema("url", null, "boolean")); + verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); + + // One match + result = + executeQuery( + String.format( + "source=%s | where cidr(host, '199.120.110.0/24') | fields url", + TEST_INDEX_WEBLOG)); + verifySchema(result, schema("url", null, "boolean")); + verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); + + // Multiple matches + result = + executeQuery( + String.format( + "source=%s | where cidr(host, '199.0.0.0/8') | fields url", TEST_INDEX_WEBLOG)); + verifySchema(result, schema("url", null, "boolean")); + verifyDataRows( + result, rows("/history/apollo/"), rows("/shuttle/missions/sts-73/mission-sts-73.html")); + } } From 128bb4be39622212b29320fa46da75bae0973aaf Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 29 Oct 2024 15:26:23 -0700 Subject: [PATCH 14/40] Fix typos in `UnaryPredicateOperator`. Signed-off-by: currantw --- .../predicate/UnaryPredicateOperator.java | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java index 9c097217ce..8889387d1f 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java @@ -5,26 +5,20 @@ package org.opensearch.sql.expression.operator.predicate; -import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; -import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; -import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; -import static org.opensearch.sql.expression.function.FunctionDSL.impl; - -import java.util.Arrays; -import java.util.List; -import java.util.stream.Collectors; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprBooleanValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; -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.expression.function.FunctionBuilder; -import org.opensearch.sql.expression.function.FunctionDSL; -import org.opensearch.sql.expression.function.FunctionName; -import org.opensearch.sql.expression.function.FunctionSignature; -import org.opensearch.sql.expression.function.SerializableFunction; +import org.opensearch.sql.expression.function.*; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.expression.function.FunctionDSL.impl; /** * The definition of unary predicate function not, Accepts one Boolean value and produces a Boolean. @@ -148,8 +142,8 @@ private static DefaultFunctionResolver nullIf() { /** * v2 if v1 is null. * - * @param v1 varable 1 - * @param v2 varable 2 + * @param v1 variable 1 + * @param v2 variable 2 * @return v2 if v1 is null */ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { @@ -157,10 +151,10 @@ public static ExprValue exprIfNull(ExprValue v1, ExprValue v2) { } /** - * return null if v1 equls to v2. + * return null if v1 equals to v2. * - * @param v1 varable 1 - * @param v2 varable 2 + * @param v1 variable 1 + * @param v2 variable 2 * @return null if v1 equals to v2 */ public static ExprValue exprNullIf(ExprValue v1, ExprValue v2) { From 3f267a7647aa12e47c08541d95b7d20044e66d07 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 29 Oct 2024 15:45:54 -0700 Subject: [PATCH 15/40] Rename `cidr` to `cidrmatch` for consistency with Spark (see issue #706). Signed-off-by: currantw --- .../main/java/org/opensearch/sql/expression/DSL.java | 7 ++++--- .../sql/expression/function/BuiltinFunctionName.java | 2 +- .../org/opensearch/sql/expression/ip/IPFunction.java | 10 +++++----- .../opensearch/sql/expression/ip/IPFunctionTest.java | 12 ++++++------ .../java/org/opensearch/sql/ppl/IPFunctionIT.java | 2 +- ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 2 +- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 2 +- 7 files changed, 19 insertions(+), 18 deletions(-) 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 f987211355..3e694e0b7a 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -5,7 +5,6 @@ package org.opensearch.sql.expression; -import java.util.Arrays; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.data.model.ExprShortValue; import org.opensearch.sql.data.model.ExprValue; @@ -26,6 +25,8 @@ import org.opensearch.sql.expression.span.SpanExpression; import org.opensearch.sql.expression.window.ranking.RankingWindowFunction; +import java.util.Arrays; + public class DSL { private DSL() {} @@ -563,8 +564,8 @@ public static FunctionExpression regexp(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.REGEXP, expressions); } - public static FunctionExpression cidr(Expression... expressions) { - return compile(FunctionProperties.None, BuiltinFunctionName.CIDR, expressions); + public static FunctionExpression cidrmatch(Expression... expressions) { + return compile(FunctionProperties.None, BuiltinFunctionName.CIDRMATCH, expressions); } public static FunctionExpression concat(Expression... expressions) { 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 cc4e5b6d8d..a67308c96a 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 @@ -133,7 +133,7 @@ public enum BuiltinFunctionName { TOSTRING(FunctionName.of("tostring")), /** IP Functions. */ - CIDR(FunctionName.of("cidr")), + CIDRMATCH(FunctionName.of("cidrmatch")), /** Arithmetic Operators. */ ADD(FunctionName.of("+")), diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index 9349ed02db..40d1ec10ed 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -30,13 +30,13 @@ public class IPFunction { Pattern.compile("(?
.+)[/](?[0-9]+)"); public void register(BuiltinFunctionRepository repository) { - repository.register(cidr()); + repository.register(cidrmatch()); } - private DefaultFunctionResolver cidr() { + private DefaultFunctionResolver cidrmatch() { return define( - BuiltinFunctionName.CIDR.getName(), - impl(nullMissingHandling(IPFunction::exprCidr), BOOLEAN, STRING, STRING)); + BuiltinFunctionName.CIDRMATCH.getName(), + impl(nullMissingHandling(IPFunction::exprCidrMatch), BOOLEAN, STRING, STRING)); } /** @@ -49,7 +49,7 @@ private DefaultFunctionResolver cidr() { * @return null if the address is not valid; true if the address is in the range; otherwise false. * @throws SemanticCheckException if the range is not valid */ - private ExprValue exprCidr(ExprValue addressExprValue, ExprValue rangeExprValue) { + private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprValue) { // Get address String addressString = addressExprValue.stringValue(); 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 index 149a6b87c1..96ca53ee5a 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -45,12 +45,12 @@ public class IPFunctionTest { @Mock private Environment env; @Test - public void cidr_invalid_address() { + public void cidrmatch_invalid_address() { assertEquals(LITERAL_NULL, execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); } @Test - public void cidr_invalid_range() { + public void cidrmatch_invalid_range() { assertThrows( SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); @@ -63,21 +63,21 @@ public void cidr_invalid_range() { } @Test - public void cidr_valid_ipv4() { + 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 cidr_valid_ipv6() { + public void cidrmatch_valid_ipv6() { assertEquals(LITERAL_FALSE, execute(IPv6AddressBelow, IPv6Range)); assertEquals(LITERAL_TRUE, execute(IPv6AddressWithin, IPv6Range)); assertEquals(LITERAL_FALSE, execute(IPv6AddressAbove, IPv6Range)); } @Test - public void cidr_valid_different_versions() { + public void cidrmatch_valid_different_versions() { assertEquals(LITERAL_FALSE, execute(IPv4AddressWithin, IPv6Range)); assertEquals(LITERAL_FALSE, execute(IPv6AddressWithin, IPv4Range)); } @@ -89,7 +89,7 @@ public void cidr_valid_different_versions() { private ExprValue execute(ExprValue field, ExprValue range) { final String fieldName = "ip_address"; - FunctionExpression exp = DSL.cidr(DSL.ref(fieldName, STRING), DSL.literal(range)); + 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. diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java index fee60b23a7..b5a0cdd716 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -20,7 +20,7 @@ public void init() throws IOException { } @Test - public void testCidr() throws IOException { + public void test_cidrmatch() throws IOException { JSONObject result; diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index 7446a6e6e2..21cee12675 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -322,7 +322,7 @@ CAST: 'CAST'; LIKE: 'LIKE'; ISNULL: 'ISNULL'; ISNOTNULL: 'ISNOTNULL'; -CIDR: 'CIDR'; +CIDRMATCH: 'CIDRMATCH'; // FLOWCONTROL FUNCTIONS IFNULL: 'IFNULL'; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 579a933171..754b7e17a7 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -629,7 +629,7 @@ conditionFunctionName : LIKE | ISNULL | ISNOTNULL - | CIDR + | CIDRMATCH ; // flow control function return non-boolean value From d473fc7761869d3452ddb35ac6b89686c944509c Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 29 Oct 2024 16:24:35 -0700 Subject: [PATCH 16/40] Minor documentation to better align with Spark `cidrmatch` behaviour. Signed-off-by: currantw --- .../sql/expression/ip/IPFunction.java | 4 +-- docs/user/ppl/functions/condition.rst | 4 +-- docs/user/ppl/functions/ip.rst | 26 +++++++++---------- docs/user/ppl/index.rst | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index 40d1ec10ed..c7f81bf564 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -40,8 +40,8 @@ private DefaultFunctionResolver cidrmatch() { } /** - * Returns whether the given IP address is within the specified IP address range. Supports both - * IPv4 and IPv6 addresses. + * Returns whether the given IP address is within the specified CIDR IP address range. + * Supports both IPv4 and IPv6 addresses. * * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or diff --git a/docs/user/ppl/functions/condition.rst b/docs/user/ppl/functions/condition.rst index 96c3e64e72..9ce130072e 100644 --- a/docs/user/ppl/functions/condition.rst +++ b/docs/user/ppl/functions/condition.rst @@ -101,7 +101,7 @@ NULLIF Description >>>>>>>>>>> -Usage: nullif(field1, field2) return null if two parameters are same, otherwiser return field1. +Usage: nullif(field1, field2) return null if two parameters are same, otherwise return field1. Argument type: all the supported data type, (NOTE : if two parameters has different type, if two parameters has different type, you will fail semantic check) @@ -152,7 +152,7 @@ IF Description >>>>>>>>>>> -Usage: if(condition, expr1, expr2) return expr1 if condition is true, otherwiser return expr2. +Usage: if(condition, expr1, expr2) return expr1 if condition is true, otherwise return expr2. Argument type: all the supported data type, (NOTE : if expr1 and expr2 are different type, you will fail semantic check diff --git a/docs/user/ppl/functions/ip.rst b/docs/user/ppl/functions/ip.rst index 8b9e370198..f6c5d3e787 100644 --- a/docs/user/ppl/functions/ip.rst +++ b/docs/user/ppl/functions/ip.rst @@ -1,6 +1,6 @@ -================ -IP Functions -================ +==================== +IP Address Functions +==================== .. rubric:: Table of contents @@ -8,13 +8,13 @@ IP Functions :local: :depth: 1 -CIDR ------- +CIDRMATCH +--------- Description >>>>>>>>>>> -Usage: cidr(address, range) returns whether the given IP address is within the specified IP address range. Supports both IPv4 and IPv6 addresses. +Usage: cidrmatch(ip, cidr) returns whether the given IP address is within the specified CIDR IP address range. Supports both IPv4 and IPv6 addresses. Argument type: STRING, STRING @@ -22,11 +22,11 @@ Return type: BOOLEAN Example:: - os> source=weblogs | where cidr(address, "199.120.110.0/24") | fields host, method, url - fetched rows / total rows = 1/1 - +----------------+--------+----------------------------------------------+ - | host | method | url | - |----------------+--------+----------------------------------------------+ - | 199.120.110.21 | GET | /shuttle/missions/sts-73/mission-sts-73.html | - +----------------+--------+----------------------------------------------+ + os> source=weblogs | where cidrmatch(host, "199.120.110.0/24") | fields host + fetched rows / total rows = 1/3 + +----------------+ + | host | + |----------------+ + | 199.120.110.21 | + +----------------+ diff --git a/docs/user/ppl/index.rst b/docs/user/ppl/index.rst index 8106e044dd..9525874c59 100644 --- a/docs/user/ppl/index.rst +++ b/docs/user/ppl/index.rst @@ -102,7 +102,7 @@ The query start with search command and then flowing a set of command delimited - `System Functions `_ - - `IP Functions `_ + - `IP Address Functions `_ * **Optimization** From 9ba705aad612b4445fe0738ec904b74fc7d5618e Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 30 Oct 2024 10:22:43 -0700 Subject: [PATCH 17/40] Refactor implementation to use `com.github.seancfoley:ipaddress:5.4.2` (already a dependency for :core) and to match behaviour in Spark. Signed-off-by: currantw --- core/build.gradle | 1 + .../sql/expression/ip/IPFunction.java | 98 +++++++++---------- .../sql/expression/ip/IPFunctionTest.java | 68 ++++++++++--- 3 files changed, 98 insertions(+), 69 deletions(-) diff --git a/core/build.gradle b/core/build.gradle index f36777030c..c596251342 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -57,6 +57,7 @@ dependencies { api group: 'com.google.code.gson', name: 'gson', version: '2.8.9' api group: 'com.tdunning', name: 't-digest', version: '3.3' api project(':common') + implementation "com.github.seancfoley:ipaddress:5.4.2" testImplementation('org.junit.jupiter:junit-jupiter:5.9.3') testImplementation group: 'org.hamcrest', name: 'hamcrest-library', version: '2.1' diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index c7f81bf564..1c3a7a8123 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -5,15 +5,9 @@ package org.opensearch.sql.expression.ip; -import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; -import static org.opensearch.sql.expression.function.FunctionDSL.*; - -import com.google.common.net.InetAddresses; -import java.math.BigInteger; -import java.net.InetAddress; -import java.util.regex.Matcher; -import java.util.regex.Pattern; +import inet.ipaddr.AddressStringException; +import inet.ipaddr.IPAddressString; +import inet.ipaddr.IPAddressStringParameters; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; @@ -22,13 +16,14 @@ import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.DefaultFunctionResolver; +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; +import static org.opensearch.sql.expression.function.FunctionDSL.*; + /** Utility class that defines and registers IP functions. */ @UtilityClass public class IPFunction { - private static final Pattern cidrPattern = - Pattern.compile("(?
.+)[/](?[0-9]+)"); - public void register(BuiltinFunctionRepository repository) { repository.register(cidrmatch()); } @@ -40,8 +35,8 @@ private DefaultFunctionResolver cidrmatch() { } /** - * Returns whether the given IP address is within the specified CIDR IP address range. - * Supports both IPv4 and IPv6 addresses. + * Returns whether the given IP address is within the specified CIDR IP address range. Supports + * both IPv4 and IPv6 addresses. * * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or @@ -51,56 +46,51 @@ private DefaultFunctionResolver cidrmatch() { */ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprValue) { - // Get address String addressString = addressExprValue.stringValue(); - if (!InetAddresses.isInetAddress(addressString)) { - return ExprValueUtils.nullValue(); - } - - InetAddress address = InetAddresses.forString(addressString); - - // Get range and network length String rangeString = rangeExprValue.stringValue(); - Matcher cidrMatcher = cidrPattern.matcher(rangeString); - if (!cidrMatcher.matches()) - throw new SemanticCheckException( - String.format("CIDR notation '%s' in not valid", rangeString)); - - String rangeAddressString = cidrMatcher.group("address"); - if (!InetAddresses.isInetAddress(rangeAddressString)) + 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) { throw new SemanticCheckException( - String.format("IP address '%s' in not valid", rangeAddressString)); - - InetAddress rangeAddress = InetAddresses.forString(rangeAddressString); - - // Address and range must use the same IP version (IPv4 or IPv6). - if (!address.getClass().equals(rangeAddress.getClass())) { - return ExprValueUtils.booleanValue(false); + String.format( + "IP address '%s' is not supported. Error details: %s", + addressString, e.getMessage())); } - int networkLengthBits = Integer.parseInt(cidrMatcher.group("networkLength")); - int addressLengthBits = address.getAddress().length * Byte.SIZE; + // Get and validate CIDR IP address range. + IPAddressString range = new IPAddressString(rangeExprValue.stringValue(), validationOptions); - if (networkLengthBits > addressLengthBits) + try { + range.validate(); + } catch (AddressStringException e) { throw new SemanticCheckException( - String.format("Network length of '%s' bits is not valid", networkLengthBits)); - - // Build bounds by converting the address to an integer, setting all the non-significant bits to - // zero for the lower bounds and one for the upper bounds, and then converting back to - // addresses. - BigInteger lowerBoundInt = InetAddresses.toBigInteger(rangeAddress); - BigInteger upperBoundInt = InetAddresses.toBigInteger(rangeAddress); + String.format( + "CIDR IP address range '%s' is not supported. Error details: %s", + rangeString, e.getMessage())); + } - int hostLengthBits = addressLengthBits - networkLengthBits; - for (int bit = 0; bit < hostLengthBits; bit++) { - lowerBoundInt = lowerBoundInt.clearBit(bit); - upperBoundInt = upperBoundInt.setBit(bit); + // Address and range must use the same IP version (IPv4 or IPv6). + if (address.isIPv4() ^ range.isIPv4()) { + throw new SemanticCheckException( + String.format( + "IP address '%s' and CIDR IP address range '%s' are not compatible. Both must be" + + " either IPv4 or IPv6.", + addressString, rangeString)); } - // Convert the address to an integer and compare it to the bounds. - BigInteger addressInt = InetAddresses.toBigInteger(address); - return ExprValueUtils.booleanValue( - (addressInt.compareTo(lowerBoundInt) >= 0) && (addressInt.compareTo(upperBoundInt) <= 0)); + return ExprValueUtils.booleanValue(range.contains(address)); } } 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 index 96ca53ee5a..ad31402cd7 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -5,10 +5,10 @@ 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.*; import static org.mockito.Mockito.when; -import static org.opensearch.sql.data.model.ExprValueUtils.*; +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; @@ -46,20 +46,45 @@ public class IPFunctionTest { @Test public void cidrmatch_invalid_address() { - assertEquals(LITERAL_NULL, execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); + SemanticCheckException exception = + assertThrows( + SemanticCheckException.class, + () -> execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); + assertTrue( + exception.getMessage().matches("IP address 'INVALID' is not supported. Error details: .*")); } @Test public void cidrmatch_invalid_range() { - assertThrows( - SemanticCheckException.class, - () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); - assertThrows( - SemanticCheckException.class, - () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID/32"))); - assertThrows( - SemanticCheckException.class, - () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("198.51.100.0/33"))); + SemanticCheckException exception; + + exception = + assertThrows( + SemanticCheckException.class, + () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); + assertTrue( + exception + .getMessage() + .matches("CIDR IP address range 'INVALID' is not supported. Error details: .*")); + + exception = + assertThrows( + SemanticCheckException.class, + () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID/32"))); + assertTrue( + exception + .getMessage() + .matches("CIDR IP address range 'INVALID/32' is not supported. Error details: .*")); + + exception = + assertThrows( + SemanticCheckException.class, + () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("198.51.100.0/33"))); + assertTrue( + exception + .getMessage() + .matches( + "CIDR IP address range '198.51.100.0/33' is not supported. Error details: .*")); } @Test @@ -78,8 +103,21 @@ public void cidrmatch_valid_ipv6() { @Test public void cidrmatch_valid_different_versions() { - assertEquals(LITERAL_FALSE, execute(IPv4AddressWithin, IPv6Range)); - assertEquals(LITERAL_FALSE, execute(IPv6AddressWithin, IPv4Range)); + 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()); } /** From c95152edc82f031df2bc699d36967435d3d42669 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 30 Oct 2024 20:56:29 -0700 Subject: [PATCH 18/40] Update OpenSearchIpType to covert to string type. Signed-off-by: currantw --- .../org/opensearch/sql/ppl/IPFunctionIT.java | 20 ++++---- .../data/type/OpenSearchDataType.java | 2 +- .../data/type/OpenSearchIpType.java | 4 +- .../data/value/OpenSearchExprIpValue.java | 5 ++ .../value/OpenSearchExprValueFactory.java | 46 +++---------------- 5 files changed, 24 insertions(+), 53 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java index b5a0cdd716..d2542ffe9d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -28,27 +28,27 @@ public void test_cidrmatch() throws IOException { result = executeQuery( String.format( - "source=%s | where cidr(host, '199.120.111.0/24') | fields url", + "source=%s | where cidrmatch(host, '199.120.111.0/24') | fields host", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("url", null, "boolean")); - verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); + verifySchema(result, schema("host", null, "string")); + verifyDataRows(result); // One match result = executeQuery( String.format( - "source=%s | where cidr(host, '199.120.110.0/24') | fields url", + "source=%s | where cidrmatch(host, '199.120.110.0/24') | fields host", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("url", null, "boolean")); - verifyDataRows(result, rows("/shuttle/missions/sts-73/mission-sts-73.html")); + verifySchema(result, schema("host", null, "string")); + verifyDataRows(result, rows("199.120.110.21")); // Multiple matches result = executeQuery( String.format( - "source=%s | where cidr(host, '199.0.0.0/8') | fields url", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("url", null, "boolean")); - verifyDataRows( - result, rows("/history/apollo/"), rows("/shuttle/missions/sts-73/mission-sts-73.html")); + "source=%s | where cidrmatch(host, '199.0.0.0/8') | fields host", + TEST_INDEX_WEBLOG)); + verifySchema(result, schema("host", null, "string")); + verifyDataRows(result, rows("199.72.81.55"), rows("199.120.110.21")); } } 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..e86e604fa1 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.STRING), GeoPoint("geo_point", ExprCoreType.UNKNOWN), Binary("binary", ExprCoreType.UNKNOWN), Date("date", ExprCoreType.TIMESTAMP), 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 index 22581ec28c..9f3a521ca1 100644 --- 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 @@ -5,7 +5,7 @@ package org.opensearch.sql.opensearch.data.type; -import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; import lombok.EqualsAndHashCode; @@ -20,7 +20,7 @@ public class OpenSearchIpType extends OpenSearchDataType { private OpenSearchIpType() { super(MappingType.Ip); - exprCoreType = UNKNOWN; + exprCoreType = STRING; } public static OpenSearchIpType of() { 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 index 30b3784bfc..3ecc39457c 100644 --- 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 @@ -45,4 +45,9 @@ public boolean equal(ExprValue other) { public int hashCode() { return Objects.hashCode(ip); } + + @Override + public String stringValue() { + return 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 d9e21436b7..db705a1181 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 @@ -5,37 +5,17 @@ package org.opensearch.sql.opensearch.data.value; -import static org.opensearch.sql.data.type.ExprCoreType.ARRAY; -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.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.LONG; -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; -import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; -import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER; -import static org.opensearch.sql.utils.DateTimeFormatters.STRICT_HOUR_MINUTE_SECOND_FORMATTER; -import static org.opensearch.sql.utils.DateTimeFormatters.STRICT_YEAR_MONTH_DAY_FORMATTER; +import static org.opensearch.sql.data.type.ExprCoreType.*; +import static org.opensearch.sql.utils.DateTimeFormatters.*; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.ImmutableMap; -import java.time.Instant; -import java.time.LocalDate; -import java.time.LocalTime; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; +import java.time.*; import java.time.format.DateTimeParseException; import java.time.temporal.TemporalAccessor; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import java.util.function.BiFunction; import lombok.Getter; import lombok.Setter; @@ -43,21 +23,7 @@ import org.opensearch.common.time.DateFormatter; import org.opensearch.common.time.DateFormatters; import org.opensearch.common.time.FormatNames; -import org.opensearch.sql.data.model.ExprBooleanValue; -import org.opensearch.sql.data.model.ExprByteValue; -import org.opensearch.sql.data.model.ExprCollectionValue; -import org.opensearch.sql.data.model.ExprDateValue; -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.ExprLongValue; -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.ExprTupleValue; -import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.*; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.opensearch.data.type.OpenSearchBinaryType; @@ -136,7 +102,7 @@ public void extendTypeMapping(Map typeMapping) { OpenSearchExprValueFactory::createOpenSearchDateType) .put( OpenSearchDataType.of(OpenSearchDataType.MappingType.Ip), - (c, dt) -> new OpenSearchExprIpValue(c.stringValue())) + (c, dt) -> new ExprStringValue(c.stringValue())) .put( OpenSearchDataType.of(OpenSearchDataType.MappingType.Binary), (c, dt) -> new OpenSearchExprBinaryValue(c.stringValue())) From 9c7f5b6398c4c16e5a53fa295873aca84d84c4f8 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 30 Oct 2024 20:57:35 -0700 Subject: [PATCH 19/40] Run Spotless. Signed-off-by: currantw --- .../java/org/opensearch/sql/expression/ip/IPFunction.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index 1c3a7a8123..f29c92789c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -5,6 +5,10 @@ package org.opensearch.sql.expression.ip; +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; +import static org.opensearch.sql.expression.function.FunctionDSL.*; + import inet.ipaddr.AddressStringException; import inet.ipaddr.IPAddressString; import inet.ipaddr.IPAddressStringParameters; @@ -16,10 +20,6 @@ import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.DefaultFunctionResolver; -import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; -import static org.opensearch.sql.expression.function.FunctionDSL.*; - /** Utility class that defines and registers IP functions. */ @UtilityClass public class IPFunction { From b7fb7f7989444c9960450ad68e7ff77275bd76d7 Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 30 Oct 2024 21:20:07 -0700 Subject: [PATCH 20/40] Minor cleanup. Signed-off-by: currantw --- .../sql/expression/ip/IPFunction.java | 10 ++-- .../sql/expression/ip/IPFunctionTest.java | 57 ++++++------------- 2 files changed, 23 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index f29c92789c..82e7778f71 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -41,8 +41,9 @@ private DefaultFunctionResolver cidrmatch() { * @param addressExprValue IP address (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or * "2001:0db8::/32") - * @return null if the address is not valid; true if the address is in the range; otherwise false. - * @throws SemanticCheckException if the range is not valid + * @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 + * same version (IPv4 or IPv6). */ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprValue) { @@ -66,8 +67,7 @@ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprV } catch (AddressStringException e) { throw new SemanticCheckException( String.format( - "IP address '%s' is not supported. Error details: %s", - addressString, e.getMessage())); + "IP address '%s' is not valid. Error details: %s", addressString, e.getMessage())); } // Get and validate CIDR IP address range. @@ -78,7 +78,7 @@ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprV } catch (AddressStringException e) { throw new SemanticCheckException( String.format( - "CIDR IP address range '%s' is not supported. Error details: %s", + "CIDR IP address range '%s' is not valid. Error details: %s", rangeString, e.getMessage())); } 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 index ad31402cd7..42959a7f90 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -51,58 +51,23 @@ public void cidrmatch_invalid_address() { SemanticCheckException.class, () -> execute(ExprValueUtils.stringValue("INVALID"), IPv4Range)); assertTrue( - exception.getMessage().matches("IP address 'INVALID' is not supported. Error details: .*")); + exception.getMessage().matches("IP address 'INVALID' is not valid. Error details: .*")); } @Test public void cidrmatch_invalid_range() { - SemanticCheckException exception; - - exception = + SemanticCheckException exception = assertThrows( SemanticCheckException.class, () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID"))); assertTrue( exception .getMessage() - .matches("CIDR IP address range 'INVALID' is not supported. Error details: .*")); - - exception = - assertThrows( - SemanticCheckException.class, - () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID/32"))); - assertTrue( - exception - .getMessage() - .matches("CIDR IP address range 'INVALID/32' is not supported. Error details: .*")); - - exception = - assertThrows( - SemanticCheckException.class, - () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("198.51.100.0/33"))); - assertTrue( - exception - .getMessage() - .matches( - "CIDR IP address range '198.51.100.0/33' is not supported. Error details: .*")); - } - - @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)); + .matches("CIDR IP address range 'INVALID' is not valid. Error details: .*")); } @Test - public void cidrmatch_valid_different_versions() { + public void cidrmatch_different_versions() { SemanticCheckException exception; exception = @@ -120,6 +85,20 @@ public void cidrmatch_valid_different_versions() { 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. From 4b60ae181e1b5dd5ef9b1483eb734c2056ff41e7 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 31 Oct 2024 10:35:14 -0700 Subject: [PATCH 21/40] Update typos to fix failing unit tests, plus run Spotless. Signed-off-by: currantw --- .../opensearch/sql/analysis/AnalyzerTest.java | 84 +++---------------- .../datetime/DateAddAndAddDateTest.java | 10 +-- .../datetime/DateSubAndSubDateTest.java | 10 +-- .../function/DefaultFunctionResolverTest.java | 2 +- 4 files changed, 15 insertions(+), 91 deletions(-) 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 8d935b11d2..2888116848 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -6,86 +6,31 @@ package org.opensearch.sql.analysis; import static java.util.Collections.emptyList; -import static org.junit.jupiter.api.Assertions.assertAll; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; import static org.opensearch.sql.analysis.NestedAnalyzer.isNestedFunction; -import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; -import static org.opensearch.sql.ast.dsl.AstDSL.alias; -import static org.opensearch.sql.ast.dsl.AstDSL.argument; -import static org.opensearch.sql.ast.dsl.AstDSL.booleanLiteral; -import static org.opensearch.sql.ast.dsl.AstDSL.compare; -import static org.opensearch.sql.ast.dsl.AstDSL.field; -import static org.opensearch.sql.ast.dsl.AstDSL.filter; -import static org.opensearch.sql.ast.dsl.AstDSL.filteredAggregate; -import static org.opensearch.sql.ast.dsl.AstDSL.function; -import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral; -import static org.opensearch.sql.ast.dsl.AstDSL.nestedAllTupleFields; -import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; -import static org.opensearch.sql.ast.dsl.AstDSL.relation; -import static org.opensearch.sql.ast.dsl.AstDSL.span; -import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; -import static org.opensearch.sql.ast.dsl.AstDSL.unresolvedArg; -import static org.opensearch.sql.ast.tree.Sort.NullOrder; -import static org.opensearch.sql.ast.tree.Sort.SortOption; +import static org.opensearch.sql.ast.dsl.AstDSL.*; +import static org.opensearch.sql.ast.tree.Sort.*; import static org.opensearch.sql.ast.tree.Sort.SortOption.DEFAULT_ASC; -import static org.opensearch.sql.ast.tree.Sort.SortOrder; import static org.opensearch.sql.data.model.ExprValueUtils.integerValue; import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; -import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; -import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; -import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; -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.TIMESTAMP; +import static org.opensearch.sql.data.type.ExprCoreType.*; import static org.opensearch.sql.expression.DSL.literal; -import static org.opensearch.sql.utils.MLCommonsConstants.ACTION; -import static org.opensearch.sql.utils.MLCommonsConstants.ALGO; -import static org.opensearch.sql.utils.MLCommonsConstants.ASYNC; -import static org.opensearch.sql.utils.MLCommonsConstants.CLUSTERID; -import static org.opensearch.sql.utils.MLCommonsConstants.KMEANS; -import static org.opensearch.sql.utils.MLCommonsConstants.MODELID; -import static org.opensearch.sql.utils.MLCommonsConstants.PREDICT; -import static org.opensearch.sql.utils.MLCommonsConstants.RCF; -import static org.opensearch.sql.utils.MLCommonsConstants.RCF_ANOMALOUS; -import static org.opensearch.sql.utils.MLCommonsConstants.RCF_ANOMALY_GRADE; -import static org.opensearch.sql.utils.MLCommonsConstants.RCF_SCORE; -import static org.opensearch.sql.utils.MLCommonsConstants.RCF_TIME_FIELD; -import static org.opensearch.sql.utils.MLCommonsConstants.STATUS; -import static org.opensearch.sql.utils.MLCommonsConstants.TASKID; -import static org.opensearch.sql.utils.MLCommonsConstants.TRAIN; +import static org.opensearch.sql.utils.MLCommonsConstants.*; import static org.opensearch.sql.utils.SystemIndexUtils.DATASOURCES_TABLE_NAME; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; +import java.util.*; import java.util.Map; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.opensearch.sql.ast.dsl.AstDSL; -import org.opensearch.sql.ast.expression.Argument; -import org.opensearch.sql.ast.expression.DataType; -import org.opensearch.sql.ast.expression.HighlightFunction; -import org.opensearch.sql.ast.expression.Literal; -import org.opensearch.sql.ast.expression.ParseMethod; -import org.opensearch.sql.ast.expression.ScoreFunction; -import org.opensearch.sql.ast.expression.SpanUnit; -import org.opensearch.sql.ast.tree.AD; -import org.opensearch.sql.ast.tree.CloseCursor; -import org.opensearch.sql.ast.tree.FetchCursor; -import org.opensearch.sql.ast.tree.Kmeans; -import org.opensearch.sql.ast.tree.ML; -import org.opensearch.sql.ast.tree.Paginate; +import org.opensearch.sql.ast.expression.*; +import org.opensearch.sql.ast.tree.*; import org.opensearch.sql.ast.tree.RareTopN.CommandType; -import org.opensearch.sql.ast.tree.UnresolvedPlan; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; @@ -95,16 +40,7 @@ import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.function.OpenSearchFunctions; import org.opensearch.sql.expression.window.WindowDefinition; -import org.opensearch.sql.planner.logical.LogicalAD; -import org.opensearch.sql.planner.logical.LogicalCloseCursor; -import org.opensearch.sql.planner.logical.LogicalFetchCursor; -import org.opensearch.sql.planner.logical.LogicalFilter; -import org.opensearch.sql.planner.logical.LogicalMLCommons; -import org.opensearch.sql.planner.logical.LogicalPaginate; -import org.opensearch.sql.planner.logical.LogicalPlan; -import org.opensearch.sql.planner.logical.LogicalPlanDSL; -import org.opensearch.sql.planner.logical.LogicalProject; -import org.opensearch.sql.planner.logical.LogicalRelation; +import org.opensearch.sql.planner.logical.*; import org.opensearch.sql.planner.physical.datasource.DataSourceTable; class AnalyzerTest extends AnalyzerTestBase { @@ -158,7 +94,7 @@ public void filter_relation_with_invalid_qualifiedName_ExpressionEvaluationExcep "= 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]," - + "[STRUCT,STRUCT],[ARRAY,ARRAY]}, but get [STRING,INTEGER]", + + "[STRUCT,STRUCT],[ARRAY,ARRAY]}, but got [STRING,INTEGER]", exception.getMessage()); } diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java index 519e97bdc6..c5dfdefe00 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java @@ -10,13 +10,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; -import java.time.Duration; -import java.time.Instant; -import java.time.LocalDate; -import java.time.LocalDateTime; -import java.time.LocalTime; -import java.time.Period; -import java.time.ZoneOffset; +import java.time.*; import org.junit.jupiter.api.Test; import org.opensearch.sql.exception.ExpressionEvaluationException; @@ -156,7 +150,7 @@ public void adddate_has_second_signature_but_not_date_add() { () -> date_add(LocalDateTime.of(1961, 4, 12, 9, 7), 100500)); assertEquals( "date_add function expected {[DATE,INTERVAL],[TIMESTAMP,INTERVAL]," - + "[TIME,INTERVAL]}, but get [TIMESTAMP,INTEGER]", + + "[TIME,INTERVAL]}, but got [TIMESTAMP,INTEGER]", exception.getMessage()); } diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java index 123ecda0bd..b40e180816 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java @@ -10,13 +10,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; -import java.time.Duration; -import java.time.Instant; -import java.time.LocalDate; -import java.time.LocalDateTime; -import java.time.LocalTime; -import java.time.Period; -import java.time.ZoneOffset; +import java.time.*; import org.junit.jupiter.api.Test; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.exception.ExpressionEvaluationException; @@ -139,7 +133,7 @@ public void subdate_has_second_signature_but_not_date_sub() { ExpressionEvaluationException.class, () -> date_sub(LocalDateTime.of(1961, 4, 12, 9, 7), 100500)); assertEquals( - "date_sub function expected {[DATE,INTERVAL],[TIMESTAMP,INTERVAL],[TIME,INTERVAL]}, but get" + "date_sub function expected {[DATE,INTERVAL],[TIMESTAMP,INTERVAL],[TIME,INTERVAL]}, but got" + " [TIMESTAMP,INTEGER]", exception.getMessage()); } diff --git a/core/src/test/java/org/opensearch/sql/expression/function/DefaultFunctionResolverTest.java b/core/src/test/java/org/opensearch/sql/expression/function/DefaultFunctionResolverTest.java index ad9e8a6661..0c0439a764 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/DefaultFunctionResolverTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/DefaultFunctionResolverTest.java @@ -72,7 +72,7 @@ void resolve_function_not_match() { assertThrows( ExpressionEvaluationException.class, () -> resolver.resolve(functionSignature)); assertEquals( - "add function expected {[INTEGER,INTEGER]}, but get [BOOLEAN,BOOLEAN]", + "add function expected {[INTEGER,INTEGER]}, but got [BOOLEAN,BOOLEAN]", exception.getMessage()); } From 08af013f45ec466a3de4edb9d9d3215108d2d7db Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 31 Oct 2024 12:04:56 -0700 Subject: [PATCH 22/40] Fix remaining failing unit tests. Signed-off-by: currantw --- .../PercentileApproxAggregatorTest.java | 5 +- .../value/OpenSearchExprValueFactory.java | 12 ++-- .../data/type/OpenSearchDataTypeTest.java | 26 +------- .../value/OpenSearchExprValueFactoryTest.java | 64 +++++++------------ 4 files changed, 33 insertions(+), 74 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java b/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java index ac617e7b32..c0b72813d2 100644 --- a/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java @@ -16,7 +16,8 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; -import static org.opensearch.sql.data.model.ExprValueUtils.*; +import static org.opensearch.sql.data.model.ExprValueUtils.integerValue; +import static org.opensearch.sql.data.model.ExprValueUtils.longValue; import static org.opensearch.sql.data.type.ExprCoreType.*; import java.util.ArrayList; @@ -195,7 +196,7 @@ public void test_percentile_with_invalid_size() { "percentile_approx function expected" + " {[INTEGER,DOUBLE],[INTEGER,DOUBLE,DOUBLE],[LONG,DOUBLE],[LONG,DOUBLE,DOUBLE]," + "[FLOAT,DOUBLE],[FLOAT,DOUBLE,DOUBLE],[DOUBLE,DOUBLE],[DOUBLE,DOUBLE,DOUBLE]}," - + " but get [DOUBLE,STRING]", + + " but got [DOUBLE,STRING]", exception2.getMessage()); } 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 db705a1181..0336c6ccac 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 @@ -170,14 +170,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())); } } 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..c46f4f141c 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 @@ -5,29 +5,9 @@ package org.opensearch.sql.opensearch.data.type; -import static org.junit.jupiter.api.Assertions.assertAll; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNotSame; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assumptions.assumeFalse; -import static org.opensearch.sql.data.type.ExprCoreType.ARRAY; -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.LONG; -import static org.opensearch.sql.data.type.ExprCoreType.SHORT; -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.TIMESTAMP; -import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; +import static org.opensearch.sql.data.type.ExprCoreType.*; import static org.opensearch.sql.opensearch.data.type.OpenSearchDataType.MappingType; import java.util.Map; @@ -110,7 +90,7 @@ private static Stream getTestDataWithType() { Arguments.of(MappingType.Nested, "nested", ARRAY), 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.Ip, "ip", STRING)); } @ParameterizedTest(name = "{1}") 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 5fd40ef6c4..534b8e5e8d 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 @@ -5,34 +5,9 @@ package org.opensearch.sql.opensearch.data.value; -import static org.junit.jupiter.api.Assertions.assertAll; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.opensearch.sql.data.model.ExprValueUtils.booleanValue; -import static org.opensearch.sql.data.model.ExprValueUtils.byteValue; -import static org.opensearch.sql.data.model.ExprValueUtils.collectionValue; -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.longValue; -import static org.opensearch.sql.data.model.ExprValueUtils.nullValue; -import static org.opensearch.sql.data.model.ExprValueUtils.shortValue; -import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; -import static org.opensearch.sql.data.type.ExprCoreType.ARRAY; -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.LONG; -import static org.opensearch.sql.data.type.ExprCoreType.SHORT; -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; -import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; +import static org.junit.jupiter.api.Assertions.*; +import static org.opensearch.sql.data.model.ExprValueUtils.*; +import static org.opensearch.sql.data.type.ExprCoreType.*; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -49,12 +24,7 @@ import org.junit.jupiter.api.Test; import org.opensearch.OpenSearchParseException; 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.ExprTimeValue; -import org.opensearch.sql.data.model.ExprTimestampValue; -import org.opensearch.sql.data.model.ExprTupleValue; -import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.*; import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.data.type.OpenSearchDateType; import org.opensearch.sql.opensearch.data.type.OpenSearchTextType; @@ -62,6 +32,8 @@ class OpenSearchExprValueFactoryTest { + static final String fieldIp = "ipV"; + private static final Map MAPPING = new ImmutableMap.Builder() .put("byteV", OpenSearchDataType.of(BYTE)) @@ -112,7 +84,7 @@ class OpenSearchExprValueFactoryTest { "textKeywordV", OpenSearchTextType.of( Map.of("words", OpenSearchDataType.of(OpenSearchDataType.MappingType.Keyword)))) - .put("ipV", OpenSearchDataType.of(OpenSearchDataType.MappingType.Ip)) + .put(fieldIp, OpenSearchDataType.of(OpenSearchDataType.MappingType.Ip)) .put("geoV", OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint)) .put("binaryV", OpenSearchDataType.of(OpenSearchDataType.MappingType.Binary)) .build(); @@ -660,12 +632,12 @@ public void constructArrayOfGeoPointsReturnsAll() { @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("192.168.0.1"), - new OpenSearchExprIpValue("192.168.0.2"))), - tupleValue("{\"ipV\":[\"192.168.0.1\",\"192.168.0.2\"]}").get("ipV")); + new ExprCollectionValue(List.of(new ExprStringValue(ip1), new ExprStringValue(ip2))), + tupleValue(String.format("{\"%s\":[\"%s\",\"%s\"]}", fieldIp, ip1, ip2)).get(fieldIp)); } @Test @@ -741,9 +713,17 @@ public void constructStruct() { @Test public void constructIP() { + final String valueIpv4 = "192.168.0.1"; + assertEquals( + stringValue(valueIpv4), + tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, valueIpv4)).get(fieldIp)); + assertEquals(stringValue(valueIpv4), constructFromObject(fieldIp, valueIpv4)); + + final String valueIpv6 = "2001:0db7::ff00:42:8329"; assertEquals( - new OpenSearchExprIpValue("192.168.0.1"), - tupleValue("{\"ipV\":\"192.168.0.1\"}").get("ipV")); + stringValue(valueIpv6), + tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, valueIpv6)).get(fieldIp)); + assertEquals(stringValue(valueIpv6), constructFromObject(fieldIp, valueIpv6)); } private static final double TOLERANCE = 1E-5; From 4b05c24116909cb63957b6ba9426daedbecaa4e7 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 7 Nov 2024 08:33:14 -0800 Subject: [PATCH 23/40] Address minor review comments. Signed-off-by: currantw --- .../sql/exception/QueryEngineException.java | 4 ++++ .../sql/exception/SemanticCheckException.java | 5 +++++ .../opensearch/sql/expression/ip/IPFunction.java | 15 +++++++++------ docs/category.json | 9 +++++---- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/exception/QueryEngineException.java b/core/src/main/java/org/opensearch/sql/exception/QueryEngineException.java index b3d13bef71..122d4963fa 100644 --- a/core/src/main/java/org/opensearch/sql/exception/QueryEngineException.java +++ b/core/src/main/java/org/opensearch/sql/exception/QueryEngineException.java @@ -11,4 +11,8 @@ public class QueryEngineException extends RuntimeException { public QueryEngineException(String message) { super(message); } + + public QueryEngineException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/core/src/main/java/org/opensearch/sql/exception/SemanticCheckException.java b/core/src/main/java/org/opensearch/sql/exception/SemanticCheckException.java index 6e0c184af8..c43dfdffc8 100644 --- a/core/src/main/java/org/opensearch/sql/exception/SemanticCheckException.java +++ b/core/src/main/java/org/opensearch/sql/exception/SemanticCheckException.java @@ -7,7 +7,12 @@ /** Semantic Check Exception. */ public class SemanticCheckException extends QueryEngineException { + public SemanticCheckException(String message) { super(message); } + + public SemanticCheckException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index 82e7778f71..8dde5e6108 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -65,9 +65,10 @@ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprV try { address.validate(); } catch (AddressStringException e) { - throw new SemanticCheckException( + String msg = String.format( - "IP address '%s' is not valid. Error details: %s", addressString, e.getMessage())); + "IP address '%s' is not valid. Error details: %s", addressString, e.getMessage()); + throw new SemanticCheckException(msg, e); } // Get and validate CIDR IP address range. @@ -76,19 +77,21 @@ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprV try { range.validate(); } catch (AddressStringException e) { - throw new SemanticCheckException( + String msg = String.format( "CIDR IP address range '%s' is not valid. Error details: %s", - rangeString, e.getMessage())); + 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()) { - throw new SemanticCheckException( + 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)); + addressString, rangeString); + throw new SemanticCheckException(msg); } return ExprValueUtils.booleanValue(range.contains(address)); diff --git a/docs/category.json b/docs/category.json index e90c674a2e..ca3d345e8b 100644 --- a/docs/category.json +++ b/docs/category.json @@ -28,12 +28,13 @@ "user/ppl/cmd/where.rst", "user/ppl/general/identifiers.rst", "user/ppl/general/datatypes.rst", - "user/ppl/functions/math.rst", - "user/ppl/functions/datetime.rst", - "user/ppl/functions/string.rst", "user/ppl/functions/condition.rst", + "user/ppl/functions/datetime.rst", + "user/ppl/functions/expressions.rst", + "user/ppl/functions/ip.rst", + "user/ppl/functions/math.rst", "user/ppl/functions/relevance.rst", - "user/ppl/functions/expressions.rst" + "user/ppl/functions/string.rst" ], "sql_cli": [ "user/dql/expressions.rst", From 17decc8f648fbaf994cdc7702f880eede5f5f0f6 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 7 Nov 2024 08:38:59 -0800 Subject: [PATCH 24/40] Apply Spotless Signed-off-by: currantw --- core/src/main/java/org/opensearch/sql/expression/DSL.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 3e694e0b7a..54bd35e70f 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -5,6 +5,7 @@ package org.opensearch.sql.expression; +import java.util.Arrays; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.data.model.ExprShortValue; import org.opensearch.sql.data.model.ExprValue; @@ -25,8 +26,6 @@ import org.opensearch.sql.expression.span.SpanExpression; import org.opensearch.sql.expression.window.ranking.RankingWindowFunction; -import java.util.Arrays; - public class DSL { private DSL() {} From e8b6f4ec261306add9e286b92e395504fb538b87 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 7 Nov 2024 08:42:17 -0800 Subject: [PATCH 25/40] Cleanup: remove a few redundant local variables. Signed-off-by: currantw --- .../operator/predicate/UnaryPredicateOperator.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java index 8889387d1f..6b190bbcd0 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java @@ -105,8 +105,7 @@ private static DefaultFunctionResolver ifFunction() { .map(v -> impl((UnaryPredicateOperator::exprIf), v, BOOLEAN, v, v)) .collect(Collectors.toList()); - DefaultFunctionResolver functionResolver = FunctionDSL.define(functionName, functionsOne); - return functionResolver; + return FunctionDSL.define(functionName, functionsOne); } private static DefaultFunctionResolver ifNull() { @@ -122,21 +121,18 @@ private static DefaultFunctionResolver ifNull() { .map(v -> impl((UnaryPredicateOperator::exprIfNull), v, v, v)) .collect(Collectors.toList()); - DefaultFunctionResolver functionResolver = FunctionDSL.define(functionName, functionsOne); - return functionResolver; + return FunctionDSL.define(functionName, functionsOne); } private static DefaultFunctionResolver nullIf() { FunctionName functionName = BuiltinFunctionName.NULLIF.getName(); List typeList = ExprCoreType.coreTypes(); - DefaultFunctionResolver functionResolver = - FunctionDSL.define( + return FunctionDSL.define( functionName, typeList.stream() .map(v -> impl((UnaryPredicateOperator::exprNullIf), v, v, v)) .collect(Collectors.toList())); - return functionResolver; } /** From ac02e67eb8fb14cb91bfd156bbee850d3b66d2a5 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 7 Nov 2024 18:38:38 -0800 Subject: [PATCH 26/40] More Spotless Signed-off-by: currantw --- .../predicate/UnaryPredicateOperator.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java index 6b190bbcd0..7d54dd8339 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java @@ -5,21 +5,20 @@ package org.opensearch.sql.expression.operator.predicate; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.expression.function.FunctionDSL.impl; + +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprBooleanValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.expression.function.*; -import java.util.Arrays; -import java.util.List; -import java.util.stream.Collectors; - -import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; -import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; -import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; -import static org.opensearch.sql.expression.function.FunctionDSL.impl; - /** * The definition of unary predicate function not, Accepts one Boolean value and produces a Boolean. */ @@ -129,10 +128,10 @@ private static DefaultFunctionResolver nullIf() { List typeList = ExprCoreType.coreTypes(); return FunctionDSL.define( - functionName, - typeList.stream() - .map(v -> impl((UnaryPredicateOperator::exprNullIf), v, v, v)) - .collect(Collectors.toList())); + functionName, + typeList.stream() + .map(v -> impl((UnaryPredicateOperator::exprNullIf), v, v, v)) + .collect(Collectors.toList())); } /** From 8c17404d3264a5881cfa299706982e5b1d2c41bb Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 7 Nov 2024 22:15:46 -0800 Subject: [PATCH 27/40] Add missing unit test Signed-off-by: currantw --- .../data/value/OpenSearchExprIpValueTest.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) 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 38a4ad3199..8d23d4bd90 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 @@ -5,40 +5,44 @@ 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 static org.junit.jupiter.api.Assertions.*; import org.junit.jupiter.api.Test; import org.opensearch.sql.opensearch.data.type.OpenSearchIpType; public class OpenSearchExprIpValueTest { - private OpenSearchExprIpValue ipValue = new OpenSearchExprIpValue("192.168.0.1"); + private final String ipString = "192.168.0.1"; + private final OpenSearchExprIpValue ipValue = new OpenSearchExprIpValue(ipString); @Test - void value() { - assertEquals("192.168.0.1", ipValue.value()); + void testValue() { + assertEquals(ipString, ipValue.value()); } @Test - void type() { + void testType() { assertEquals(OpenSearchIpType.of(), ipValue.type()); } @Test - void compare() { - assertEquals(0, ipValue.compareTo(new OpenSearchExprIpValue("192.168.0.1"))); - assertEquals(ipValue, new OpenSearchExprIpValue("192.168.0.1")); + void testCompare() { + assertEquals(0, ipValue.compareTo(new OpenSearchExprIpValue(ipString))); + assertEquals(ipValue, new OpenSearchExprIpValue(ipString)); } @Test - void equal() { - assertTrue(ipValue.equal(new OpenSearchExprIpValue("192.168.0.1"))); + void testEqual() { + assertTrue(ipValue.equal(new OpenSearchExprIpValue(ipString))); } @Test void testHashCode() { assertNotNull(ipValue.hashCode()); } + + @Test + void testStringValue() { + assertEquals(ipString, ipValue.stringValue()); + } } From 20c9672de36e04a408cf8eacf59f69c733a5d90a Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 12 Nov 2024 08:38:50 -0800 Subject: [PATCH 28/40] Get rid of wildcard imports. Signed-off-by: currantw --- .../sql/expression/ip/IPFunction.java | 4 +- .../opensearch/sql/analysis/AnalyzerTest.java | 82 +++++- .../datetime/DateSubAndSubDateTest.java | 8 +- .../sql/expression/ip/IPFunctionTest.java | 4 +- .../org/opensearch/sql/ppl/IPFunctionIT.java | 5 +- .../value/OpenSearchExprValueFactory.java | 238 ++++++++++-------- 6 files changed, 225 insertions(+), 116 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index 8dde5e6108..48738de54c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -7,7 +7,9 @@ import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.opensearch.sql.data.type.ExprCoreType.STRING; -import static org.opensearch.sql.expression.function.FunctionDSL.*; +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; 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 2888116848..2412bd9474 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -6,31 +6,86 @@ package org.opensearch.sql.analysis; import static java.util.Collections.emptyList; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opensearch.sql.analysis.DataSourceSchemaIdentifierNameResolver.DEFAULT_DATASOURCE_NAME; import static org.opensearch.sql.analysis.NestedAnalyzer.isNestedFunction; -import static org.opensearch.sql.ast.dsl.AstDSL.*; -import static org.opensearch.sql.ast.tree.Sort.*; +import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; +import static org.opensearch.sql.ast.dsl.AstDSL.alias; +import static org.opensearch.sql.ast.dsl.AstDSL.argument; +import static org.opensearch.sql.ast.dsl.AstDSL.booleanLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.compare; +import static org.opensearch.sql.ast.dsl.AstDSL.field; +import static org.opensearch.sql.ast.dsl.AstDSL.filter; +import static org.opensearch.sql.ast.dsl.AstDSL.filteredAggregate; +import static org.opensearch.sql.ast.dsl.AstDSL.function; +import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.nestedAllTupleFields; +import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; +import static org.opensearch.sql.ast.dsl.AstDSL.relation; +import static org.opensearch.sql.ast.dsl.AstDSL.span; +import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.unresolvedArg; +import static org.opensearch.sql.ast.tree.Sort.NullOrder; +import static org.opensearch.sql.ast.tree.Sort.SortOption; import static org.opensearch.sql.ast.tree.Sort.SortOption.DEFAULT_ASC; +import static org.opensearch.sql.ast.tree.Sort.SortOrder; import static org.opensearch.sql.data.model.ExprValueUtils.integerValue; import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; -import static org.opensearch.sql.data.type.ExprCoreType.*; +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; +import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; +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.TIMESTAMP; import static org.opensearch.sql.expression.DSL.literal; -import static org.opensearch.sql.utils.MLCommonsConstants.*; +import static org.opensearch.sql.utils.MLCommonsConstants.ACTION; +import static org.opensearch.sql.utils.MLCommonsConstants.ALGO; +import static org.opensearch.sql.utils.MLCommonsConstants.ASYNC; +import static org.opensearch.sql.utils.MLCommonsConstants.CLUSTERID; +import static org.opensearch.sql.utils.MLCommonsConstants.KMEANS; +import static org.opensearch.sql.utils.MLCommonsConstants.MODELID; +import static org.opensearch.sql.utils.MLCommonsConstants.PREDICT; +import static org.opensearch.sql.utils.MLCommonsConstants.RCF; +import static org.opensearch.sql.utils.MLCommonsConstants.RCF_ANOMALOUS; +import static org.opensearch.sql.utils.MLCommonsConstants.RCF_ANOMALY_GRADE; +import static org.opensearch.sql.utils.MLCommonsConstants.RCF_SCORE; +import static org.opensearch.sql.utils.MLCommonsConstants.RCF_TIME_FIELD; +import static org.opensearch.sql.utils.MLCommonsConstants.STATUS; +import static org.opensearch.sql.utils.MLCommonsConstants.TASKID; +import static org.opensearch.sql.utils.MLCommonsConstants.TRAIN; import static org.opensearch.sql.utils.SystemIndexUtils.DATASOURCES_TABLE_NAME; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.commons.lang3.tuple.ImmutablePair; import org.apache.commons.lang3.tuple.Pair; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.opensearch.sql.ast.dsl.AstDSL; -import org.opensearch.sql.ast.expression.*; -import org.opensearch.sql.ast.tree.*; +import org.opensearch.sql.ast.expression.Argument; +import org.opensearch.sql.ast.expression.DataType; +import org.opensearch.sql.ast.expression.HighlightFunction; +import org.opensearch.sql.ast.expression.Literal; +import org.opensearch.sql.ast.expression.ParseMethod; +import org.opensearch.sql.ast.expression.ScoreFunction; +import org.opensearch.sql.ast.expression.SpanUnit; +import org.opensearch.sql.ast.tree.AD; +import org.opensearch.sql.ast.tree.CloseCursor; +import org.opensearch.sql.ast.tree.FetchCursor; +import org.opensearch.sql.ast.tree.Kmeans; +import org.opensearch.sql.ast.tree.ML; +import org.opensearch.sql.ast.tree.Paginate; import org.opensearch.sql.ast.tree.RareTopN.CommandType; +import org.opensearch.sql.ast.tree.UnresolvedPlan; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; @@ -40,7 +95,16 @@ import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.function.OpenSearchFunctions; import org.opensearch.sql.expression.window.WindowDefinition; -import org.opensearch.sql.planner.logical.*; +import org.opensearch.sql.planner.logical.LogicalAD; +import org.opensearch.sql.planner.logical.LogicalCloseCursor; +import org.opensearch.sql.planner.logical.LogicalFetchCursor; +import org.opensearch.sql.planner.logical.LogicalFilter; +import org.opensearch.sql.planner.logical.LogicalMLCommons; +import org.opensearch.sql.planner.logical.LogicalPaginate; +import org.opensearch.sql.planner.logical.LogicalPlan; +import org.opensearch.sql.planner.logical.LogicalPlanDSL; +import org.opensearch.sql.planner.logical.LogicalProject; +import org.opensearch.sql.planner.logical.LogicalRelation; import org.opensearch.sql.planner.physical.datasource.DataSourceTable; class AnalyzerTest extends AnalyzerTestBase { diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java index b40e180816..897f49cfee 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateSubAndSubDateTest.java @@ -10,7 +10,13 @@ import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; -import java.time.*; +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Period; +import java.time.ZoneOffset; import org.junit.jupiter.api.Test; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.exception.ExpressionEvaluationException; 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 index 42959a7f90..ccee1783d1 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -5,7 +5,9 @@ package org.opensearch.sql.expression.ip; -import static org.junit.jupiter.api.Assertions.*; +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; diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java index d2542ffe9d..cd64b7359b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -6,7 +6,10 @@ package org.opensearch.sql.ppl; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WEBLOG; -import static org.opensearch.sql.util.MatcherUtils.*; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.schema; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; +import static org.opensearch.sql.util.MatcherUtils.verifySchema; import java.io.IOException; import org.json.JSONObject; 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 0336c6ccac..9908439886 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 @@ -5,17 +5,37 @@ package org.opensearch.sql.opensearch.data.value; -import static org.opensearch.sql.data.type.ExprCoreType.*; -import static org.opensearch.sql.utils.DateTimeFormatters.*; +import static org.opensearch.sql.data.type.ExprCoreType.ARRAY; +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.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.LONG; +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; +import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; +import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER; +import static org.opensearch.sql.utils.DateTimeFormatters.STRICT_HOUR_MINUTE_SECOND_FORMATTER; +import static org.opensearch.sql.utils.DateTimeFormatters.STRICT_YEAR_MONTH_DAY_FORMATTER; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.ImmutableMap; -import java.time.*; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalTime; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; import java.time.format.DateTimeParseException; import java.time.temporal.TemporalAccessor; -import java.util.*; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.function.BiFunction; import lombok.Getter; import lombok.Setter; @@ -23,7 +43,21 @@ import org.opensearch.common.time.DateFormatter; import org.opensearch.common.time.DateFormatters; import org.opensearch.common.time.FormatNames; -import org.opensearch.sql.data.model.*; +import org.opensearch.sql.data.model.ExprBooleanValue; +import org.opensearch.sql.data.model.ExprByteValue; +import org.opensearch.sql.data.model.ExprCollectionValue; +import org.opensearch.sql.data.model.ExprDateValue; +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.ExprLongValue; +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.ExprTupleValue; +import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.opensearch.data.type.OpenSearchBinaryType; @@ -37,34 +71,8 @@ /** Construct ExprValue from OpenSearch response. */ public class OpenSearchExprValueFactory { - /** The Mapping of Field and ExprType. */ - private final Map typeMapping; - - /** Whether to support nested value types (such as arrays) */ - private final boolean fieldTypeTolerance; - - /** - * Extend existing mapping by new data without overwrite. Called from aggregation only {@see - * AggregationQueryBuilder#buildTypeMapping}. - * - * @param typeMapping A data type mapping produced by aggregation. - */ - public void extendTypeMapping(Map typeMapping) { - for (var field : typeMapping.keySet()) { - // Prevent overwriting, because aggregation engine may be not aware - // of all niceties of all types. - if (!this.typeMapping.containsKey(field)) { - this.typeMapping.put(field, typeMapping.get(field)); - } - } - } - - @Getter @Setter private OpenSearchAggregationResponseParser parser; - private static final String TOP_PATH = ""; - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - private static final Map> typeActionMap = new ImmutableMap.Builder>() .put( @@ -108,6 +116,14 @@ public void extendTypeMapping(Map typeMapping) { (c, dt) -> new OpenSearchExprBinaryValue(c.stringValue())) .build(); + /** The Mapping of Field and ExprType. */ + private final Map typeMapping; + + /** Whether to support nested value types (such as arrays) */ + private final boolean fieldTypeTolerance; + + @Getter @Setter private OpenSearchAggregationResponseParser parser; + /** Constructor of OpenSearchExprValueFactory. */ public OpenSearchExprValueFactory( Map typeMapping, boolean fieldTypeTolerance) { @@ -115,78 +131,6 @@ public OpenSearchExprValueFactory( this.fieldTypeTolerance = fieldTypeTolerance; } - /** - * - * - * 
-   * The struct construction has the following assumption:
-   *  1. The field has OpenSearch Object data type.
-   *     See 
-   *       docs
-   *  2. The deeper field is flattened in the typeMapping. e.g.
-   *     { "employ",       "STRUCT"  }
-   *     { "employ.id",    "INTEGER" }
-   *     { "employ.state", "STRING"  }
-   *
- */ - public ExprValue construct(String jsonString, boolean supportArrays) { - try { - return parse( - new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)), - TOP_PATH, - Optional.of(STRUCT), - fieldTypeTolerance || supportArrays); - } catch (JsonProcessingException e) { - throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e); - } - } - - /** - * Construct ExprValue from field and its value object. Throw exception if trying to construct - * from field of unsupported type.
- * Todo, add IP, GeoPoint support after we have function implementation around it. - * - * @param field field name - * @param value value object - * @return ExprValue - */ - public ExprValue construct(String field, Object value, boolean supportArrays) { - return parse(new ObjectContent(value), field, type(field), supportArrays); - } - - private ExprValue parse( - Content content, String field, Optional fieldType, boolean supportArrays) { - if (content.isNull() || !fieldType.isPresent()) { - return ExprNullValue.of(); - } - - final ExprType type = fieldType.get(); - - if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { - return parseGeoPoint(content, supportArrays); - } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Nested)) - || content.isArray()) { - return parseArray(content, field, type, supportArrays); - } 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 { - throw new IllegalStateException( - String.format( - "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); - } - } - - /** - * In OpenSearch, it is possible field doesn't have type definition in mapping. but has empty - * value. For example, {"empty_field": []}. - */ - private Optional type(String field) { - return Optional.ofNullable(typeMapping.get(field)); - } - /** * Parse value with the first matching formatter into {@link ExprValue} with corresponding {@link * ExprCoreType}. @@ -274,6 +218,94 @@ private static ExprValue createOpenSearchDateType(Content value, ExprType type) return new ExprTimestampValue((Instant) value.objectValue()); } + /** + * Extend existing mapping by new data without overwrite. Called from aggregation only {@see + * AggregationQueryBuilder#buildTypeMapping}. + * + * @param typeMapping A data type mapping produced by aggregation. + */ + public void extendTypeMapping(Map typeMapping) { + for (var field : typeMapping.keySet()) { + // Prevent overwriting, because aggregation engine may be not aware + // of all niceties of all types. + if (!this.typeMapping.containsKey(field)) { + this.typeMapping.put(field, typeMapping.get(field)); + } + } + } + + /** + * + * + * 
+   * The struct construction has the following assumption:
+   *  1. The field has OpenSearch Object data type.
+   *     See 
+   *       docs
+   *  2. The deeper field is flattened in the typeMapping. e.g.
+   *     { "employ",       "STRUCT"  }
+   *     { "employ.id",    "INTEGER" }
+   *     { "employ.state", "STRING"  }
+   *
+ */ + public ExprValue construct(String jsonString, boolean supportArrays) { + try { + return parse( + new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)), + TOP_PATH, + Optional.of(STRUCT), + fieldTypeTolerance || supportArrays); + } catch (JsonProcessingException e) { + throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e); + } + } + + /** + * Construct ExprValue from field and its value object. Throw exception if trying to construct + * from field of unsupported type.
+ * Todo, add IP, GeoPoint support after we have function implementation around it. + * + * @param field field name + * @param value value object + * @return ExprValue + */ + public ExprValue construct(String field, Object value, boolean supportArrays) { + return parse(new ObjectContent(value), field, type(field), supportArrays); + } + + private ExprValue parse( + Content content, String field, Optional fieldType, boolean supportArrays) { + if (content.isNull() || !fieldType.isPresent()) { + return ExprNullValue.of(); + } + + final ExprType type = fieldType.get(); + + if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { + return parseGeoPoint(content, supportArrays); + } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Nested)) + || content.isArray()) { + return parseArray(content, field, type, supportArrays); + } 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 { + throw new IllegalStateException( + String.format( + "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); + } + } + + /** + * In OpenSearch, it is possible field doesn't have type definition in mapping. but has empty + * value. For example, {"empty_field": []}. + */ + private Optional type(String field) { + return Optional.ofNullable(typeMapping.get(field)); + } + /** * Parse struct content. * From fb0be946b9fcc26b055ac56cb332f2f5453533d7 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 12 Nov 2024 08:45:38 -0800 Subject: [PATCH 29/40] Fix more wildcard import statements Signed-off-by: currantw --- .../predicate/UnaryPredicateOperator.java | 9 ++- .../datetime/DateAddAndAddDateTest.java | 8 ++- .../data/type/OpenSearchDataTypeTest.java | 68 ++++++++++++------- .../data/value/OpenSearchExprIpValueTest.java | 4 +- .../value/OpenSearchExprValueFactoryTest.java | 44 +++++++++--- 5 files changed, 98 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java index 7d54dd8339..aefb15e999 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java @@ -17,7 +17,14 @@ import org.opensearch.sql.data.model.ExprBooleanValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.expression.function.*; +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.expression.function.FunctionBuilder; +import org.opensearch.sql.expression.function.FunctionDSL; +import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.expression.function.FunctionSignature; +import org.opensearch.sql.expression.function.SerializableFunction; /** * The definition of unary predicate function not, Accepts one Boolean value and produces a Boolean. diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java index c5dfdefe00..b4ab3a8567 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateAddAndAddDateTest.java @@ -10,7 +10,13 @@ import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; -import java.time.*; +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.Period; +import java.time.ZoneOffset; import org.junit.jupiter.api.Test; import org.opensearch.sql.exception.ExpressionEvaluationException; 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 c46f4f141c..abbd524441 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 @@ -5,9 +5,29 @@ package org.opensearch.sql.opensearch.data.type; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeFalse; -import static org.opensearch.sql.data.type.ExprCoreType.*; +import static org.opensearch.sql.data.type.ExprCoreType.ARRAY; +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.LONG; +import static org.opensearch.sql.data.type.ExprCoreType.SHORT; +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.TIMESTAMP; +import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; import static org.opensearch.sql.opensearch.data.type.OpenSearchDataType.MappingType; import java.util.Map; @@ -34,6 +54,28 @@ class OpenSearchDataTypeTest { private static final OpenSearchDateType dateType = OpenSearchDateType.of(emptyFormatString); + private static Stream getTestDataWithType() { + return Stream.of( + Arguments.of(MappingType.Text, "text", OpenSearchTextType.of()), + Arguments.of(MappingType.Keyword, "keyword", STRING), + Arguments.of(MappingType.Byte, "byte", BYTE), + Arguments.of(MappingType.Short, "short", SHORT), + Arguments.of(MappingType.Integer, "integer", INTEGER), + Arguments.of(MappingType.Long, "long", LONG), + Arguments.of(MappingType.HalfFloat, "half_float", FLOAT), + Arguments.of(MappingType.Float, "float", FLOAT), + Arguments.of(MappingType.ScaledFloat, "scaled_float", DOUBLE), + Arguments.of(MappingType.Double, "double", DOUBLE), + Arguments.of(MappingType.Boolean, "boolean", BOOLEAN), + Arguments.of(MappingType.Date, "timestamp", TIMESTAMP), + Arguments.of(MappingType.DateNanos, "timestamp", TIMESTAMP), + Arguments.of(MappingType.Object, "object", STRUCT), + Arguments.of(MappingType.Nested, "nested", ARRAY), + Arguments.of(MappingType.GeoPoint, "geo_point", OpenSearchGeoPointType.of()), + Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of()), + Arguments.of(MappingType.Ip, "ip", STRING)); + } + @Test public void isCompatible() { assertTrue(STRING.isCompatible(textType)); @@ -71,28 +113,6 @@ public void shouldCast() { assertFalse(textKeywordType.shouldCast(STRING)); } - private static Stream getTestDataWithType() { - return Stream.of( - Arguments.of(MappingType.Text, "text", OpenSearchTextType.of()), - Arguments.of(MappingType.Keyword, "keyword", STRING), - Arguments.of(MappingType.Byte, "byte", BYTE), - Arguments.of(MappingType.Short, "short", SHORT), - Arguments.of(MappingType.Integer, "integer", INTEGER), - Arguments.of(MappingType.Long, "long", LONG), - Arguments.of(MappingType.HalfFloat, "half_float", FLOAT), - Arguments.of(MappingType.Float, "float", FLOAT), - Arguments.of(MappingType.ScaledFloat, "scaled_float", DOUBLE), - Arguments.of(MappingType.Double, "double", DOUBLE), - Arguments.of(MappingType.Boolean, "boolean", BOOLEAN), - Arguments.of(MappingType.Date, "timestamp", TIMESTAMP), - Arguments.of(MappingType.DateNanos, "timestamp", TIMESTAMP), - Arguments.of(MappingType.Object, "object", STRUCT), - Arguments.of(MappingType.Nested, "nested", ARRAY), - Arguments.of(MappingType.GeoPoint, "geo_point", OpenSearchGeoPointType.of()), - Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of()), - Arguments.of(MappingType.Ip, "ip", STRING)); - } - @ParameterizedTest(name = "{1}") @MethodSource("getTestDataWithType") public void of_MappingType(MappingType mappingType, String name, ExprType dataType) { 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 8d23d4bd90..8483d82b20 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 @@ -5,7 +5,9 @@ package org.opensearch.sql.opensearch.data.value; -import static org.junit.jupiter.api.Assertions.*; +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; 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 534b8e5e8d..91c0539d4b 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 @@ -5,9 +5,34 @@ package org.opensearch.sql.opensearch.data.value; -import static org.junit.jupiter.api.Assertions.*; -import static org.opensearch.sql.data.model.ExprValueUtils.*; -import static org.opensearch.sql.data.type.ExprCoreType.*; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opensearch.sql.data.model.ExprValueUtils.booleanValue; +import static org.opensearch.sql.data.model.ExprValueUtils.byteValue; +import static org.opensearch.sql.data.model.ExprValueUtils.collectionValue; +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.longValue; +import static org.opensearch.sql.data.model.ExprValueUtils.nullValue; +import static org.opensearch.sql.data.model.ExprValueUtils.shortValue; +import static org.opensearch.sql.data.model.ExprValueUtils.stringValue; +import static org.opensearch.sql.data.type.ExprCoreType.ARRAY; +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.LONG; +import static org.opensearch.sql.data.type.ExprCoreType.SHORT; +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; +import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -24,7 +49,13 @@ import org.junit.jupiter.api.Test; import org.opensearch.OpenSearchParseException; import org.opensearch.geometry.utils.Geohash; -import org.opensearch.sql.data.model.*; +import org.opensearch.sql.data.model.ExprCollectionValue; +import org.opensearch.sql.data.model.ExprDateValue; +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.ExprTupleValue; +import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.data.type.OpenSearchDateType; import org.opensearch.sql.opensearch.data.type.OpenSearchTextType; @@ -88,10 +119,9 @@ class OpenSearchExprValueFactoryTest { .put("geoV", OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint)) .put("binaryV", OpenSearchDataType.of(OpenSearchDataType.MappingType.Binary)) .build(); - + private static final double TOLERANCE = 1E-5; private final OpenSearchExprValueFactory exprValueFactory = new OpenSearchExprValueFactory(MAPPING, true); - private final OpenSearchExprValueFactory exprValueFactoryNoArrays = new OpenSearchExprValueFactory(MAPPING, false); @@ -726,8 +756,6 @@ public void constructIP() { assertEquals(stringValue(valueIpv6), constructFromObject(fieldIp, valueIpv6)); } - private static final double TOLERANCE = 1E-5; - @Test public void constructGeoPoint() { final double lat = 42.60355556; From 3af85af2890657dea298e44cf3226c227d601a6a Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 12 Nov 2024 09:04:25 -0800 Subject: [PATCH 30/40] Add miss test data for doctest Signed-off-by: currantw --- doctest/test_data/weblogs.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 doctest/test_data/weblogs.json diff --git a/doctest/test_data/weblogs.json b/doctest/test_data/weblogs.json new file mode 100644 index 0000000000..4228e9c4d2 --- /dev/null +++ b/doctest/test_data/weblogs.json @@ -0,0 +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"} From aa9cb8565ce70d4b0530e34846282066e103138b Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 13 Nov 2024 14:12:13 -0800 Subject: [PATCH 31/40] Fix some more failing integration tests Signed-off-by: currantw --- .../src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java | 2 +- integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java | 2 +- .../src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 74acad4f52..08ad78e33c 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 @@ -157,7 +157,7 @@ public void ipTypeShouldPassJdbcFormatter() { executeQuery( "SELECT host AS hostIP FROM " + TestsConstants.TEST_INDEX_WEBLOG + " ORDER BY hostIP", "jdbc"), - containsString("\"type\": \"ip\"")); + containsString("\"type\": \"string\"")); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java index fe5c2ff270..12c8049f56 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java @@ -50,7 +50,7 @@ public void test_nonnumeric_data_types() throws IOException { schema("binary_value", "binary"), schema("date_value", "timestamp"), schema("date_nanos_value", "timestamp"), - schema("ip_value", "ip"), + schema("ip_value", "string"), schema("object_value", "struct"), schema("nested_value", "array"), schema("geo_point_value", "geo_point")); diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java index c1356ce838..3f92925c80 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java @@ -89,7 +89,7 @@ public void typeof_opensearch_types() throws IOException { "BOOLEAN", "OBJECT", "KEYWORD", - "IP", + "KEYWORD", "BINARY", "GEO_POINT")); } From cff2418b45b4ba0dfa054b4148dff83db94625e9 Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 14 Nov 2024 09:47:21 -0800 Subject: [PATCH 32/40] Fix failing doc test Signed-off-by: currantw --- docs/user/dql/metadata.rst | 3 ++- docs/user/ppl/functions/ip.rst | 7 +++---- doctest/test_docs.py | 16 ++++++++-------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/user/dql/metadata.rst b/docs/user/dql/metadata.rst index fa233020a3..aba4eb0c75 100644 --- a/docs/user/dql/metadata.rst +++ b/docs/user/dql/metadata.rst @@ -35,7 +35,7 @@ Example 1: Show All Indices Information SQL query:: os> SHOW TABLES LIKE '%' - fetched rows / total rows = 9/9 + fetched rows / total rows = 10/10 +----------------+-------------+-----------------+------------+---------+----------+------------+-----------+---------------------------+----------------+ | TABLE_CAT | TABLE_SCHEM | TABLE_NAME | TABLE_TYPE | REMARKS | TYPE_CAT | TYPE_SCHEM | TYPE_NAME | SELF_REFERENCING_COL_NAME | REF_GENERATION | |----------------+-------------+-----------------+------------+---------+----------+------------+-----------+---------------------------+----------------| @@ -47,6 +47,7 @@ SQL query:: | docTestCluster | null | nested | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | nyc_taxi | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | people | BASE TABLE | null | null | null | null | null | null | + | docTestCluster | null | weblogs | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | wildcard | BASE TABLE | null | null | null | null | null | null | +----------------+-------------+-----------------+------------+---------+----------+------------+-----------+---------------------------+----------------+ diff --git a/docs/user/ppl/functions/ip.rst b/docs/user/ppl/functions/ip.rst index f6c5d3e787..ff6d929d45 100644 --- a/docs/user/ppl/functions/ip.rst +++ b/docs/user/ppl/functions/ip.rst @@ -22,11 +22,10 @@ Return type: BOOLEAN Example:: - os> source=weblogs | where cidrmatch(host, "199.120.110.0/24") | fields host - fetched rows / total rows = 1/3 + os> source=weblogs | where cidrmatch(host, "199.120.110.0/24") | fields host ; + fetched rows / total rows = 1/1 +----------------+ | host | - |----------------+ + |----------------| | 199.120.110.21 | +----------------+ - diff --git a/doctest/test_docs.py b/doctest/test_docs.py index 881078a9bd..30306dce2e 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -1,21 +1,20 @@ # Copyright OpenSearch Contributors # SPDX-License-Identifier: Apache-2.0 +import click import doctest +import json import os import os.path -import zc.customdoctests -import json -import re import random +import re import subprocess import unittest -import click - +import zc.customdoctests from functools import partial +from opensearch_sql_cli.formatter import Formatter from opensearch_sql_cli.opensearch_connection import OpenSearchConnection from opensearch_sql_cli.utils import OutputSettings -from opensearch_sql_cli.formatter import Formatter from opensearchpy import OpenSearch, helpers ENDPOINT = "http://localhost:9200" @@ -29,7 +28,7 @@ WILDCARD = "wildcard" NESTED = "nested" DATASOURCES = ".ql-datasources" - +WEBLOGS = "weblogs" class DocTestConnection(OpenSearchConnection): @@ -122,6 +121,7 @@ def set_up_test_indices(test): load_file("wildcard.json", index_name=WILDCARD) load_file("nested_objects.json", index_name=NESTED) load_file("datasources.json", index_name=DATASOURCES) + load_file("weblogs.json", index_name=WEBLOGS) def load_file(filename, index_name): @@ -150,7 +150,7 @@ def set_up(test): def tear_down(test): # drop leftover tables after each test - test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI, BOOKS, APACHE, WILDCARD, NESTED], ignore_unavailable=True) + test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI, BOOKS, APACHE, WILDCARD, NESTED, WEBLOGS], ignore_unavailable=True) docsuite = partial(doctest.DocFileSuite, From 8324c4c0ed13e224ef17bce46363383b15632a84 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 19 Nov 2024 09:59:30 -0800 Subject: [PATCH 33/40] Address minor review comments. Signed-off-by: currantw --- doctest/test_docs.py | 11 ++++++----- .../data/value/OpenSearchExprValueFactory.java | 4 +--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/doctest/test_docs.py b/doctest/test_docs.py index 30306dce2e..1d46766c6d 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -1,20 +1,21 @@ # Copyright OpenSearch Contributors # SPDX-License-Identifier: Apache-2.0 -import click import doctest -import json import os import os.path -import random +import zc.customdoctests +import json import re +import random import subprocess import unittest -import zc.customdoctests +import click + from functools import partial -from opensearch_sql_cli.formatter import Formatter from opensearch_sql_cli.opensearch_connection import OpenSearchConnection from opensearch_sql_cli.utils import OutputSettings +from opensearch_sql_cli.formatter import Formatter from opensearchpy import OpenSearch, helpers ENDPOINT = "http://localhost:9200" 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 9908439886..ab63663f87 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 @@ -228,9 +228,7 @@ public void extendTypeMapping(Map typeMapping) { for (var field : typeMapping.keySet()) { // Prevent overwriting, because aggregation engine may be not aware // of all niceties of all types. - if (!this.typeMapping.containsKey(field)) { - this.typeMapping.put(field, typeMapping.get(field)); - } + this.typeMapping.putIfAbsent(field, typeMapping.get(field)); } } From e758077fc07ed3f2713e9bed1938d84fe26b4d3a Mon Sep 17 00:00:00 2001 From: currantw Date: Thu, 21 Nov 2024 15:48:42 -0800 Subject: [PATCH 34/40] Revert changes to cast IP to string. Signed-off-by: currantw --- .../PercentileApproxAggregatorTest.java | 10 +- .../org/opensearch/sql/legacy/JdbcTestIT.java | 2 +- .../org/opensearch/sql/ppl/DataTypeIT.java | 2 +- .../org/opensearch/sql/ppl/IPFunctionIT.java | 12 +- .../opensearch/sql/ppl/SystemFunctionIT.java | 2 +- .../weblogs_index_mapping.json | 5 +- integ-test/src/test/resources/weblogs.json | 6 +- .../data/type/OpenSearchDataType.java | 2 +- .../data/type/OpenSearchIpType.java | 4 +- .../data/value/OpenSearchExprIpValue.java | 5 - .../value/OpenSearchExprValueFactory.java | 194 +++++++++--------- .../data/type/OpenSearchDataTypeTest.java | 44 ++-- .../data/value/OpenSearchExprIpValueTest.java | 5 - .../value/OpenSearchExprValueFactoryTest.java | 17 +- 14 files changed, 153 insertions(+), 157 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java b/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java index c0b72813d2..33fc325204 100644 --- a/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/aggregation/PercentileApproxAggregatorTest.java @@ -13,12 +13,18 @@ package org.opensearch.sql.expression.aggregation; -import static org.junit.jupiter.api.Assertions.*; +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.ArgumentMatchers.any; import static org.mockito.Mockito.when; import static org.opensearch.sql.data.model.ExprValueUtils.integerValue; import static org.opensearch.sql.data.model.ExprValueUtils.longValue; -import static org.opensearch.sql.data.type.ExprCoreType.*; +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.LONG; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; import java.util.ArrayList; import java.util.List; 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 08ad78e33c..74acad4f52 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 @@ -157,7 +157,7 @@ public void ipTypeShouldPassJdbcFormatter() { executeQuery( "SELECT host AS hostIP FROM " + TestsConstants.TEST_INDEX_WEBLOG + " ORDER BY hostIP", "jdbc"), - containsString("\"type\": \"string\"")); + containsString("\"type\": \"ip\"")); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java index 12c8049f56..fe5c2ff270 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/DataTypeIT.java @@ -50,7 +50,7 @@ public void test_nonnumeric_data_types() throws IOException { schema("binary_value", "binary"), schema("date_value", "timestamp"), schema("date_nanos_value", "timestamp"), - schema("ip_value", "string"), + schema("ip_value", "ip"), schema("object_value", "struct"), schema("nested_value", "array"), schema("geo_point_value", "geo_point")); diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java index cd64b7359b..8fe14809ac 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -31,27 +31,27 @@ public void test_cidrmatch() throws IOException { result = executeQuery( String.format( - "source=%s | where cidrmatch(host, '199.120.111.0/24') | fields host", + "source=%s | where cidrmatch(host_string, '199.120.111.0/24') | fields host_string", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("host", null, "string")); + verifySchema(result, schema("host_string", null, "string")); verifyDataRows(result); // One match result = executeQuery( String.format( - "source=%s | where cidrmatch(host, '199.120.110.0/24') | fields host", + "source=%s | where cidrmatch(host_string, '199.120.110.0/24') | fields host_string", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("host", null, "string")); + verifySchema(result, schema("host_string", null, "string")); verifyDataRows(result, rows("199.120.110.21")); // Multiple matches result = executeQuery( String.format( - "source=%s | where cidrmatch(host, '199.0.0.0/8') | fields host", + "source=%s | where cidrmatch(host_string, '199.0.0.0/8') | fields host_string", TEST_INDEX_WEBLOG)); - verifySchema(result, schema("host", null, "string")); + verifySchema(result, schema("host_string", null, "string")); verifyDataRows(result, rows("199.72.81.55"), rows("199.120.110.21")); } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java index 3f92925c80..c1356ce838 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/SystemFunctionIT.java @@ -89,7 +89,7 @@ public void typeof_opensearch_types() throws IOException { "BOOLEAN", "OBJECT", "KEYWORD", - "KEYWORD", + "IP", "BINARY", "GEO_POINT")); } 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 05b9784313..bff3e20bb9 100644 --- a/integ-test/src/test/resources/indexDefinitions/weblogs_index_mapping.json +++ b/integ-test/src/test/resources/indexDefinitions/weblogs_index_mapping.json @@ -1,9 +1,12 @@ { "mappings": { "properties": { - "host": { + "host_ip": { "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 4228e9c4d2..d2e9a968f8 100644 --- a/integ-test/src/test/resources/weblogs.json +++ b/integ-test/src/test/resources/weblogs.json @@ -1,6 +1,6 @@ {"index":{}} -{"host": "199.72.81.55", "method": "GET", "url": "/history/apollo/", "response": "200", "bytes": "6245"} +{"host_ip": "199.72.81.55", "host_string": "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"} +{"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"} {"index":{}} -{"host": "205.212.115.106", "method": "GET", "url": "/shuttle/countdown/countdown.html", "response": "200", "bytes": "3985"} +{"host_ip": "205.212.115.106", "host_string": "205.212.115.106", "method": "GET", "url": "/shuttle/countdown/countdown.html", "response": "200", "bytes": "3985"} 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 e86e604fa1..c35eacfc72 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.STRING), + Ip("ip", ExprCoreType.UNKNOWN), GeoPoint("geo_point", ExprCoreType.UNKNOWN), Binary("binary", ExprCoreType.UNKNOWN), Date("date", ExprCoreType.TIMESTAMP), 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 index 9f3a521ca1..22581ec28c 100644 --- 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 @@ -5,7 +5,7 @@ package org.opensearch.sql.opensearch.data.type; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; +import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; import lombok.EqualsAndHashCode; @@ -20,7 +20,7 @@ public class OpenSearchIpType extends OpenSearchDataType { private OpenSearchIpType() { super(MappingType.Ip); - exprCoreType = STRING; + exprCoreType = UNKNOWN; } public static OpenSearchIpType of() { 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 index 3ecc39457c..30b3784bfc 100644 --- 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 @@ -45,9 +45,4 @@ public boolean equal(ExprValue other) { public int hashCode() { return Objects.hashCode(ip); } - - @Override - public String stringValue() { - return 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 ab63663f87..da51c34cec 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 @@ -71,8 +71,32 @@ /** Construct ExprValue from OpenSearch response. */ public class OpenSearchExprValueFactory { + /** The Mapping of Field and ExprType. */ + private final Map typeMapping; + + /** Whether to support nested value types (such as arrays) */ + private final boolean fieldTypeTolerance; + + /** + * Extend existing mapping by new data without overwrite. Called from aggregation only {@see + * AggregationQueryBuilder#buildTypeMapping}. + * + * @param typeMapping A data type mapping produced by aggregation. + */ + public void extendTypeMapping(Map typeMapping) { + for (var field : typeMapping.keySet()) { + // Prevent overwriting, because aggregation engine may be not aware + // of all niceties of all types. + this.typeMapping.putIfAbsent(field, typeMapping.get(field)); + } + } + + @Getter @Setter private OpenSearchAggregationResponseParser parser; + private static final String TOP_PATH = ""; + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final Map> typeActionMap = new ImmutableMap.Builder>() .put( @@ -110,20 +134,12 @@ public class OpenSearchExprValueFactory { OpenSearchExprValueFactory::createOpenSearchDateType) .put( OpenSearchDataType.of(OpenSearchDataType.MappingType.Ip), - (c, dt) -> new ExprStringValue(c.stringValue())) + (c, dt) -> new OpenSearchExprIpValue(c.stringValue())) .put( OpenSearchDataType.of(OpenSearchDataType.MappingType.Binary), (c, dt) -> new OpenSearchExprBinaryValue(c.stringValue())) .build(); - /** The Mapping of Field and ExprType. */ - private final Map typeMapping; - - /** Whether to support nested value types (such as arrays) */ - private final boolean fieldTypeTolerance; - - @Getter @Setter private OpenSearchAggregationResponseParser parser; - /** Constructor of OpenSearchExprValueFactory. */ public OpenSearchExprValueFactory( Map typeMapping, boolean fieldTypeTolerance) { @@ -131,6 +147,80 @@ public OpenSearchExprValueFactory( this.fieldTypeTolerance = fieldTypeTolerance; } + /** + * + * + * 
+   * The struct construction has the following assumption:
+   *  1. The field has OpenSearch Object data type.
+   *     See 
+   *       docs
+   *  2. The deeper field is flattened in the typeMapping. e.g.
+   *     { "employ",       "STRUCT"  }
+   *     { "employ.id",    "INTEGER" }
+   *     { "employ.state", "STRING"  }
+   *
+ */ + public ExprValue construct(String jsonString, boolean supportArrays) { + try { + return parse( + new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)), + TOP_PATH, + Optional.of(STRUCT), + fieldTypeTolerance || supportArrays); + } catch (JsonProcessingException e) { + throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e); + } + } + + /** + * Construct ExprValue from field and its value object. Throw exception if trying to construct + * from field of unsupported type.
+ * Todo, add IP, GeoPoint support after we have function implementation around it. + * + * @param field field name + * @param value value object + * @return ExprValue + */ + public ExprValue construct(String field, Object value, boolean supportArrays) { + return parse(new ObjectContent(value), field, type(field), supportArrays); + } + + private ExprValue parse( + Content content, String field, Optional fieldType, boolean supportArrays) { + if (content.isNull() || !fieldType.isPresent()) { + return ExprNullValue.of(); + } + + final ExprType type = fieldType.get(); + + if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { + return parseGeoPoint(content, supportArrays); + } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Nested)) + || content.isArray()) { + return parseArray(content, field, type, supportArrays); + } 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 { + throw new IllegalStateException( + String.format( + "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); + } + } + } + + /** + * In OpenSearch, it is possible field doesn't have type definition in mapping. but has empty + * value. For example, {"empty_field": []}. + */ + private Optional type(String field) { + return Optional.ofNullable(typeMapping.get(field)); + } + /** * Parse value with the first matching formatter into {@link ExprValue} with corresponding {@link * ExprCoreType}. @@ -218,92 +308,6 @@ private static ExprValue createOpenSearchDateType(Content value, ExprType type) return new ExprTimestampValue((Instant) value.objectValue()); } - /** - * Extend existing mapping by new data without overwrite. Called from aggregation only {@see - * AggregationQueryBuilder#buildTypeMapping}. - * - * @param typeMapping A data type mapping produced by aggregation. - */ - public void extendTypeMapping(Map typeMapping) { - for (var field : typeMapping.keySet()) { - // Prevent overwriting, because aggregation engine may be not aware - // of all niceties of all types. - this.typeMapping.putIfAbsent(field, typeMapping.get(field)); - } - } - - /** - * - * - * 
-   * The struct construction has the following assumption:
-   *  1. The field has OpenSearch Object data type.
-   *     See 
-   *       docs
-   *  2. The deeper field is flattened in the typeMapping. e.g.
-   *     { "employ",       "STRUCT"  }
-   *     { "employ.id",    "INTEGER" }
-   *     { "employ.state", "STRING"  }
-   *
- */ - public ExprValue construct(String jsonString, boolean supportArrays) { - try { - return parse( - new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)), - TOP_PATH, - Optional.of(STRUCT), - fieldTypeTolerance || supportArrays); - } catch (JsonProcessingException e) { - throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e); - } - } - - /** - * Construct ExprValue from field and its value object. Throw exception if trying to construct - * from field of unsupported type.
- * Todo, add IP, GeoPoint support after we have function implementation around it. - * - * @param field field name - * @param value value object - * @return ExprValue - */ - public ExprValue construct(String field, Object value, boolean supportArrays) { - return parse(new ObjectContent(value), field, type(field), supportArrays); - } - - private ExprValue parse( - Content content, String field, Optional fieldType, boolean supportArrays) { - if (content.isNull() || !fieldType.isPresent()) { - return ExprNullValue.of(); - } - - final ExprType type = fieldType.get(); - - if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { - return parseGeoPoint(content, supportArrays); - } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Nested)) - || content.isArray()) { - return parseArray(content, field, type, supportArrays); - } 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 { - throw new IllegalStateException( - String.format( - "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); - } - } - - /** - * In OpenSearch, it is possible field doesn't have type definition in mapping. but has empty - * value. For example, {"empty_field": []}. - */ - private Optional type(String field) { - return Optional.ofNullable(typeMapping.get(field)); - } - /** * Parse struct content. * 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 abbd524441..76fbbd6e65 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 @@ -54,28 +54,6 @@ class OpenSearchDataTypeTest { private static final OpenSearchDateType dateType = OpenSearchDateType.of(emptyFormatString); - private static Stream getTestDataWithType() { - return Stream.of( - Arguments.of(MappingType.Text, "text", OpenSearchTextType.of()), - Arguments.of(MappingType.Keyword, "keyword", STRING), - Arguments.of(MappingType.Byte, "byte", BYTE), - Arguments.of(MappingType.Short, "short", SHORT), - Arguments.of(MappingType.Integer, "integer", INTEGER), - Arguments.of(MappingType.Long, "long", LONG), - Arguments.of(MappingType.HalfFloat, "half_float", FLOAT), - Arguments.of(MappingType.Float, "float", FLOAT), - Arguments.of(MappingType.ScaledFloat, "scaled_float", DOUBLE), - Arguments.of(MappingType.Double, "double", DOUBLE), - Arguments.of(MappingType.Boolean, "boolean", BOOLEAN), - Arguments.of(MappingType.Date, "timestamp", TIMESTAMP), - Arguments.of(MappingType.DateNanos, "timestamp", TIMESTAMP), - Arguments.of(MappingType.Object, "object", STRUCT), - Arguments.of(MappingType.Nested, "nested", ARRAY), - Arguments.of(MappingType.GeoPoint, "geo_point", OpenSearchGeoPointType.of()), - Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of()), - Arguments.of(MappingType.Ip, "ip", STRING)); - } - @Test public void isCompatible() { assertTrue(STRING.isCompatible(textType)); @@ -113,6 +91,28 @@ public void shouldCast() { assertFalse(textKeywordType.shouldCast(STRING)); } + private static Stream getTestDataWithType() { + return Stream.of( + Arguments.of(MappingType.Text, "text", OpenSearchTextType.of()), + Arguments.of(MappingType.Keyword, "keyword", STRING), + Arguments.of(MappingType.Byte, "byte", BYTE), + Arguments.of(MappingType.Short, "short", SHORT), + Arguments.of(MappingType.Integer, "integer", INTEGER), + Arguments.of(MappingType.Long, "long", LONG), + Arguments.of(MappingType.HalfFloat, "half_float", FLOAT), + Arguments.of(MappingType.Float, "float", FLOAT), + Arguments.of(MappingType.ScaledFloat, "scaled_float", DOUBLE), + Arguments.of(MappingType.Double, "double", DOUBLE), + Arguments.of(MappingType.Boolean, "boolean", BOOLEAN), + Arguments.of(MappingType.Date, "timestamp", TIMESTAMP), + Arguments.of(MappingType.DateNanos, "timestamp", TIMESTAMP), + Arguments.of(MappingType.Object, "object", STRUCT), + Arguments.of(MappingType.Nested, "nested", ARRAY), + Arguments.of(MappingType.GeoPoint, "geo_point", OpenSearchGeoPointType.of()), + Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of()), + Arguments.of(MappingType.Ip, "ip", OpenSearchIpType.of())); + } + @ParameterizedTest(name = "{1}") @MethodSource("getTestDataWithType") public void of_MappingType(MappingType mappingType, String name, ExprType dataType) { 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 8483d82b20..5ee175f304 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 @@ -42,9 +42,4 @@ void testEqual() { void testHashCode() { assertNotNull(ipValue.hashCode()); } - - @Test - void testStringValue() { - assertEquals(ipString, ipValue.stringValue()); - } } 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 91c0539d4b..d82926077e 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 @@ -51,7 +51,6 @@ 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.ExprStringValue; import org.opensearch.sql.data.model.ExprTimeValue; import org.opensearch.sql.data.model.ExprTimestampValue; import org.opensearch.sql.data.model.ExprTupleValue; @@ -666,7 +665,8 @@ public void constructArrayOfIPsReturnsAll() { final String ip2 = "192.168.0.2"; assertEquals( - new ExprCollectionValue(List.of(new ExprStringValue(ip1), new ExprStringValue(ip2))), + new ExprCollectionValue( + List.of(new OpenSearchExprIpValue(ip1), new OpenSearchExprIpValue(ip2))), tupleValue(String.format("{\"%s\":[\"%s\",\"%s\"]}", fieldIp, ip1, ip2)).get(fieldIp)); } @@ -743,17 +743,10 @@ public void constructStruct() { @Test public void constructIP() { - final String valueIpv4 = "192.168.0.1"; - assertEquals( - stringValue(valueIpv4), - tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, valueIpv4)).get(fieldIp)); - assertEquals(stringValue(valueIpv4), constructFromObject(fieldIp, valueIpv4)); - - final String valueIpv6 = "2001:0db7::ff00:42:8329"; + final String valueIp = "192.168.0.1"; assertEquals( - stringValue(valueIpv6), - tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, valueIpv6)).get(fieldIp)); - assertEquals(stringValue(valueIpv6), constructFromObject(fieldIp, valueIpv6)); + new OpenSearchExprIpValue(valueIp), + tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, valueIp)).get(fieldIp)); } @Test From 677bdc873963eb027b19b5db47dab3aaaddfab01 Mon Sep 17 00:00:00 2001 From: currantw Date: Fri, 22 Nov 2024 10:09:43 -0800 Subject: [PATCH 35/40] Fix failing tests and add TODOs for #3145 (add support for IP address type). Signed-off-by: currantw --- .../java/org/opensearch/sql/expression/ip/IPFunction.java | 2 ++ .../java/org/opensearch/sql/expression/ip/IPFunctionTest.java | 1 + .../src/test/java/org/opensearch/sql/legacy/JdbcTestIT.java | 4 +++- .../src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java index 48738de54c..855d557916 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java @@ -31,6 +31,8 @@ public void register(BuiltinFunctionRepository repository) { } private DefaultFunctionResolver cidrmatch() { + + // TODO #3145: Add support for IP address data type. return define( BuiltinFunctionName.CIDRMATCH.getName(), impl(nullMissingHandling(IPFunction::exprCidrMatch), BOOLEAN, STRING, STRING)); 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 index ccee1783d1..b50bf9fd1f 100644 --- a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java @@ -32,6 +32,7 @@ public class IPFunctionTest { 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"); 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 74acad4f52..005119a9bc 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,7 +155,9 @@ public void dateFunctionNameCaseInsensitiveTest() { public void ipTypeShouldPassJdbcFormatter() { assertThat( executeQuery( - "SELECT host AS hostIP FROM " + TestsConstants.TEST_INDEX_WEBLOG + " ORDER BY hostIP", + "SELECT host_ip AS hostIP FROM " + + TestsConstants.TEST_INDEX_WEBLOG + + " ORDER BY hostIP", "jdbc"), containsString("\"type\": \"ip\"")); } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java index 8fe14809ac..adb044d0d2 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/IPFunctionIT.java @@ -25,6 +25,7 @@ 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 From 43d05d6b1dce23cba68c618892506c11bb8fffce Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 27 Nov 2024 16:13:04 -0800 Subject: [PATCH 36/40] Update `ip.rst` for consistency with Spark Signed-off-by: currantw --- docs/user/ppl/functions/ip.rst | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/user/ppl/functions/ip.rst b/docs/user/ppl/functions/ip.rst index ff6d929d45..3387974af5 100644 --- a/docs/user/ppl/functions/ip.rst +++ b/docs/user/ppl/functions/ip.rst @@ -14,18 +14,25 @@ CIDRMATCH Description >>>>>>>>>>> -Usage: cidrmatch(ip, cidr) returns whether the given IP address is within the specified CIDR IP address range. Supports both IPv4 and IPv6 addresses. +Usage: `cidrmatch(ip, cidr)` checks if `ip` is within the specified `cidr` range. Argument type: STRING, STRING Return type: BOOLEAN -Example:: +Example: - os> source=weblogs | where cidrmatch(host, "199.120.110.0/24") | fields host ; + os> source=weblogs | where cidrmatch(host, '199.120.110.0/24') | fields host fetched rows / total rows = 1/1 +----------------+ | host | |----------------| | 199.120.110.21 | +----------------+ + +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 + From 23219cb51874cfedd757f5e46ea4e2192261dccc Mon Sep 17 00:00:00 2001 From: currantw Date: Wed, 27 Nov 2024 16:14:56 -0800 Subject: [PATCH 37/40] Update `ip.rst` for consistency with Spark Signed-off-by: currantw --- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 1 + 1 file changed, 1 insertion(+) diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 754b7e17a7..54ec23dcb9 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -830,6 +830,7 @@ keywordsCanBeId | textFunctionName | mathematicalFunctionName | positionFunctionName + | conditionFunctionName // commands | SEARCH | DESCRIBE From d3be058cf4567853162289312526566629af029b Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 2 Dec 2024 12:34:03 -0800 Subject: [PATCH 38/40] Rename "function" and "operator" utility classes to be plural. Signed-off-by: currantw --- ...Function.java => AggregatorFunctions.java} | 2 +- ...meFunction.java => DateTimeFunctions.java} | 296 +++++++++--------- .../function/BuiltinFunctionRepository.java | 36 +-- .../ip/{IPFunction.java => IPFunctions.java} | 4 +- ...Function.java => ArithmeticFunctions.java} | 2 +- ...nction.java => MathematicalFunctions.java} | 2 +- ...stOperator.java => TypeCastOperators.java} | 3 +- ...tor.java => BinaryPredicateOperators.java} | 4 +- ...ator.java => UnaryPredicateOperators.java} | 11 +- .../{TextFunction.java => TextFunctions.java} | 22 +- .../sql/planner/physical/FilterOperator.java | 4 +- .../expression/datetime/ToSecondsTest.java | 2 +- .../predicate/UnaryPredicateOperatorTest.java | 4 +- 13 files changed, 197 insertions(+), 195 deletions(-) rename core/src/main/java/org/opensearch/sql/expression/aggregation/{AggregatorFunction.java => AggregatorFunctions.java} (99%) rename core/src/main/java/org/opensearch/sql/expression/datetime/{DateTimeFunction.java => DateTimeFunctions.java} (86%) rename core/src/main/java/org/opensearch/sql/expression/ip/{IPFunction.java => IPFunctions.java} (97%) rename core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/{ArithmeticFunction.java => ArithmeticFunctions.java} (99%) rename core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/{MathematicalFunction.java => MathematicalFunctions.java} (99%) rename core/src/main/java/org/opensearch/sql/expression/operator/convert/{TypeCastOperator.java => TypeCastOperators.java} (99%) rename core/src/main/java/org/opensearch/sql/expression/operator/predicate/{BinaryPredicateOperator.java => BinaryPredicateOperators.java} (98%) rename core/src/main/java/org/opensearch/sql/expression/operator/predicate/{UnaryPredicateOperator.java => UnaryPredicateOperators.java} (93%) rename core/src/main/java/org/opensearch/sql/expression/text/{TextFunction.java => TextFunctions.java} (95%) diff --git a/core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunction.java b/core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunctions.java similarity index 99% rename from core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunction.java rename to core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunctions.java index 631eb2e613..698fb20408 100644 --- a/core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunctions.java @@ -40,7 +40,7 @@ * count accepts values of all types. */ @UtilityClass -public class AggregatorFunction { +public class AggregatorFunctions { /** * Register Aggregation Function. * diff --git a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunctions.java similarity index 86% rename from core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java rename to core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunctions.java index a42a599ad8..2d9dec15f5 100644 --- a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunctions.java @@ -101,7 +101,7 @@ */ @UtilityClass @SuppressWarnings("unchecked") -public class DateTimeFunction { +public class DateTimeFunctions { // The number of seconds per day public static final long SECONDS_PER_DAY = 86400; @@ -357,8 +357,8 @@ private DefaultFunctionResolver adddate() { BuiltinFunctionName.ADDDATE.getName(), (SerializableFunction>[]) (Stream.concat( - get_date_add_date_sub_signatures(DateTimeFunction::exprAddDateInterval), - get_adddate_subdate_signatures(DateTimeFunction::exprAddDateDays)) + get_date_add_date_sub_signatures(DateTimeFunctions::exprAddDateInterval), + get_adddate_subdate_signatures(DateTimeFunctions::exprAddDateDays)) .toArray(SerializableFunction[]::new))); } @@ -375,41 +375,41 @@ private DefaultFunctionResolver addtime() { return define( BuiltinFunctionName.ADDTIME.getName(), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), TIME, TIME, TIME), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIME, TIME, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), TIME, TIME, DATE), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIME, TIME, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIME, TIME, TIMESTAMP), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIMESTAMP, DATE, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIMESTAMP, DATE, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIMESTAMP, DATE, TIMESTAMP), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIMESTAMP, TIMESTAMP, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIMESTAMP, TIMESTAMP, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprAddTime), TIMESTAMP, TIMESTAMP, TIMESTAMP)); @@ -425,13 +425,13 @@ private DefaultFunctionResolver convert_tz() { return define( BuiltinFunctionName.CONVERT_TZ.getName(), impl( - nullMissingHandling(DateTimeFunction::exprConvertTZ), + nullMissingHandling(DateTimeFunctions::exprConvertTZ), TIMESTAMP, TIMESTAMP, STRING, STRING), impl( - nullMissingHandling(DateTimeFunction::exprConvertTZ), + nullMissingHandling(DateTimeFunctions::exprConvertTZ), TIMESTAMP, STRING, STRING, @@ -445,9 +445,9 @@ private DefaultFunctionResolver convert_tz() { private DefaultFunctionResolver date() { return define( BuiltinFunctionName.DATE.getName(), - impl(nullMissingHandling(DateTimeFunction::exprDate), DATE, STRING), - impl(nullMissingHandling(DateTimeFunction::exprDate), DATE, DATE), - impl(nullMissingHandling(DateTimeFunction::exprDate), DATE, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprDate), DATE, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprDate), DATE, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprDate), DATE, TIMESTAMP)); } /** @@ -458,35 +458,35 @@ private DefaultFunctionResolver datediff() { return define( BuiltinFunctionName.DATEDIFF.getName(), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), LONG, DATE, DATE), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, DATE, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), LONG, DATE, TIME), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, DATE, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), LONG, TIME, DATE), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, TIME, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), LONG, TIME, TIME), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, TIME, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, TIMESTAMP, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, DATE, TIMESTAMP), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, TIMESTAMP, TIMESTAMP), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, TIMESTAMP, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprDateDiff), + nullMissingHandlingWithProperties(DateTimeFunctions::exprDateDiff), LONG, TIME, TIMESTAMP)); @@ -501,15 +501,15 @@ private DefaultFunctionResolver datediff() { private FunctionResolver datetime() { return define( BuiltinFunctionName.DATETIME.getName(), - impl(nullMissingHandling(DateTimeFunction::exprDateTime), TIMESTAMP, STRING, STRING), - impl(nullMissingHandling(DateTimeFunction::exprDateTimeNoTimezone), TIMESTAMP, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprDateTime), TIMESTAMP, STRING, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprDateTimeNoTimezone), TIMESTAMP, STRING)); } private DefaultFunctionResolver date_add() { return define( BuiltinFunctionName.DATE_ADD.getName(), (SerializableFunction>[]) - get_date_add_date_sub_signatures(DateTimeFunction::exprAddDateInterval) + get_date_add_date_sub_signatures(DateTimeFunctions::exprAddDateInterval) .toArray(SerializableFunction[]::new)); } @@ -517,7 +517,7 @@ private DefaultFunctionResolver date_sub() { return define( BuiltinFunctionName.DATE_SUB.getName(), (SerializableFunction>[]) - get_date_add_date_sub_signatures(DateTimeFunction::exprSubDateInterval) + get_date_add_date_sub_signatures(DateTimeFunctions::exprSubDateInterval) .toArray(SerializableFunction[]::new)); } @@ -525,9 +525,9 @@ private DefaultFunctionResolver date_sub() { private DefaultFunctionResolver day() { return define( BuiltinFunctionName.DAY.getName(), - impl(nullMissingHandling(DateTimeFunction::exprDayOfMonth), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprDayOfMonth), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprDayOfMonth), INTEGER, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprDayOfMonth), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfMonth), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfMonth), INTEGER, STRING)); } /** @@ -537,9 +537,9 @@ private DefaultFunctionResolver day() { private DefaultFunctionResolver dayName() { return define( BuiltinFunctionName.DAYNAME.getName(), - impl(nullMissingHandling(DateTimeFunction::exprDayName), STRING, DATE), - impl(nullMissingHandling(DateTimeFunction::exprDayName), STRING, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprDayName), STRING, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprDayName), STRING, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprDayName), STRING, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprDayName), STRING, STRING)); } /** DAYOFMONTH(STRING/DATE/TIMESTAMP). return the day of the month (1-31). */ @@ -549,12 +549,12 @@ private DefaultFunctionResolver dayOfMonth(BuiltinFunctionName name) { implWithProperties( nullMissingHandlingWithProperties( (functionProperties, arg) -> - DateTimeFunction.dayOfMonthToday(functionProperties.getQueryStartClock())), + DateTimeFunctions.dayOfMonthToday(functionProperties.getQueryStartClock())), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprDayOfMonth), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprDayOfMonth), INTEGER, STRING), - impl(nullMissingHandling(DateTimeFunction::exprDayOfMonth), INTEGER, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprDayOfMonth), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfMonth), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfMonth), INTEGER, TIMESTAMP)); } /** @@ -567,12 +567,12 @@ private DefaultFunctionResolver dayOfWeek(FunctionName name) { implWithProperties( nullMissingHandlingWithProperties( (functionProperties, arg) -> - DateTimeFunction.dayOfWeekToday(functionProperties.getQueryStartClock())), + DateTimeFunctions.dayOfWeekToday(functionProperties.getQueryStartClock())), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprDayOfWeek), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprDayOfWeek), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprDayOfWeek), INTEGER, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprDayOfWeek), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfWeek), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfWeek), INTEGER, STRING)); } /** DAYOFYEAR(STRING/DATE/TIMESTAMP). return the day of the year for date (1-366). */ @@ -582,111 +582,111 @@ private DefaultFunctionResolver dayOfYear(BuiltinFunctionName dayOfYear) { implWithProperties( nullMissingHandlingWithProperties( (functionProperties, arg) -> - DateTimeFunction.dayOfYearToday(functionProperties.getQueryStartClock())), + DateTimeFunctions.dayOfYearToday(functionProperties.getQueryStartClock())), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprDayOfYear), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprDayOfYear), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprDayOfYear), INTEGER, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprDayOfYear), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfYear), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprDayOfYear), INTEGER, STRING)); } private DefaultFunctionResolver extract() { return define( BuiltinFunctionName.EXTRACT.getName(), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprExtractForTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprExtractForTime), LONG, STRING, TIME), - impl(nullMissingHandling(DateTimeFunction::exprExtract), LONG, STRING, DATE), - impl(nullMissingHandling(DateTimeFunction::exprExtract), LONG, STRING, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprExtract), LONG, STRING, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprExtract), LONG, STRING, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprExtract), LONG, STRING, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprExtract), LONG, STRING, STRING)); } /** FROM_DAYS(LONG). return the date value given the day number N. */ private DefaultFunctionResolver from_days() { return define( BuiltinFunctionName.FROM_DAYS.getName(), - impl(nullMissingHandling(DateTimeFunction::exprFromDays), DATE, LONG)); + impl(nullMissingHandling(DateTimeFunctions::exprFromDays), DATE, LONG)); } private FunctionResolver from_unixtime() { return define( BuiltinFunctionName.FROM_UNIXTIME.getName(), - impl(nullMissingHandling(DateTimeFunction::exprFromUnixTime), TIMESTAMP, DOUBLE), + impl(nullMissingHandling(DateTimeFunctions::exprFromUnixTime), TIMESTAMP, DOUBLE), impl( - nullMissingHandling(DateTimeFunction::exprFromUnixTimeFormat), STRING, DOUBLE, STRING)); + nullMissingHandling(DateTimeFunctions::exprFromUnixTimeFormat), STRING, DOUBLE, STRING)); } private DefaultFunctionResolver get_format() { return define( BuiltinFunctionName.GET_FORMAT.getName(), - impl(nullMissingHandling(DateTimeFunction::exprGetFormat), STRING, STRING, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprGetFormat), STRING, STRING, STRING)); } /** HOUR(STRING/TIME/DATE/TIMESTAMP). return the hour value for time. */ private DefaultFunctionResolver hour(BuiltinFunctionName name) { return define( name.getName(), - impl(nullMissingHandling(DateTimeFunction::exprHour), INTEGER, STRING), - impl(nullMissingHandling(DateTimeFunction::exprHour), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprHour), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprHour), INTEGER, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprHour), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprHour), INTEGER, TIME), + impl(nullMissingHandling(DateTimeFunctions::exprHour), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprHour), INTEGER, TIMESTAMP)); } private DefaultFunctionResolver last_day() { return define( BuiltinFunctionName.LAST_DAY.getName(), - impl(nullMissingHandling(DateTimeFunction::exprLastDay), DATE, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprLastDay), DATE, STRING), implWithProperties( nullMissingHandlingWithProperties( (functionProperties, arg) -> - DateTimeFunction.exprLastDayToday(functionProperties.getQueryStartClock())), + DateTimeFunctions.exprLastDayToday(functionProperties.getQueryStartClock())), DATE, TIME), - impl(nullMissingHandling(DateTimeFunction::exprLastDay), DATE, DATE), - impl(nullMissingHandling(DateTimeFunction::exprLastDay), DATE, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprLastDay), DATE, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprLastDay), DATE, TIMESTAMP)); } private FunctionResolver makedate() { return define( BuiltinFunctionName.MAKEDATE.getName(), - impl(nullMissingHandling(DateTimeFunction::exprMakeDate), DATE, DOUBLE, DOUBLE)); + impl(nullMissingHandling(DateTimeFunctions::exprMakeDate), DATE, DOUBLE, DOUBLE)); } private FunctionResolver maketime() { return define( BuiltinFunctionName.MAKETIME.getName(), - impl(nullMissingHandling(DateTimeFunction::exprMakeTime), TIME, DOUBLE, DOUBLE, DOUBLE)); + impl(nullMissingHandling(DateTimeFunctions::exprMakeTime), TIME, DOUBLE, DOUBLE, DOUBLE)); } /** MICROSECOND(STRING/TIME/TIMESTAMP). return the microsecond value for time. */ private DefaultFunctionResolver microsecond() { return define( BuiltinFunctionName.MICROSECOND.getName(), - impl(nullMissingHandling(DateTimeFunction::exprMicrosecond), INTEGER, STRING), - impl(nullMissingHandling(DateTimeFunction::exprMicrosecond), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprMicrosecond), INTEGER, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprMicrosecond), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprMicrosecond), INTEGER, TIME), + impl(nullMissingHandling(DateTimeFunctions::exprMicrosecond), INTEGER, TIMESTAMP)); } /** MINUTE(STRING/TIME/TIMESTAMP). return the minute value for time. */ private DefaultFunctionResolver minute(BuiltinFunctionName name) { return define( name.getName(), - impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, STRING), - impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprMinute), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprMinute), INTEGER, TIME), + impl(nullMissingHandling(DateTimeFunctions::exprMinute), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprMinute), INTEGER, TIMESTAMP)); } /** MINUTE(STRING/TIME/TIMESTAMP). return the minute value for time. */ private DefaultFunctionResolver minute_of_day() { return define( BuiltinFunctionName.MINUTE_OF_DAY.getName(), - impl(nullMissingHandling(DateTimeFunction::exprMinuteOfDay), INTEGER, STRING), - impl(nullMissingHandling(DateTimeFunction::exprMinuteOfDay), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprMinuteOfDay), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprMinuteOfDay), INTEGER, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprMinuteOfDay), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprMinuteOfDay), INTEGER, TIME), + impl(nullMissingHandling(DateTimeFunctions::exprMinuteOfDay), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprMinuteOfDay), INTEGER, TIMESTAMP)); } /** MONTH(STRING/DATE/TIMESTAMP). return the month for date (1-12). */ @@ -696,21 +696,21 @@ private DefaultFunctionResolver month(BuiltinFunctionName month) { implWithProperties( nullMissingHandlingWithProperties( (functionProperties, arg) -> - DateTimeFunction.monthOfYearToday(functionProperties.getQueryStartClock())), + DateTimeFunctions.monthOfYearToday(functionProperties.getQueryStartClock())), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprMonth), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprMonth), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprMonth), INTEGER, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprMonth), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprMonth), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprMonth), INTEGER, STRING)); } /** MONTHNAME(STRING/DATE/TIMESTAMP). return the full name of the month for date. */ private DefaultFunctionResolver monthName() { return define( BuiltinFunctionName.MONTHNAME.getName(), - impl(nullMissingHandling(DateTimeFunction::exprMonthName), STRING, DATE), - impl(nullMissingHandling(DateTimeFunction::exprMonthName), STRING, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprMonthName), STRING, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprMonthName), STRING, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprMonthName), STRING, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprMonthName), STRING, STRING)); } /** @@ -720,7 +720,7 @@ private DefaultFunctionResolver monthName() { private DefaultFunctionResolver period_add() { return define( BuiltinFunctionName.PERIOD_ADD.getName(), - impl(nullMissingHandling(DateTimeFunction::exprPeriodAdd), INTEGER, INTEGER, INTEGER)); + impl(nullMissingHandling(DateTimeFunctions::exprPeriodAdd), INTEGER, INTEGER, INTEGER)); } /** @@ -731,35 +731,35 @@ private DefaultFunctionResolver period_add() { private DefaultFunctionResolver period_diff() { return define( BuiltinFunctionName.PERIOD_DIFF.getName(), - impl(nullMissingHandling(DateTimeFunction::exprPeriodDiff), INTEGER, INTEGER, INTEGER)); + impl(nullMissingHandling(DateTimeFunctions::exprPeriodDiff), INTEGER, INTEGER, INTEGER)); } /** QUARTER(STRING/DATE/TIMESTAMP). return the month for date (1-4). */ private DefaultFunctionResolver quarter() { return define( BuiltinFunctionName.QUARTER.getName(), - impl(nullMissingHandling(DateTimeFunction::exprQuarter), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprQuarter), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprQuarter), INTEGER, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprQuarter), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprQuarter), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprQuarter), INTEGER, STRING)); } private DefaultFunctionResolver sec_to_time() { return define( BuiltinFunctionName.SEC_TO_TIME.getName(), - impl((nullMissingHandling(DateTimeFunction::exprSecToTime)), TIME, INTEGER), - impl((nullMissingHandling(DateTimeFunction::exprSecToTime)), TIME, LONG), - impl((nullMissingHandling(DateTimeFunction::exprSecToTimeWithNanos)), TIME, DOUBLE), - impl((nullMissingHandling(DateTimeFunction::exprSecToTimeWithNanos)), TIME, FLOAT)); + impl((nullMissingHandling(DateTimeFunctions::exprSecToTime)), TIME, INTEGER), + impl((nullMissingHandling(DateTimeFunctions::exprSecToTime)), TIME, LONG), + impl((nullMissingHandling(DateTimeFunctions::exprSecToTimeWithNanos)), TIME, DOUBLE), + impl((nullMissingHandling(DateTimeFunctions::exprSecToTimeWithNanos)), TIME, FLOAT)); } /** SECOND(STRING/TIME/TIMESTAMP). return the second value for time. */ private DefaultFunctionResolver second(BuiltinFunctionName name) { return define( name.getName(), - impl(nullMissingHandling(DateTimeFunction::exprSecond), INTEGER, STRING), - impl(nullMissingHandling(DateTimeFunction::exprSecond), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprSecond), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprSecond), INTEGER, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprSecond), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprSecond), INTEGER, TIME), + impl(nullMissingHandling(DateTimeFunctions::exprSecond), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprSecond), INTEGER, TIMESTAMP)); } private DefaultFunctionResolver subdate() { @@ -767,8 +767,8 @@ private DefaultFunctionResolver subdate() { BuiltinFunctionName.SUBDATE.getName(), (SerializableFunction>[]) (Stream.concat( - get_date_add_date_sub_signatures(DateTimeFunction::exprSubDateInterval), - get_adddate_subdate_signatures(DateTimeFunction::exprSubDateDays)) + get_date_add_date_sub_signatures(DateTimeFunctions::exprSubDateInterval), + get_adddate_subdate_signatures(DateTimeFunctions::exprSubDateDays)) .toArray(SerializableFunction[]::new))); } @@ -785,41 +785,41 @@ private DefaultFunctionResolver subtime() { return define( BuiltinFunctionName.SUBTIME.getName(), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), TIME, TIME, TIME), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIME, TIME, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), TIME, TIME, DATE), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIME, TIME, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIME, TIME, TIMESTAMP), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIMESTAMP, TIMESTAMP, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIMESTAMP, TIMESTAMP, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIMESTAMP, DATE, TIME), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIMESTAMP, DATE, DATE), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIMESTAMP, DATE, TIMESTAMP), implWithProperties( - nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime), + nullMissingHandlingWithProperties(DateTimeFunctions::exprSubTime), TIMESTAMP, TIMESTAMP, TIMESTAMP)); @@ -835,7 +835,7 @@ private DefaultFunctionResolver str_to_date() { implWithProperties( nullMissingHandlingWithProperties( (functionProperties, arg, format) -> - DateTimeFunction.exprStrToDate(functionProperties, arg, format)), + DateTimeFunctions.exprStrToDate(functionProperties, arg, format)), TIMESTAMP, STRING, STRING)); @@ -848,10 +848,10 @@ private DefaultFunctionResolver str_to_date() { private DefaultFunctionResolver time() { return define( BuiltinFunctionName.TIME.getName(), - impl(nullMissingHandling(DateTimeFunction::exprTime), TIME, STRING), - impl(nullMissingHandling(DateTimeFunction::exprTime), TIME, DATE), - impl(nullMissingHandling(DateTimeFunction::exprTime), TIME, TIME), - impl(nullMissingHandling(DateTimeFunction::exprTime), TIME, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprTime), TIME, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprTime), TIME, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprTime), TIME, TIME), + impl(nullMissingHandling(DateTimeFunctions::exprTime), TIME, TIMESTAMP)); } /** @@ -867,16 +867,16 @@ private DefaultFunctionResolver time() { private DefaultFunctionResolver timediff() { return define( BuiltinFunctionName.TIMEDIFF.getName(), - impl(nullMissingHandling(DateTimeFunction::exprTimeDiff), TIME, TIME, TIME)); + impl(nullMissingHandling(DateTimeFunctions::exprTimeDiff), TIME, TIME, TIME)); } /** TIME_TO_SEC(STRING/TIME/TIMESTAMP). return the time argument, converted to seconds. */ private DefaultFunctionResolver time_to_sec() { return define( BuiltinFunctionName.TIME_TO_SEC.getName(), - impl(nullMissingHandling(DateTimeFunction::exprTimeToSec), LONG, STRING), - impl(nullMissingHandling(DateTimeFunction::exprTimeToSec), LONG, TIME), - impl(nullMissingHandling(DateTimeFunction::exprTimeToSec), LONG, TIMESTAMP)); + impl(nullMissingHandling(DateTimeFunctions::exprTimeToSec), LONG, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprTimeToSec), LONG, TIME), + impl(nullMissingHandling(DateTimeFunctions::exprTimeToSec), LONG, TIMESTAMP)); } /** @@ -914,7 +914,7 @@ private DefaultFunctionResolver timestampadd() { return define( BuiltinFunctionName.TIMESTAMPADD.getName(), impl( - nullMissingHandling(DateTimeFunction::exprTimestampAdd), + nullMissingHandling(DateTimeFunctions::exprTimestampAdd), TIMESTAMP, STRING, INTEGER, @@ -943,7 +943,7 @@ private DefaultFunctionResolver timestampdiff() { return define( BuiltinFunctionName.TIMESTAMPDIFF.getName(), impl( - nullMissingHandling(DateTimeFunction::exprTimestampDiff), + nullMissingHandling(DateTimeFunctions::exprTimestampDiff), TIMESTAMP, STRING, TIMESTAMP, @@ -962,9 +962,9 @@ private DefaultFunctionResolver timestampdiff() { private DefaultFunctionResolver to_days() { return define( BuiltinFunctionName.TO_DAYS.getName(), - impl(nullMissingHandling(DateTimeFunction::exprToDays), LONG, STRING), - impl(nullMissingHandling(DateTimeFunction::exprToDays), LONG, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprToDays), LONG, DATE)); + impl(nullMissingHandling(DateTimeFunctions::exprToDays), LONG, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprToDays), LONG, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprToDays), LONG, DATE)); } /** @@ -975,8 +975,8 @@ private DefaultFunctionResolver to_days() { private DefaultFunctionResolver to_seconds() { return define( BuiltinFunctionName.TO_SECONDS.getName(), - impl(nullMissingHandling(DateTimeFunction::exprToSeconds), LONG, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprToSecondsForIntType), LONG, LONG)); + impl(nullMissingHandling(DateTimeFunctions::exprToSeconds), LONG, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprToSecondsForIntType), LONG, LONG)); } private FunctionResolver unix_timestamp() { @@ -984,11 +984,11 @@ private FunctionResolver unix_timestamp() { BuiltinFunctionName.UNIX_TIMESTAMP.getName(), implWithProperties( functionProperties -> - DateTimeFunction.unixTimeStamp(functionProperties.getQueryStartClock()), + DateTimeFunctions.unixTimeStamp(functionProperties.getQueryStartClock()), LONG), - impl(nullMissingHandling(DateTimeFunction::unixTimeStampOf), DOUBLE, DATE), - impl(nullMissingHandling(DateTimeFunction::unixTimeStampOf), DOUBLE, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::unixTimeStampOf), DOUBLE, DOUBLE)); + impl(nullMissingHandling(DateTimeFunctions::unixTimeStampOf), DOUBLE, DATE), + impl(nullMissingHandling(DateTimeFunctions::unixTimeStampOf), DOUBLE, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::unixTimeStampOf), DOUBLE, DOUBLE)); } /** UTC_DATE(). return the current UTC Date in format yyyy-MM-dd */ @@ -1019,24 +1019,24 @@ private DefaultFunctionResolver week(BuiltinFunctionName week) { implWithProperties( nullMissingHandlingWithProperties( (functionProperties, arg) -> - DateTimeFunction.weekOfYearToday( + DateTimeFunctions.weekOfYearToday( DEFAULT_WEEK_OF_YEAR_MODE, functionProperties.getQueryStartClock())), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprWeekWithoutMode), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprWeekWithoutMode), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprWeekWithoutMode), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprWeekWithoutMode), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprWeekWithoutMode), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprWeekWithoutMode), INTEGER, STRING), implWithProperties( nullMissingHandlingWithProperties( (functionProperties, time, modeArg) -> - DateTimeFunction.weekOfYearToday( + DateTimeFunctions.weekOfYearToday( modeArg, functionProperties.getQueryStartClock())), INTEGER, TIME, INTEGER), - impl(nullMissingHandling(DateTimeFunction::exprWeek), INTEGER, DATE, INTEGER), - impl(nullMissingHandling(DateTimeFunction::exprWeek), INTEGER, TIMESTAMP, INTEGER), - impl(nullMissingHandling(DateTimeFunction::exprWeek), INTEGER, STRING, INTEGER)); + impl(nullMissingHandling(DateTimeFunctions::exprWeek), INTEGER, DATE, INTEGER), + impl(nullMissingHandling(DateTimeFunctions::exprWeek), INTEGER, TIMESTAMP, INTEGER), + impl(nullMissingHandling(DateTimeFunctions::exprWeek), INTEGER, STRING, INTEGER)); } private DefaultFunctionResolver weekday() { @@ -1050,18 +1050,18 @@ private DefaultFunctionResolver weekday() { - 1)), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprWeekday), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprWeekday), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprWeekday), INTEGER, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprWeekday), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprWeekday), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprWeekday), INTEGER, STRING)); } /** YEAR(STRING/DATE/TIMESTAMP). return the year for date (1000-9999). */ private DefaultFunctionResolver year() { return define( BuiltinFunctionName.YEAR.getName(), - impl(nullMissingHandling(DateTimeFunction::exprYear), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprYear), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprYear), INTEGER, STRING)); + impl(nullMissingHandling(DateTimeFunctions::exprYear), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprYear), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprYear), INTEGER, STRING)); } /** YEARWEEK(DATE[,mode]). return the week number for date. */ @@ -1075,9 +1075,9 @@ private DefaultFunctionResolver yearweek() { DEFAULT_WEEK_OF_YEAR_MODE, functionProperties.getQueryStartClock())), INTEGER, TIME), - impl(nullMissingHandling(DateTimeFunction::exprYearweekWithoutMode), INTEGER, DATE), - impl(nullMissingHandling(DateTimeFunction::exprYearweekWithoutMode), INTEGER, TIMESTAMP), - impl(nullMissingHandling(DateTimeFunction::exprYearweekWithoutMode), INTEGER, STRING), + impl(nullMissingHandling(DateTimeFunctions::exprYearweekWithoutMode), INTEGER, DATE), + impl(nullMissingHandling(DateTimeFunctions::exprYearweekWithoutMode), INTEGER, TIMESTAMP), + impl(nullMissingHandling(DateTimeFunctions::exprYearweekWithoutMode), INTEGER, STRING), implWithProperties( nullMissingHandlingWithProperties( (functionProperties, time, modeArg) -> @@ -1085,9 +1085,9 @@ private DefaultFunctionResolver yearweek() { INTEGER, TIME, INTEGER), - impl(nullMissingHandling(DateTimeFunction::exprYearweek), INTEGER, DATE, INTEGER), - impl(nullMissingHandling(DateTimeFunction::exprYearweek), INTEGER, TIMESTAMP, INTEGER), - impl(nullMissingHandling(DateTimeFunction::exprYearweek), INTEGER, STRING, INTEGER)); + impl(nullMissingHandling(DateTimeFunctions::exprYearweek), INTEGER, DATE, INTEGER), + impl(nullMissingHandling(DateTimeFunctions::exprYearweek), INTEGER, TIMESTAMP, INTEGER), + impl(nullMissingHandling(DateTimeFunctions::exprYearweek), INTEGER, STRING, INTEGER)); } /** diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java index 7b6a1d84ee..79ea58b860 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java @@ -24,17 +24,17 @@ import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.expression.Expression; -import org.opensearch.sql.expression.aggregation.AggregatorFunction; -import org.opensearch.sql.expression.datetime.DateTimeFunction; +import org.opensearch.sql.expression.aggregation.AggregatorFunctions; +import org.opensearch.sql.expression.datetime.DateTimeFunctions; import org.opensearch.sql.expression.datetime.IntervalClause; -import org.opensearch.sql.expression.ip.IPFunction; -import org.opensearch.sql.expression.operator.arthmetic.ArithmeticFunction; -import org.opensearch.sql.expression.operator.arthmetic.MathematicalFunction; -import org.opensearch.sql.expression.operator.convert.TypeCastOperator; -import org.opensearch.sql.expression.operator.predicate.BinaryPredicateOperator; -import org.opensearch.sql.expression.operator.predicate.UnaryPredicateOperator; +import org.opensearch.sql.expression.ip.IPFunctions; +import org.opensearch.sql.expression.operator.arthmetic.ArithmeticFunctions; +import org.opensearch.sql.expression.operator.arthmetic.MathematicalFunctions; +import org.opensearch.sql.expression.operator.convert.TypeCastOperators; +import org.opensearch.sql.expression.operator.predicate.BinaryPredicateOperators; +import org.opensearch.sql.expression.operator.predicate.UnaryPredicateOperators; import org.opensearch.sql.expression.system.SystemFunctions; -import org.opensearch.sql.expression.text.TextFunction; +import org.opensearch.sql.expression.text.TextFunctions; import org.opensearch.sql.expression.window.WindowFunctions; import org.opensearch.sql.storage.StorageEngine; @@ -70,19 +70,19 @@ public static synchronized BuiltinFunctionRepository getInstance() { instance = new BuiltinFunctionRepository(new HashMap<>()); // Register all built-in functions - ArithmeticFunction.register(instance); - BinaryPredicateOperator.register(instance); - MathematicalFunction.register(instance); - UnaryPredicateOperator.register(instance); - AggregatorFunction.register(instance); - DateTimeFunction.register(instance); + ArithmeticFunctions.register(instance); + BinaryPredicateOperators.register(instance); + MathematicalFunctions.register(instance); + UnaryPredicateOperators.register(instance); + AggregatorFunctions.register(instance); + DateTimeFunctions.register(instance); IntervalClause.register(instance); WindowFunctions.register(instance); - TextFunction.register(instance); - TypeCastOperator.register(instance); + TextFunctions.register(instance); + TypeCastOperators.register(instance); SystemFunctions.register(instance); OpenSearchFunctions.register(instance); - IPFunction.register(instance); + IPFunctions.register(instance); } return instance; } diff --git a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java similarity index 97% rename from core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java rename to core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java index 855d557916..3aa70a2d18 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java @@ -24,7 +24,7 @@ /** Utility class that defines and registers IP functions. */ @UtilityClass -public class IPFunction { +public class IPFunctions { public void register(BuiltinFunctionRepository repository) { repository.register(cidrmatch()); @@ -35,7 +35,7 @@ private DefaultFunctionResolver cidrmatch() { // TODO #3145: Add support for IP address data type. return define( BuiltinFunctionName.CIDRMATCH.getName(), - impl(nullMissingHandling(IPFunction::exprCidrMatch), BOOLEAN, STRING, STRING)); + impl(nullMissingHandling(IPFunctions::exprCidrMatch), BOOLEAN, STRING, STRING)); } /** diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/ArithmeticFunction.java b/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/ArithmeticFunctions.java similarity index 99% rename from core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/ArithmeticFunction.java rename to core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/ArithmeticFunctions.java index 82b91e1d34..164de6d74c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/ArithmeticFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/ArithmeticFunctions.java @@ -37,7 +37,7 @@ * module, Accepts two numbers and produces a number. */ @UtilityClass -public class ArithmeticFunction { +public class ArithmeticFunctions { /** * Register Arithmetic Function. * diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java b/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctions.java similarity index 99% rename from core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java rename to core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctions.java index 22f4b76573..102834f60d 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctions.java @@ -46,7 +46,7 @@ import org.opensearch.sql.expression.function.SerializableFunction; @UtilityClass -public class MathematicalFunction { +public class MathematicalFunctions { /** * Register Mathematical Functions. * diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java similarity index 99% rename from core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperator.java rename to core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java index db4b29f3b9..55e223d94c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java @@ -42,7 +42,8 @@ import org.opensearch.sql.expression.function.FunctionDSL; @UtilityClass -public class TypeCastOperator { +public class TypeCastOperators { + /** Register Type Cast Operator. */ public static void register(BuiltinFunctionRepository repository) { repository.register(castToString()); diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperators.java similarity index 98% rename from core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java rename to core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperators.java index bf6b3c22f5..96ff7785b7 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperators.java @@ -36,7 +36,7 @@ * equalTo, Compare the left expression and right expression and produces a Boolean. */ @UtilityClass -public class BinaryPredicateOperator { +public class BinaryPredicateOperators { /** * Register Binary Predicate Function. * @@ -401,7 +401,7 @@ private static DefaultFunctionResolver notLike() { BuiltinFunctionName.NOT_LIKE.getName(), impl( nullMissingHandling( - (v1, v2) -> UnaryPredicateOperator.not(OperatorUtils.matches(v1, v2))), + (v1, v2) -> UnaryPredicateOperators.not(OperatorUtils.matches(v1, v2))), BOOLEAN, STRING, STRING)); diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperators.java similarity index 93% rename from core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java rename to core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperators.java index aefb15e999..07bb5b2299 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperators.java @@ -30,7 +30,8 @@ * The definition of unary predicate function not, Accepts one Boolean value and produces a Boolean. */ @UtilityClass -public class UnaryPredicateOperator { +public class UnaryPredicateOperators { + /** Register Unary Predicate Function. */ public static void register(BuiltinFunctionRepository repository) { repository.register(not()); @@ -45,7 +46,7 @@ public static void register(BuiltinFunctionRepository repository) { private static DefaultFunctionResolver not() { return FunctionDSL.define( BuiltinFunctionName.NOT.getName(), - FunctionDSL.impl(UnaryPredicateOperator::not, BOOLEAN, BOOLEAN)); + FunctionDSL.impl(UnaryPredicateOperators::not, BOOLEAN, BOOLEAN)); } /** @@ -108,7 +109,7 @@ private static DefaultFunctionResolver ifFunction() { org.apache.commons.lang3.tuple.Pair>> functionsOne = typeList.stream() - .map(v -> impl((UnaryPredicateOperator::exprIf), v, BOOLEAN, v, v)) + .map(v -> impl((UnaryPredicateOperators::exprIf), v, BOOLEAN, v, v)) .collect(Collectors.toList()); return FunctionDSL.define(functionName, functionsOne); @@ -124,7 +125,7 @@ private static DefaultFunctionResolver ifNull() { org.apache.commons.lang3.tuple.Pair>> functionsOne = typeList.stream() - .map(v -> impl((UnaryPredicateOperator::exprIfNull), v, v, v)) + .map(v -> impl((UnaryPredicateOperators::exprIfNull), v, v, v)) .collect(Collectors.toList()); return FunctionDSL.define(functionName, functionsOne); @@ -137,7 +138,7 @@ private static DefaultFunctionResolver nullIf() { return FunctionDSL.define( functionName, typeList.stream() - .map(v -> impl((UnaryPredicateOperator::exprNullIf), v, v, v)) + .map(v -> impl((UnaryPredicateOperators::exprNullIf), v, v, v)) .collect(Collectors.toList())); } diff --git a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java b/core/src/main/java/org/opensearch/sql/expression/text/TextFunctions.java similarity index 95% rename from core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java rename to core/src/main/java/org/opensearch/sql/expression/text/TextFunctions.java index d670843551..a6eaa7ab74 100644 --- a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/text/TextFunctions.java @@ -38,7 +38,7 @@ * implementation should rely on ExprValue. */ @UtilityClass -public class TextFunction { +public class TextFunctions { private static String EMPTY_STRING = ""; /** @@ -76,9 +76,9 @@ public void register(BuiltinFunctionRepository repository) { private DefaultFunctionResolver substringSubstr(FunctionName functionName) { return define( functionName, - impl(nullMissingHandling(TextFunction::exprSubstrStart), STRING, STRING, INTEGER), + impl(nullMissingHandling(TextFunctions::exprSubstrStart), STRING, STRING, INTEGER), impl( - nullMissingHandling(TextFunction::exprSubstrStartLength), + nullMissingHandling(TextFunctions::exprSubstrStartLength), STRING, STRING, INTEGER, @@ -267,7 +267,7 @@ private DefaultFunctionResolver strcmp() { private DefaultFunctionResolver right() { return define( BuiltinFunctionName.RIGHT.getName(), - impl(nullMissingHandling(TextFunction::exprRight), STRING, STRING, INTEGER)); + impl(nullMissingHandling(TextFunctions::exprRight), STRING, STRING, INTEGER)); } /** @@ -279,7 +279,7 @@ private DefaultFunctionResolver right() { private DefaultFunctionResolver left() { return define( BuiltinFunctionName.LEFT.getName(), - impl(nullMissingHandling(TextFunction::exprLeft), STRING, STRING, INTEGER)); + impl(nullMissingHandling(TextFunctions::exprLeft), STRING, STRING, INTEGER)); } /** @@ -292,7 +292,7 @@ private DefaultFunctionResolver left() { private DefaultFunctionResolver ascii() { return define( BuiltinFunctionName.ASCII.getName(), - impl(nullMissingHandling(TextFunction::exprAscii), INTEGER, STRING)); + impl(nullMissingHandling(TextFunctions::exprAscii), INTEGER, STRING)); } /** @@ -310,14 +310,14 @@ private DefaultFunctionResolver locate() { BuiltinFunctionName.LOCATE.getName(), impl( nullMissingHandling( - (SerializableBiFunction) TextFunction::exprLocate), + (SerializableBiFunction) TextFunctions::exprLocate), INTEGER, STRING, STRING), impl( nullMissingHandling( (SerializableTriFunction) - TextFunction::exprLocate), + TextFunctions::exprLocate), INTEGER, STRING, STRING, @@ -337,7 +337,7 @@ private DefaultFunctionResolver position() { BuiltinFunctionName.POSITION.getName(), impl( nullMissingHandling( - (SerializableBiFunction) TextFunction::exprLocate), + (SerializableBiFunction) TextFunctions::exprLocate), INTEGER, STRING, STRING)); @@ -353,7 +353,7 @@ private DefaultFunctionResolver position() { private DefaultFunctionResolver replace() { return define( BuiltinFunctionName.REPLACE.getName(), - impl(nullMissingHandling(TextFunction::exprReplace), STRING, STRING, STRING, STRING)); + impl(nullMissingHandling(TextFunctions::exprReplace), STRING, STRING, STRING, STRING)); } /** @@ -365,7 +365,7 @@ private DefaultFunctionResolver replace() { private DefaultFunctionResolver reverse() { return define( BuiltinFunctionName.REVERSE.getName(), - impl(nullMissingHandling(TextFunction::exprReverse), STRING, STRING)); + impl(nullMissingHandling(TextFunctions::exprReverse), STRING, STRING)); } private static ExprValue exprSubstrStart(ExprValue exprValue, ExprValue start) { diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/FilterOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/FilterOperator.java index ec61d53163..192ea5cb4f 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/FilterOperator.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/FilterOperator.java @@ -13,13 +13,13 @@ import lombok.ToString; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.expression.Expression; -import org.opensearch.sql.expression.operator.predicate.BinaryPredicateOperator; +import org.opensearch.sql.expression.operator.predicate.BinaryPredicateOperators; import org.opensearch.sql.storage.bindingtuple.BindingTuple; /** * The Filter operator represents WHERE clause and uses the conditions to evaluate the input {@link * BindingTuple}. The Filter operator only returns the results that evaluated to true. The NULL and - * MISSING are handled by the logic defined in {@link BinaryPredicateOperator}. + * MISSING are handled by the logic defined in {@link BinaryPredicateOperators}. */ @EqualsAndHashCode(callSuper = false) @ToString diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ToSecondsTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ToSecondsTest.java index 910fe42a52..e983eb28f6 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ToSecondsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ToSecondsTest.java @@ -8,7 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.opensearch.sql.data.type.ExprCoreType.LONG; -import static org.opensearch.sql.expression.datetime.DateTimeFunction.SECONDS_PER_DAY; +import static org.opensearch.sql.expression.datetime.DateTimeFunctions.SECONDS_PER_DAY; import java.time.Duration; import java.time.LocalDate; diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java index f7a1a7008a..7de4f456c9 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java @@ -221,12 +221,12 @@ public void test_if_predicate(Expression v1, Expression v2, Expression v3, Expre @ParameterizedTest @MethodSource("exprIfNullArguments") public void test_exprIfNull_predicate(ExprValue v1, ExprValue v2, ExprValue expected) { - assertEquals(expected.value(), UnaryPredicateOperator.exprIfNull(v1, v2).value()); + assertEquals(expected.value(), UnaryPredicateOperators.exprIfNull(v1, v2).value()); } @ParameterizedTest @MethodSource("exprNullIfArguments") public void test_exprNullIf_predicate(ExprValue v1, ExprValue v2, ExprValue expected) { - assertEquals(expected.value(), UnaryPredicateOperator.exprNullIf(v1, v2).value()); + assertEquals(expected.value(), UnaryPredicateOperators.exprNullIf(v1, v2).value()); } } From c2167638d74f46555820db376fb6b920bc65ac1d Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 2 Dec 2024 12:47:21 -0800 Subject: [PATCH 39/40] Minor documentation updates Signed-off-by: currantw --- .../org/opensearch/sql/expression/ip/IPFunctions.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 3aa70a2d18..e1c295a404 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 @@ -39,11 +39,11 @@ private DefaultFunctionResolver cidrmatch() { } /** - * Returns whether the given IP address is within the specified CIDR IP address range. Supports - * both IPv4 and IPv6 addresses. + * 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 (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @param rangeExprValue IP address range in CIDR notation (e.g. "198.51.100.0/24" or + * @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 * "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 @@ -51,6 +51,7 @@ 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(); From 1b608c62bc9a712a1ef08a098ffdc131fce8292a Mon Sep 17 00:00:00 2001 From: currantw Date: Mon, 2 Dec 2024 12:56:34 -0800 Subject: [PATCH 40/40] Apply spotless Signed-off-by: currantw --- .../sql/expression/datetime/DateTimeFunctions.java | 5 ++++- .../java/org/opensearch/sql/expression/ip/IPFunctions.java | 3 ++- .../org/opensearch/sql/expression/text/TextFunctions.java | 6 ++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunctions.java b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunctions.java index 2d9dec15f5..411bd27993 100644 --- a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunctions.java @@ -615,7 +615,10 @@ private FunctionResolver from_unixtime() { BuiltinFunctionName.FROM_UNIXTIME.getName(), impl(nullMissingHandling(DateTimeFunctions::exprFromUnixTime), TIMESTAMP, DOUBLE), impl( - nullMissingHandling(DateTimeFunctions::exprFromUnixTimeFormat), STRING, DOUBLE, STRING)); + nullMissingHandling(DateTimeFunctions::exprFromUnixTimeFormat), + STRING, + DOUBLE, + STRING)); } private DefaultFunctionResolver get_format() { 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 e1c295a404..b3e7fad211 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 @@ -42,7 +42,8 @@ private DefaultFunctionResolver cidrmatch() { * 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 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 * "2001:0db8::/32") * @return true if the address is in the range; otherwise false. diff --git a/core/src/main/java/org/opensearch/sql/expression/text/TextFunctions.java b/core/src/main/java/org/opensearch/sql/expression/text/TextFunctions.java index a6eaa7ab74..8a5302070c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/text/TextFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/text/TextFunctions.java @@ -310,7 +310,8 @@ private DefaultFunctionResolver locate() { BuiltinFunctionName.LOCATE.getName(), impl( nullMissingHandling( - (SerializableBiFunction) TextFunctions::exprLocate), + (SerializableBiFunction) + TextFunctions::exprLocate), INTEGER, STRING, STRING), @@ -337,7 +338,8 @@ private DefaultFunctionResolver position() { BuiltinFunctionName.POSITION.getName(), impl( nullMissingHandling( - (SerializableBiFunction) TextFunctions::exprLocate), + (SerializableBiFunction) + TextFunctions::exprLocate), INTEGER, STRING, STRING));