diff --git a/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonUtils.java b/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonUtils.java index da8dc2a2413..16727295fea 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/jsonUDF/JsonUtils.java +++ b/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()) { diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java index 8159fd6c115..5916205e0f4 100644 --- a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java +++ b/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}]"; diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLJsonBuiltinFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLJsonBuiltinFunctionIT.java index 0ec367aa318..99af10302ae 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLJsonBuiltinFunctionIT.java +++ b/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 = diff --git a/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5167.yml b/integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/5167.yml new file mode 100644 index 00000000000..cf18c4c425c --- /dev/null +++ b/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]}" ] ] }