Merge remote-tracking branch 'upstream/main' into issues/5070

Author: LantaoJin

core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java

@@ -133,7 +133,7 @@ public static ExprValue fromObjectValue(Object o) {
     } else if (o instanceof Boolean) {
       return booleanValue((Boolean) o);
     } else if (o instanceof Double d) {
-      if (Double.isNaN(d)) {
+      if (!Double.isFinite(d)) {
         return LITERAL_NULL;
       }
       return doubleValue(d);
@@ -144,7 +144,7 @@ public static ExprValue fromObjectValue(Object o) {
     } else if (o instanceof String) {
       return stringValue((String) o);
     } else if (o instanceof Float f) {
-      if (Float.isNaN(f)) {
+      if (!Float.isFinite(f)) {
         return LITERAL_NULL;
       }
       return floatValue(f);

core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractFunctionImpl.java

@@ -94,7 +94,7 @@ private static boolean isScalarObject(Object obj) {
 
   private static String doJsonize(Object candidate) {
     if (candidate == null) {
-      return "null"; // Matches isScalarObject, but not toString-able.
+      return null;
     } else if (isScalarObject(candidate)) {
       return candidate.toString();
     } else {

core/src/test/java/org/opensearch/sql/data/model/ExprValueUtilsTest.java

@@ -9,6 +9,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.opensearch.sql.data.model.ExprValueUtils.integerValue;
 import static org.opensearch.sql.data.type.ExprCoreType.ARRAY;
 import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN;
@@ -243,6 +244,28 @@ public void constructDateAndTimeValue() {
         ExprValueUtils.fromObjectValue("2012-07-07 01:01:01", TIMESTAMP));
   }
 
+  @Test
+  public void fromObjectValue_double_infinity_returns_null() {
+    assertTrue(ExprValueUtils.fromObjectValue(Double.POSITIVE_INFINITY).isNull());
+    assertTrue(ExprValueUtils.fromObjectValue(Double.NEGATIVE_INFINITY).isNull());
+  }
+
+  @Test
+  public void fromObjectValue_float_infinity_returns_null() {
+    assertTrue(ExprValueUtils.fromObjectValue(Float.POSITIVE_INFINITY).isNull());
+    assertTrue(ExprValueUtils.fromObjectValue(Float.NEGATIVE_INFINITY).isNull());
+  }
+
+  @Test
+  public void fromObjectValue_double_nan_returns_null() {
+    assertTrue(ExprValueUtils.fromObjectValue(Double.NaN).isNull());
+  }
+
+  @Test
+  public void fromObjectValue_float_nan_returns_null() {
+    assertTrue(ExprValueUtils.fromObjectValue(Float.NaN).isNull());
+  }
+
   @Test
   public void hashCodeTest() {
     assertEquals(new ExprByteValue(1).hashCode(), new ExprByteValue(1).hashCode());

core/src/test/java/org/opensearch/sql/expression/function/jsonUDF/JsonExtractFunctionImplTest.java

@@ -0,0 +1,78 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.expression.function.jsonUDF;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import org.junit.jupiter.api.Test;
+
+public class JsonExtractFunctionImplTest {
+
+  @Test
+  public void testMissingPathReturnsNull() {
+    Object result = JsonExtractFunctionImpl.eval("{}", "name");
+    assertNull(result, "Missing path should return actual null, not string \"null\"");
+  }
+
+  @Test
+  public void testExplicitNullValueReturnsNull() {
+    Object result = JsonExtractFunctionImpl.eval("{\"name\": null}", "name");
+    assertNull(result, "Explicit JSON null value should return actual null, not string \"null\"");
+  }
+
+  @Test
+  public void testNoArgsReturnsNull() {
+    assertNull(JsonExtractFunctionImpl.eval());
+  }
+
+  @Test
+  public void testSingleArgReturnsNull() {
+    assertNull(JsonExtractFunctionImpl.eval("{}"));
+  }
+
+  @Test
+  public void testExtractStringValue() {
+    Object result = JsonExtractFunctionImpl.eval("{\"name\": \"John\"}", "name");
+    assertEquals("John", result);
+  }
+
+  @Test
+  public void testExtractNumericValue() {
+    Object result = JsonExtractFunctionImpl.eval("{\"age\": 30}", "age");
+    assertEquals("30", result);
+  }
+
+  @Test
+  public void testExtractNestedObject() {
+    Object result = JsonExtractFunctionImpl.eval("{\"user\": {\"name\": \"John\"}}", "user");
+    assertEquals("{\"name\":\"John\"}", result);
+  }
+
+  @Test
+  public void testExtractArray() {
+    Object result = JsonExtractFunctionImpl.eval("{\"items\": [1, 2, 3]}", "items");
+    assertEquals("[1,2,3]", result);
+  }
+
+  @Test
+  public void testExtractBooleanValue() {
+    Object result = JsonExtractFunctionImpl.eval("{\"active\": true}", "active");
+    assertEquals("true", result);
+  }
+
+  @Test
+  public void testMultiPathWithMissingPath() {
+    Object result = JsonExtractFunctionImpl.eval("{\"name\": \"John\"}", "name", "age");
+    assertEquals("[\"John\",null]", result);
+  }
+
+  @Test
+  public void testMultiPathAllMissing() {
+    Object result = JsonExtractFunctionImpl.eval("{}", "name", "age");
+    assertEquals("[null,null]", result);
+  }
+}

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLJsonBuiltinFunctionIT.java

@@ -194,6 +194,36 @@ public void testJsonExtractWithMultiplyResult() throws IOException {
     verifyDataRows(actual, rows("[[\"Bridge of Sighs\",\"Ponte della Paglia\"],8981.0]"));
   }
 
+  @Test
+  public void testJsonExtractReturnsNullForMissingPathAndNullValue() throws IOException {
+    JSONObject actual =
+        executeQuery(
+            String.format(
+                "source=%s | head 1 | eval a = json_extract('{}', 'name'),"
+                    + " b = json_extract('{\\\"name\\\": null}', 'name')"
+                    + " | fields a, b | head 1",
+                TEST_INDEX_PEOPLE2));
+
+    verifySchema(actual, schema("a", "string"), schema("b", "string"));
+
+    verifyDataRows(actual, rows(null, null));
+  }
+
+  @Test
+  public void testJsonExtractMultiPathWithMissingPath() throws IOException {
+    JSONObject actual =
+        executeQuery(
+            String.format(
+                "source=%s | head 1 | eval a = json_extract('{\\\"name\\\": \\\"John\\\"}',"
+                    + " 'name', 'age')"
+                    + " | fields a | head 1",
+                TEST_INDEX_PEOPLE2));
+
+    verifySchema(actual, schema("a", "string"));
+
+    verifyDataRows(actual, rows("[\"John\",null]"));
+  }
+
   @Test
   public void testJsonKeys() throws IOException {
     JSONObject actual =

integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5163.yml

@@ -0,0 +1,49 @@
+setup:
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled: true
+  - do:
+      indices.create:
+        index: bounty-types
+        body:
+          mappings:
+            properties:
+              "int_field":
+                type: integer
+              "double_field":
+                type: double
+  - do:
+      bulk:
+        index: bounty-types
+        refresh: true
+        body:
+          - '{"index": {"_id": "1"}}'
+          - '{"int_field": 2147483647, "double_field": 1.7976931348623157E308}'
+
+---
+teardown:
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled: false
+
+---
+"Double overflow to Infinity returns null instead of crashing":
+  - skip:
+      features:
+        - headers
+        - allowed_warnings
+  - do:
+      allowed_warnings:
+        - 'Loading the fielddata on the _id field is deprecated and will be removed in future versions. If you require sorting or aggregating on this field you should also include the id in the body of your documents, and map this field as a keyword field that has [doc_values] enabled'
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: source=bounty-types | where int_field = 2147483647 | eval double_mul = double_field * 2 | fields double_field, double_mul
+
+  - match: { total: 1 }
+  - match: { "datarows": [[1.7976931348623157E308, null]] }

integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5176.yml

@@ -0,0 +1,85 @@
+setup:
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled: true
+
+  - do:
+      indices.create:
+        index: issue5176
+        body:
+          settings:
+            number_of_shards: 1
+            number_of_replicas: 0
+          mappings:
+            properties:
+              id:
+                type: integer
+
+  - do:
+      bulk:
+        refresh: true
+        body:
+          - '{"index": {"_index": "issue5176", "_id": "1"}}'
+          - '{"id": 1}'
+
+---
+teardown:
+  - do:
+      indices.delete:
+        index: issue5176
+        ignore_unavailable: true
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled: false
+
+---
+"Issue 5176: json_extract returns null for missing path":
+  - skip:
+      features:
+        - headers
+  - do:
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: source=issue5176 | eval a = json_extract('{}', 'name') | fields a
+
+  - match: { total: 1 }
+  - match: { schema: [ { name: a, type: string } ] }
+  - match: { datarows: [ [ null ] ] }
+
+---
+"Issue 5176: json_extract returns null for explicit JSON null value":
+  - skip:
+      features:
+        - headers
+  - do:
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: "source=issue5176 | eval a = json_extract('{\"name\": null}', 'name') | fields a"
+
+  - match: { total: 1 }
+  - match: { schema: [ { name: a, type: string } ] }
+  - match: { datarows: [ [ null ] ] }
+
+---
+"Issue 5176: json_extract multi-path with missing path returns array with null":
+  - skip:
+      features:
+        - headers
+  - do:
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: "source=issue5176 | eval a = json_extract('{\"name\": \"John\"}', 'name', 'age') | fields a"
+
+  - match: { total: 1 }
+  - match: { schema: [ { name: a, type: string } ] }
+  - match: { datarows: [ [ "[\"John\",null]" ] ] }