Add Rust UDFs for json, json_object, json_array

[vinaykpud]

Description

DataFusion's stdlib has no JSON constructors, so PPL queries calling json(), json_object(), or json_array() on parquet-backed indices failed with:

IllegalStateException: No backend supports scalar function [JSON_OBJECT] among [datafusion]

This PR adds three custom Rust UDFs (json / json_object / json_array) and wires them through the full chain — ScalarFunction enum, JsonFunctionAdapters, the backend project-ops registry, ADDITIONAL_SCALAR_SIGS, and the substrait extension yaml.

Semantics

• json(s) — round-trips parse a JSON string and returns the canonical re-serialisation. NULL on malformed input or NULL operand (matches PPL legacy JsonFunctionImpl.eval).
• json_object(k1, v1, k2, v2, ...) — variadic key/value pairs → JSON-object literal. Values keep their original Calcite type when emitted: numerics unquoted, booleans unquoted, nested JSON-builtin results quoted-and-escaped, plain strings JSON-quoted.
• json_array(v1, v2, ...) — variadic scalars → JSON-array literal. Same per-value type fidelity as json_object.

The json_object / json_array adapters strip Calcite's leading SqlJsonConstructorNullClause flag and prepend a per-value type-tag (n / b / j / s) so the Rust UDF can recover the original type when emitting JSON.

Query shapes now supported

| eval x = json('[1,2,3,{"f1":1,"f2":[5,6]},4]')
| eval x = json('{invalid')                          (returns NULL)
| eval x = json_object('key', 123.45)                (numeric value)
| eval x = json_object('flag', true, 'count', 42)    (mixed types)
| eval x = json_object('outer', json_object('inner', 1))  (nested constructor)
| eval x = json_array(1, 2, 3, 4.5, -1)              (heterogeneous numerics)
| eval x = json_array('Tom', 'Walt')                 (strings)
| eval x = json_array(true, false, 'maybe')          (mixed types)
| eval x = json_append(payload, 'student', json_object('name', 'Tomy', 'rank', 5))
| eval x = json_append(payload, 'school.teacher', json_array('Tom', 'Walt'))
| eval x = json_extend(payload, 'student', json_object('name', 'Tommy', 'rank', 5))

Tests

• Rust unit: 15 new tests across udf/json.rs, udf/json_object.rs, udf/json_array.rs — happy path, NULL propagation, arity guards, per-tag dispatch.
• Java unit: 3 new cases in JsonFunctionAdaptersTests — flag stripping, type-tag insertion for numeric/string values, operator-identity preservation.
• Sandbox QA IT: new JsonBuiltinsIT over the shared calcs dataset covers the five primary query shapes (valid + invalid json, single-pair / nested json_object, heterogeneous-numeric / string json_array) end-to-end against a parquet-backed index.

PPL Calcite tests in the SQL plugin unblocked

• CalcitePPLJsonBuiltinFunctionIT.testJson
• CalcitePPLJsonBuiltinFunctionIT.testJsonAppend
• CalcitePPLJsonBuiltinFunctionIT.testJsonExtend

(testJsonObject and testJsonArray in the same class were also failing for the same root cause and now pass.)

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 9765d6f)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue encode_value silently falls back to string encoding when numeric parsing fails. If a value tagged "n" contains invalid numeric text (e.g., "12.34.56"), serde_json::from_str fails and the function returns Value::String(value.to_string()) instead of signaling an error. This produces a JSON object with a quoted string where an unquoted number was intended, breaking the contract that "n"-tagged values emit unquoted. The scenario arises when upstream code (e.g., a buggy adapter or corrupted data) passes malformed numeric text with an "n" tag. pub(crate) fn encode_value(tag: &str, value: &str) -> Value { match tag { "n" => serde_json::from_str::<Value>(value).unwrap_or(Value::String(value.to_string())), "b" => match value { Possible Issue build_object returns None when cells is empty, but an empty JSON object {} is a valid result for zero key-value pairs. The current logic treats zero operands as an error condition (line 117), forcing callers with no pairs to receive NULL instead of an empty object. This breaks queries like json_object() which should return {} per standard JSON constructor semantics. if cells.is_empty() || !cells.len().is_multiple_of(3) { return None; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 9765d6f

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Handle empty input correctly The function returns None for empty input, but an empty JSON object {} is valid JSON. This breaks the contract for zero-argument calls like json_object(), which should produce {} rather than NULL. sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_object.rs [116-119] fn build_object(cells: &[Option<&str>]) -> Option<String> { - if cells.is_empty() || !cells.len().is_multiple_of(3) { + if !cells.len().is_multiple_of(3) { return None; + } + if cells.is_empty() { + return Some("{}".to_string()); } ... } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that build_object returns None for empty input, but an empty JSON object {} is valid. However, the legacy behavior and test expectations need verification before changing this contract.	Medium
General	Validate numeric parse results Using unwrap_or silently converts malformed numeric strings to JSON strings instead of failing. This masks data corruption when invalid numeric values are passed with tag "n". sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_object.rs [138] -"n" => serde_json::from_str::<Value>(value).unwrap_or(Value::String(value.to_string())), +"n" => match serde_json::from_str::<Value>(value) { + Ok(v) if v.is_number() => v, + _ => Value::String(value.to_string()), +}, Suggestion importance[1-10]: 6 __ Why: The suggestion improves robustness by validating that parsed values are actually numbers. However, the current unwrap_or fallback to string is intentional for handling edge cases, and the added is_number() check provides marginal improvement.	Low
	Use case-insensitive operator comparison Case-sensitive string comparison may fail if operator names arrive in different cases. Use equalsIgnoreCase to ensure robust matching across Calcite's operator name normalization paths. sandbox/plugins/analytics-backend-datafusion/src/main/java/org/opensearch/be/datafusion/JsonFunctionAdapters.java [343-348] private static String typeTag(RexNode value) { if (value instanceof RexCall call) { String op = call.getOperator().getName(); - if (op.equals("json_object") || op.equals("json_array") || op.equals("json")) { + if (op.equalsIgnoreCase("json_object") || op.equalsIgnoreCase("json_array") || op.equalsIgnoreCase("json")) { return "j"; } } ... } Suggestion importance[1-10]: 3 __ Why: While case-insensitive comparison could be more defensive, Calcite's operator names are consistently lowercase in this codebase. The suggestion adds unnecessary overhead without evidence of actual case-sensitivity issues.	Low

---

Previous suggestions

Suggestions up to commit 4abefbe

Category	Suggestion	Impact
General	Log numeric parse failures The numeric parsing uses unwrap_or to fall back to string encoding on parse failure. However, malformed numeric input (e.g., "123abc") will silently produce a string instead of a number, potentially masking data quality issues. Consider logging the parse failure or returning an error to surface invalid numeric values. sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_object.rs [136-138] pub(crate) fn encode_value(tag: &str, value: &str) -> Value { match tag { - "n" => serde_json::from_str::<Value>(value).unwrap_or(Value::String(value.to_string())), + "n" => serde_json::from_str::<Value>(value).unwrap_or_else(|_| { + log::warn!("Failed to parse numeric value: {}", value); + Value::String(value.to_string()) + }), ... } } Suggestion importance[1-10]: 5 __ Why: Adding logging for parse failures would improve observability and help diagnose data quality issues. However, the current fallback behavior (treating malformed numerics as strings) appears intentional for robustness. The suggestion is valid but represents a moderate enhancement rather than fixing a critical issue.	Low
	Handle empty input consistently The function returns None for empty input, but an empty JSON object {} is valid JSON. This contradicts the test arity_must_be_triple which expects None for empty input. Consider returning Some("{}") for zero operands to align with standard JSON semantics, or update the test to reflect the intended behavior. sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_object.rs [116-119] fn build_object(cells: &[Option<&str>]) -> Option<String> { - if cells.is_empty() || !cells.len().is_multiple_of(3) { + if !cells.len().is_multiple_of(3) { return None; + } + if cells.is_empty() { + return Some("{}".to_string()); } ... } Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid point about empty input handling, but the current behavior (returning None for empty input) is explicitly tested and appears intentional. The suggestion to return {} would be a behavioral change requiring broader discussion. The impact is low since this is a design choice rather than a bug.	Low

Suggestions up to commit a0b559c

Category	Suggestion	Impact
General	Handle empty input correctly The function returns None when cells is empty, but an empty JSON object {} is valid JSON. Consider returning Some("{}") for empty input to align with standard JSON semantics and the json_array implementation which returns [] for empty input. sandbox/plugins/analytics-backend-datafusion/rust/src/udf/json_object.rs [116-119] fn build_object(cells: &[Option<&str>]) -> Option<String> { - if cells.is_empty() || !cells.len().is_multiple_of(3) { + if !cells.len().is_multiple_of(3) { return None; + } + if cells.is_empty() { + return Some("{}".to_string()); } ... } Suggestion importance[1-10]: 4 __ Why: While the suggestion correctly identifies that json_array returns [] for empty input, the current behavior of returning None for empty input in json_object may be intentional. The suggestion would improve consistency, but the impact is minor since empty object construction is an edge case.	Low

[github-actions]

❌ Gradle check result for a0b559c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 4abefbe

[github-actions]

✅ Gradle check result for 4abefbe: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.45%. Comparing base (5aafb9a) to head (9765d6f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #22130      +/-   ##
============================================
+ Coverage     73.33%   73.45%   +0.11%     
- Complexity    75798    75939     +141     
============================================
  Files          6070     6070              
  Lines        344610   344610              
  Branches      49576    49576              
============================================
+ Hits         252734   253117     +383     
+ Misses        71769    71329     -440     
- Partials      20107    20164      +57     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 9765d6f.

Path	Line	Severity	Description
sandbox/qa/analytics-engine-rest/src/test/java/org/opensearch/analytics/qa/MathScalarFunctionsIT.java	383	low	@AwaitsFix uses a plain description string instead of a verifiable bug-tracker URL. The annotation silences testConvInvalidRadixThrows with no linkable issue, making the suppression unauditable. While likely just non-standard practice, it means a test covering error-message fidelity on a parse path is disabled without a traceable justification.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 0 | Medium: 0 | Low: 1

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

Persistent review updated to latest commit 9765d6f

[github-actions]

✅ Gradle check result for 9765d6f: SUCCESS