Update comparison logic to compare in IPv6

Signed-off-by: currantw <[email protected]>
opensearch-project · Dec 2, 2024 · ba31d34 · ba31d34
1 parent e4fb3e7
commit ba31d34
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 29 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
@@ -9,6 +9,8 @@
 import inet.ipaddr.IPAddress;
 import inet.ipaddr.IPAddressString;
 import inet.ipaddr.IPAddressStringParameters;
+import inet.ipaddr.ipv4.IPv4Address;
+import inet.ipaddr.ipv6.IPv6Address;
 import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
 import org.opensearch.sql.exception.SemanticCheckException;
@@ -28,13 +30,7 @@ public class ExprIpValue extends AbstractExprValue {
           .toParams();
 
   public ExprIpValue(String s) {
-    try {
-      IPAddress address = new IPAddressString(s, validationOptions).toAddress();
-      value = address.isIPv4Convertible() ? address.toIPv4() : address;
-    } catch (AddressStringException e) {
-      final String errorFormatString = "IP address '%s' is not valid. Error details: %s";
-      throw new SemanticCheckException(String.format(errorFormatString, s, e.getMessage()));
-    }
+    value = stringToIpAddress(s);
   }
 
   @Override
@@ -49,18 +45,43 @@ public ExprType type() {
 
   @Override
   public int compare(ExprValue other) {
-    IPAddress otherValue = ((ExprIpValue) other).value;
-    return value.compareTo(otherValue);
+    IPAddress otherValue =
+        other instanceof ExprIpValue exprIpValue
+            ? exprIpValue.value
+            : stringToIpAddress(other.stringValue());
+
+    // Map IPv4 addresses to IPv6 for comparison
+    IPv6Address ipv6Value = toIPv6Address(value);
+    IPv6Address otherIpv6Value = toIPv6Address(otherValue);
+
+    return ipv6Value.compareTo(otherIpv6Value);
   }
 
   @Override
   public boolean equal(ExprValue other) {
-    IPAddress otherValue = ((ExprIpValue) other).value;
-    return value.equals(otherValue);
+    return compare(other) == 0;
   }
 
   @Override
   public String toString() {
     return String.format("IP %s", value());
   }
+
+  /** Returns the {@link IPAddress} corresponding to the given {@link String}. */
+  private static IPAddress stringToIpAddress(String s) {
+    try {
+      IPAddress address = new IPAddressString(s, validationOptions).toAddress();
+      return address.isIPv4Convertible() ? address.toIPv4() : address;
+    } catch (AddressStringException e) {
+      final String errorFormatString = "IP address '%s' is not valid. Error details: %s";
+      throw new SemanticCheckException(String.format(errorFormatString, s, e.getMessage()));
+    }
+  }
+
+  /** Returns the {@link IPv6Address} corresponding to the given {@link IPAddress}. */
+  private static IPv6Address toIPv6Address(IPAddress ipAddress) {
+    return ipAddress instanceof IPv4Address iPv4Address
+        ? iPv4Address.toIPv6()
+        : (IPv6Address) ipAddress;
+  }
 }
diff --git a/...test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java b/...test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java
@@ -1725,14 +1725,15 @@ void cast_to_boolean_false_in_filter(LiteralExpression expr) {
   void cast_to_ip_in_filter(LiteralExpression expr) {
     String json =
         String.format(
-            "{\n"
-                + "  \"term\" : {\n"
-                + "    \"ip_value\" : {\n"
-                + "      \"value\" : \"%s\",\n"
-                + "      \"boost\" : 1.0\n"
-                + "    }\n"
-                + "  }\n"
-                + "}",
+            """
+                        {
+                          "term" : {
+                            "ip_value" : {
+                              "value" : "%s",
+                              "boost" : 1.0
+                            }
+                          }
+                        }""",
             expr.valueOf().stringValue());
 
     assertJsonEquals(json, buildQuery(DSL.equal(ref("ip_value", IP), DSL.castIp(expr))));
@@ -1906,15 +1907,16 @@ void non_literal_in_cast_should_build_script() {
   void non_cast_nested_function_should_build_script() {
     mockToStringSerializer();
     assertJsonEquals(
-        "{\n"
-            + "  \"script\" : {\n"
-            + "    \"script\" : {\n"
-            + "      \"source\" : \"=(integer_value, abs(+(1, 0)))\",\n"
-            + "      \"lang\" : \"opensearch_query_expression\"\n"
-            + "    },\n"
-            + "    \"boost\" : 1.0\n"
-            + "  }\n"
-            + "}",
+        """
+                    {
+                      "script" : {
+                        "script" : {
+                          "source" : "=(integer_value, abs(+(1, 0)))",
+                          "lang" : "opensearch_query_expression"
+                        },
+                        "boost" : 1.0
+                      }
+                    }""",
         buildQuery(
             DSL.equal(ref("integer_value", INTEGER), DSL.abs(DSL.add(literal(1), literal(0))))));
   }

diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/utils/ArgumentFactoryTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/utils/ArgumentFactoryTest.java
@@ -81,7 +81,7 @@ public void testDedupCommandDefaultArgument() {
   }
 
   @Test
-  public void testSortCommandDefaultArgument() {
+  public void testSortCommand() {
     assertEqual(
         "source=t | sort field0",
         sort(relation("t"), field("field0", exprList(argument("asc", booleanLiteral(true))))));