[BugFix] Fix json_set crash and json_delete no-op with $.key paths (#5167) (#5339)

convertToJsonPath() unconditionally prepends "$." to input paths.
When users pass standard JSONPath syntax like "$.name", the path becomes
"$.$.name" (double-prefixed), causing json_set to crash with HTTP 500
and json_delete to silently return unchanged JSON.

Strip any existing "$." or "$" prefix before processing to prevent
double-prefixing while preserving the curly-brace-to-bracket conversion.

Signed-off-by: Heng Qian <qianheng@amazon.com>

Author: qianheng-aws

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

@@ -23,6 +23,15 @@ public class JsonUtils {
   public static String convertToJsonPath(String input) {
     if (input == null || input.isEmpty()) return "$";
 
+    // Strip leading "$." or "$" to avoid double-prefixing (issue #5167)
+    if (input.startsWith("$.")) {
+      input = input.substring(2);
+    } else if (input.startsWith("$")) {
+      input = input.substring(1);
+    }
+
+    if (input.isEmpty()) return "$";
+
     StringBuilder sb = new StringBuilder("$.");
     int i = 0;
     while (i < input.length()) {

core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java

@@ -19,6 +19,8 @@
 import org.opensearch.sql.exception.SemanticCheckException;
 import org.opensearch.sql.expression.DSL;
 import org.opensearch.sql.expression.LiteralExpression;
+import org.opensearch.sql.expression.function.jsonUDF.JsonDeleteFunctionImpl;
+import org.opensearch.sql.expression.function.jsonUDF.JsonSetFunctionImpl;
 import org.opensearch.sql.expression.function.jsonUDF.JsonUtils;
 
 @ExtendWith(MockitoExtension.class)
@@ -65,6 +67,18 @@ void test_convertToJsonPath() {
     assertEquals(targetJsonPath, convertedJsonPath);
   }
 
+  @Test
+  void test_convertToJsonPathWithDollarPrefix() {
+    // Issue #5167: paths already starting with $ or $. should not be double-prefixed
+    assertEquals("$.name", convertToJsonPath("$.name"));
+    assertEquals("$.a.b.c", convertToJsonPath("$.a.b.c"));
+    assertEquals("$.[*]", convertToJsonPath("$.[*]"));
+    assertEquals("$.a[2].c", convertToJsonPath("$.a[2].c"));
+    assertEquals("$.[3].bc[*].d[1]", convertToJsonPath("$.[3].bc[*].d[1]"));
+    // Bare $ should return $
+    assertEquals("$", convertToJsonPath("$"));
+  }
+
   @Test
   void test_convertToJsonPathWithWrongPath() {
     IllegalArgumentException e =
@@ -100,6 +114,23 @@ void test_jsonPathExpand() {
     assertEquals(expandJsonPath(node, candidate4), target4);
   }
 
+  @Test
+  void test_jsonSetWithDollarPrefixedPath() {
+    // Issue #5167: json_set with $.key path should work correctly
+    Object result =
+        JsonSetFunctionImpl.eval(
+            "{\"name\":\"alice\",\"scores\":[90,85,92]}", "$.name", "modified_alice");
+    assertEquals("{\"name\":\"modified_alice\",\"scores\":[90,85,92]}", result);
+  }
+
+  @Test
+  void test_jsonDeleteWithDollarPrefixedPath() throws Exception {
+    // Issue #5167: json_delete with $.key path should remove the key
+    Object result =
+        JsonDeleteFunctionImpl.eval("{\"name\":\"alice\",\"scores\":[90,85,92]}", "$.name");
+    assertEquals("{\"scores\":[90,85,92]}", result);
+  }
+
   @Test
   void test_jsonPathExpandAtArray() {
     String jsonStr = "[{\"c\": 1}, {\"c\": 1}, {\"c\": 1}]";

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

@@ -296,6 +296,38 @@ public void testJsonSetPartialSet() throws IOException {
     verifyDataRows(actual, rows("{\"a\":[{\"b\":1},{\"b\":{\"c\":\"3\"}}]}"));
   }
 
+  @Test
+  public void testJsonSetWithDollarPrefixedPath() throws IOException {
+    // Issue #5167: json_set with $.key path should not double-prefix
+    JSONObject actual =
+        executeQuery(
+            String.format(
+                "source=%s | eval a"
+                    + " =json_set('{\\\"name\\\":\\\"alice\\\",\\\"scores\\\":[90,85,92]}',"
+                    + " '$.name', 'modified_alice')| fields a | head 1",
+                TEST_INDEX_PEOPLE2));
+
+    verifySchema(actual, schema("a", "string"));
+
+    verifyDataRows(actual, rows("{\"name\":\"modified_alice\",\"scores\":[90,85,92]}"));
+  }
+
+  @Test
+  public void testJsonDeleteWithDollarPrefixedPath() throws IOException {
+    // Issue #5167: json_delete with $.key path should remove the key
+    JSONObject actual =
+        executeQuery(
+            String.format(
+                "source=%s | eval a"
+                    + " =json_delete('{\\\"name\\\":\\\"alice\\\",\\\"scores\\\":[90,85,92]}',"
+                    + " '$.name')| fields a | head 1",
+                TEST_INDEX_PEOPLE2));
+
+    verifySchema(actual, schema("a", "string"));
+
+    verifyDataRows(actual, rows("{\"scores\":[90,85,92]}"));
+  }
+
   @Test
   public void testJsonDelete() throws IOException {
     JSONObject actual =

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

@@ -0,0 +1,84 @@
+setup:
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled: true
+
+  - do:
+      indices.create:
+        index: issue5167
+        body:
+          settings:
+            number_of_shards: 1
+            number_of_replicas: 0
+          mappings:
+            properties:
+              int_field:
+                type: integer
+              json_data:
+                type: keyword
+
+  - do:
+      bulk:
+        refresh: true
+        body:
+          - '{"index": {"_index": "issue5167", "_id": "1"}}'
+          - '{"int_field": 42, "json_data": "{\"name\":\"alice\",\"scores\":[90,85,92]}"}'
+
+---
+teardown:
+  - do:
+      indices.delete:
+        index: issue5167
+        ignore_unavailable: true
+  - do:
+      query.settings:
+        body:
+          transient:
+            plugins.calcite.enabled: false
+
+---
+"Issue 5167: json_set with $.key path should update the value":
+  - skip:
+      features:
+        - headers
+  - do:
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: "source=issue5167 | where int_field = 42 | eval modified = json_set(json_data, '$.name', 'modified_alice') | fields modified"
+
+  - match: { total: 1 }
+  - match: { datarows: [ [ "{\"name\":\"modified_alice\",\"scores\":[90,85,92]}" ] ] }
+
+---
+"Issue 5167: json_delete with $.key path should remove the key":
+  - skip:
+      features:
+        - headers
+  - do:
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: "source=issue5167 | where int_field = 42 | eval deleted = json_delete(json_data, '$.name') | fields deleted"
+
+  - match: { total: 1 }
+  - match: { datarows: [ [ "{\"scores\":[90,85,92]}" ] ] }
+
+---
+"Issue 5167: json_set with unprefixed path still works":
+  - skip:
+      features:
+        - headers
+  - do:
+      headers:
+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: "source=issue5167 | where int_field = 42 | eval modified = json_set(json_data, 'name', 'bob') | fields modified"
+
+  - match: { total: 1 }
+  - match: { datarows: [ [ "{\"name\":\"bob\",\"scores\":[90,85,92]}" ] ] }