From ac9c30080d29833d6f4fd10cac23a73bb7d4af28 Mon Sep 17 00:00:00 2001 From: currantw Date: Tue, 17 Dec 2024 10:47:38 -0800 Subject: [PATCH] Address minor review comments. Signed-off-by: currantw --- .../org/opensearch/sql/data/model/ExprIpValue.java | 4 ++-- .../opensearch/sql/expression/ip/IPFunctions.java | 13 ++++--------- .../operator/convert/TypeCastOperators.java | 4 ++-- docs/user/ppl/cmd/trendline.rst | 2 +- .../data/value/OpenSearchExprValueFactoryTest.java | 5 +---- .../script/filter/lucene/RangeQueryTest.java | 4 ++-- 6 files changed, 12 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java index 7ca2ffe92f..8bdbec4bb5 100644 --- a/core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java +++ b/core/src/main/java/org/opensearch/sql/data/model/ExprIpValue.java @@ -14,8 +14,8 @@ public class ExprIpValue extends AbstractExprValue { private final IPAddress value; - public ExprIpValue(String s) { - value = IPUtils.toAddress(s); + public ExprIpValue(String addressString) { + value = IPUtils.toAddress(addressString); } @Override 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 e42c12841a..8b3ee23014 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/ip/IPFunctions.java @@ -52,14 +52,9 @@ private ExprValue exprCidrMatch(ExprValue addressExprValue, ExprValue rangeExprV IPAddress address = addressExprValue.ipValue(); IPAddress range = IPUtils.toRange(rangeExprValue.stringValue()); - if (IPUtils.compare(address, range.getLower()) < 0) { - return ExprValueUtils.LITERAL_FALSE; - } - - if (IPUtils.compare(address, range.getUpper()) > 0) { - return ExprValueUtils.LITERAL_FALSE; - } - - return ExprValueUtils.LITERAL_TRUE; + return (IPUtils.compare(address, range.getLower()) < 0) + || (IPUtils.compare(address, range.getUpper()) > 0) + ? ExprValueUtils.LITERAL_FALSE + : ExprValueUtils.LITERAL_TRUE; } } diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java b/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java index 696d02dc0f..b388f7d89a 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperators.java @@ -179,8 +179,8 @@ private static DefaultFunctionResolver castToBoolean() { private static DefaultFunctionResolver castToIp() { return FunctionDSL.define( BuiltinFunctionName.CAST_TO_IP.getName(), - impl(nullMissingHandling((v) -> v), IP, IP), - impl(nullMissingHandling((v) -> new ExprIpValue(v.stringValue())), IP, STRING)); + impl(nullMissingHandling((v) -> new ExprIpValue(v.stringValue())), IP, STRING), + impl(nullMissingHandling((v) -> v), IP, IP)); } private static DefaultFunctionResolver castToDate() { diff --git a/docs/user/ppl/cmd/trendline.rst b/docs/user/ppl/cmd/trendline.rst index 166a3c056f..e6df0d7a2c 100644 --- a/docs/user/ppl/cmd/trendline.rst +++ b/docs/user/ppl/cmd/trendline.rst @@ -23,7 +23,7 @@ Syntax * field: mandatory. The name of the field the moving average should be calculated for. * alias: optional. The name of the resulting column containing the moving average (defaults to the field name with "_trendline"). -And the moment only the Simple Moving Average (SMA) type is supported. +At the moment only the Simple Moving Average (SMA) type is supported. It is calculated like diff --git a/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 ad2f5988b9..5b9166e2b5 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 @@ -120,10 +120,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); @@ -761,8 +760,6 @@ public void constructIP() { tupleValue(String.format("{\"%s\":\"%s\"}", fieldIp, ipString)).get(fieldIp)); } - private static final double TOLERANCE = 1E-5; - @Test public void constructGeoPoint() { final double lat = 42.60355556; diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/RangeQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/RangeQueryTest.java index 55272d4cd7..2f5482171d 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/RangeQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/RangeQueryTest.java @@ -85,8 +85,8 @@ void test_date_has_format() { } @Test - void test_string_field_type() { - String dateString = "STRING"; + void test_non_date_field_type() { + String dateString = "2021-11-08"; OpenSearchDateType dateType = OpenSearchDateType.of(STRING); ExprValue literal = ExprValueUtils.stringValue(dateString); assertNotNull(new RangeQuery(Comparison.LT).doBuild("string_value", dateType, literal));