Skip to content

Commit ce68b0c

Browse files
Fix #5176: Return actual null from JSON_EXTRACT for missing/null paths (#5196)
* Fix #5176: Return actual null from JSON_EXTRACT for missing/null paths The doJsonize() method in JsonExtractFunctionImpl was explicitly converting null values to the string "null". When JSON_EXTRACT encounters a missing path or an explicit JSON null value, it should return actual null instead of the string "null". Signed-off-by: opensearchpplteam <opensearchpplteam@gmail.com> * Add integration and unit tests for JSON_EXTRACT null handling (#5176) Address review feedback: - Add integration test for null-return behavior (missing path and explicit null) - Add integration test for multi-path extraction with missing path - Add unit tests for multi-path extraction where some/all paths are missing Signed-off-by: opensearchpplteam <opensearchpplteam@gmail.com> * Fix spotless formatting in JsonExtractFunctionImplTest Signed-off-by: opensearchpplteam <opensearchpplteam@gmail.com> * Add YAML REST tests for JSON_EXTRACT null handling (#5176) Signed-off-by: opensearchpplteam <opensearchpplteam@gmail.com> --------- Signed-off-by: opensearchpplteam <opensearchpplteam@gmail.com>
1 parent 3da7c18 commit ce68b0c

4 files changed

Lines changed: 194 additions & 1 deletion

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ private static boolean isScalarObject(Object obj) {
9494

9595
private static String doJsonize(Object candidate) {
9696
if (candidate == null) {
97-
return "null"; // Matches isScalarObject, but not toString-able.
97+
return null;
9898
} else if (isScalarObject(candidate)) {
9999
return candidate.toString();
100100
} else {
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.expression.function.jsonUDF;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
import static org.junit.jupiter.api.Assertions.assertNull;
10+
11+
import org.junit.jupiter.api.Test;
12+
13+
public class JsonExtractFunctionImplTest {
14+
15+
@Test
16+
public void testMissingPathReturnsNull() {
17+
Object result = JsonExtractFunctionImpl.eval("{}", "name");
18+
assertNull(result, "Missing path should return actual null, not string \"null\"");
19+
}
20+
21+
@Test
22+
public void testExplicitNullValueReturnsNull() {
23+
Object result = JsonExtractFunctionImpl.eval("{\"name\": null}", "name");
24+
assertNull(result, "Explicit JSON null value should return actual null, not string \"null\"");
25+
}
26+
27+
@Test
28+
public void testNoArgsReturnsNull() {
29+
assertNull(JsonExtractFunctionImpl.eval());
30+
}
31+
32+
@Test
33+
public void testSingleArgReturnsNull() {
34+
assertNull(JsonExtractFunctionImpl.eval("{}"));
35+
}
36+
37+
@Test
38+
public void testExtractStringValue() {
39+
Object result = JsonExtractFunctionImpl.eval("{\"name\": \"John\"}", "name");
40+
assertEquals("John", result);
41+
}
42+
43+
@Test
44+
public void testExtractNumericValue() {
45+
Object result = JsonExtractFunctionImpl.eval("{\"age\": 30}", "age");
46+
assertEquals("30", result);
47+
}
48+
49+
@Test
50+
public void testExtractNestedObject() {
51+
Object result = JsonExtractFunctionImpl.eval("{\"user\": {\"name\": \"John\"}}", "user");
52+
assertEquals("{\"name\":\"John\"}", result);
53+
}
54+
55+
@Test
56+
public void testExtractArray() {
57+
Object result = JsonExtractFunctionImpl.eval("{\"items\": [1, 2, 3]}", "items");
58+
assertEquals("[1,2,3]", result);
59+
}
60+
61+
@Test
62+
public void testExtractBooleanValue() {
63+
Object result = JsonExtractFunctionImpl.eval("{\"active\": true}", "active");
64+
assertEquals("true", result);
65+
}
66+
67+
@Test
68+
public void testMultiPathWithMissingPath() {
69+
Object result = JsonExtractFunctionImpl.eval("{\"name\": \"John\"}", "name", "age");
70+
assertEquals("[\"John\",null]", result);
71+
}
72+
73+
@Test
74+
public void testMultiPathAllMissing() {
75+
Object result = JsonExtractFunctionImpl.eval("{}", "name", "age");
76+
assertEquals("[null,null]", result);
77+
}
78+
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,36 @@ public void testJsonExtractWithMultiplyResult() throws IOException {
194194
verifyDataRows(actual, rows("[[\"Bridge of Sighs\",\"Ponte della Paglia\"],8981.0]"));
195195
}
196196

197+
@Test
198+
public void testJsonExtractReturnsNullForMissingPathAndNullValue() throws IOException {
199+
JSONObject actual =
200+
executeQuery(
201+
String.format(
202+
"source=%s | head 1 | eval a = json_extract('{}', 'name'),"
203+
+ " b = json_extract('{\\\"name\\\": null}', 'name')"
204+
+ " | fields a, b | head 1",
205+
TEST_INDEX_PEOPLE2));
206+
207+
verifySchema(actual, schema("a", "string"), schema("b", "string"));
208+
209+
verifyDataRows(actual, rows(null, null));
210+
}
211+
212+
@Test
213+
public void testJsonExtractMultiPathWithMissingPath() throws IOException {
214+
JSONObject actual =
215+
executeQuery(
216+
String.format(
217+
"source=%s | head 1 | eval a = json_extract('{\\\"name\\\": \\\"John\\\"}',"
218+
+ " 'name', 'age')"
219+
+ " | fields a | head 1",
220+
TEST_INDEX_PEOPLE2));
221+
222+
verifySchema(actual, schema("a", "string"));
223+
224+
verifyDataRows(actual, rows("[\"John\",null]"));
225+
}
226+
197227
@Test
198228
public void testJsonKeys() throws IOException {
199229
JSONObject actual =
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
setup:
2+
- do:
3+
query.settings:
4+
body:
5+
transient:
6+
plugins.calcite.enabled: true
7+
8+
- do:
9+
indices.create:
10+
index: issue5176
11+
body:
12+
settings:
13+
number_of_shards: 1
14+
number_of_replicas: 0
15+
mappings:
16+
properties:
17+
id:
18+
type: integer
19+
20+
- do:
21+
bulk:
22+
refresh: true
23+
body:
24+
- '{"index": {"_index": "issue5176", "_id": "1"}}'
25+
- '{"id": 1}'
26+
27+
---
28+
teardown:
29+
- do:
30+
indices.delete:
31+
index: issue5176
32+
ignore_unavailable: true
33+
- do:
34+
query.settings:
35+
body:
36+
transient:
37+
plugins.calcite.enabled: false
38+
39+
---
40+
"Issue 5176: json_extract returns null for missing path":
41+
- skip:
42+
features:
43+
- headers
44+
- do:
45+
headers:
46+
Content-Type: 'application/json'
47+
ppl:
48+
body:
49+
query: source=issue5176 | eval a = json_extract('{}', 'name') | fields a
50+
51+
- match: { total: 1 }
52+
- match: { schema: [ { name: a, type: string } ] }
53+
- match: { datarows: [ [ null ] ] }
54+
55+
---
56+
"Issue 5176: json_extract returns null for explicit JSON null value":
57+
- skip:
58+
features:
59+
- headers
60+
- do:
61+
headers:
62+
Content-Type: 'application/json'
63+
ppl:
64+
body:
65+
query: "source=issue5176 | eval a = json_extract('{\"name\": null}', 'name') | fields a"
66+
67+
- match: { total: 1 }
68+
- match: { schema: [ { name: a, type: string } ] }
69+
- match: { datarows: [ [ null ] ] }
70+
71+
---
72+
"Issue 5176: json_extract multi-path with missing path returns array with null":
73+
- skip:
74+
features:
75+
- headers
76+
- do:
77+
headers:
78+
Content-Type: 'application/json'
79+
ppl:
80+
body:
81+
query: "source=issue5176 | eval a = json_extract('{\"name\": \"John\"}', 'name', 'age') | fields a"
82+
83+
- match: { total: 1 }
84+
- match: { schema: [ { name: a, type: string } ] }
85+
- match: { datarows: [ [ "[\"John\",null]" ] ] }

0 commit comments

Comments
 (0)