Add from_json func #103

adriangb · 2025-12-17T07:29:10Z

No description provided.

codecov-commenter · 2025-12-17T07:32:49Z

Codecov Report

❌ Patch coverage is 93.98496% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.28%. Comparing base (9cb9b1d) to head (6f5a676).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/json_from_scalar.rs	93.93%	6 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
+ Coverage   84.12%   85.28%   +1.16%     
==========================================
  Files          16       17       +1     
  Lines        1216     1346     +130     
  Branches     1216     1346     +130     
==========================================
+ Hits         1023     1148     +125     
- Misses        131      134       +3     
- Partials       62       64       +2

☔ View full report in Codecov by Sentry.
📢 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.

friendlymatthew

Makes sense to me. I think we should address 2 comments below before merge

friendlymatthew · 2025-12-17T15:24:29Z

src/json_from_scalar.rs

+impl Default for JsonFromScalar {
+    fn default() -> Self {
+        Self {
+            signature: Signature::variadic_any(Volatility::Immutable),


It's a bit odd that we model this as Signature::variadic_any when the function requires exactly 1 argument

Signature::any(1, Volatility::Immutable) seems like a better fit

friendlymatthew · 2025-12-17T15:30:34Z

src/json_from_scalar.rs

+    }
+}
+
+fn scalar_to_json_union_field(scalar: ScalarValue) -> DataFusionResult<Option<JsonUnionField>> {


It seems the return type's Option wrap is superfluous. I don't see any of the arms below return an Ok(None)

friendlymatthew · 2025-12-17T15:34:11Z

src/json_from_scalar.rs

+    match array.data_type() {
+        DataType::Null => {
+            for _ in 0..array.len() {
+                union.push(JsonUnionField::JsonNull);
+            }
+        }
+
+        DataType::Boolean => {
+            let arr = array.as_boolean();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Bool(arr.value(i)));
+                }
+            }
+        }
+
+        // Integer types - coerce to i64
+        DataType::Int8 => {
+            let arr = array.as_primitive::<Int8Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(i64::from(arr.value(i))));
+                }
+            }
+        }
+        DataType::Int16 => {
+            let arr = array.as_primitive::<Int16Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(i64::from(arr.value(i))));
+                }
+            }
+        }
+        DataType::Int32 => {
+            let arr = array.as_primitive::<Int32Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(i64::from(arr.value(i))));
+                }
+            }
+        }
+        DataType::Int64 => {
+            let arr = array.as_primitive::<Int64Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(arr.value(i)));
+                }
+            }
+        }
+        DataType::UInt8 => {
+            let arr = array.as_primitive::<UInt8Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(i64::from(arr.value(i))));
+                }
+            }
+        }
+        DataType::UInt16 => {
+            let arr = array.as_primitive::<UInt16Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(i64::from(arr.value(i))));
+                }
+            }
+        }
+        DataType::UInt32 => {
+            let arr = array.as_primitive::<UInt32Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(i64::from(arr.value(i))));
+                }
+            }
+        }
+        DataType::UInt64 => {
+            let arr = array.as_primitive::<UInt64Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Int(i64::try_from(arr.value(i)).map_err(|_| {
+                        exec_datafusion_err!("UInt64 value {} is out of range for i64", arr.value(i))
+                    })?));
+                }
+            }
+        }
+
+        // Float types - coerce to f64
+        DataType::Float32 => {
+            let arr = array.as_primitive::<Float32Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Float(f64::from(arr.value(i))));
+                }
+            }
+        }
+        DataType::Float64 => {
+            let arr = array.as_primitive::<Float64Type>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Float(arr.value(i)));
+                }
+            }
+        }
+
+        // String types
+        DataType::Utf8 => {
+            let arr = array.as_string::<i32>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Str(arr.value(i).to_string()));
+                }
+            }
+        }
+        DataType::LargeUtf8 => {
+            let arr = array.as_string::<i64>();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Str(arr.value(i).to_string()));
+                }
+            }
+        }
+        DataType::Utf8View => {
+            let arr = array.as_string_view();
+            for i in 0..arr.len() {
+                if arr.is_null(i) {
+                    union.push_none();
+                } else {
+                    union.push(JsonUnionField::Str(arr.value(i).to_string()));
+                }
+            }
+        }


More of a nit and note about future work, we could probably remove a lot of this code if we impl JsonUnion::from_iter where the Iterator::Item is an Option<T> where None represents null 🤔

Add from_json func

6737974

adriangb added a commit that referenced this pull request Dec 17, 2025

Add from_json func #103

bc6f86c

adriangb added 2 commits December 17, 2025 09:10

fmt

3220698

lints

775b60a

friendlymatthew approved these changes Dec 17, 2025

View reviewed changes

adriangb added 5 commits December 17, 2025 10:42

fix clippy

2783193

implement suggestion, fix test

f71b883

remove argument checks

fe20268

lint

c99eaea

more tests, lint

6f5a676

adriangb merged commit 9c37683 into main Dec 17, 2025
7 checks passed

adriangb deleted the cast-integers branch December 17, 2025 18:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add from_json func #103

Add from_json func #103

Uh oh!

adriangb commented Dec 17, 2025

Uh oh!

codecov-commenter commented Dec 17, 2025 •

edited

Loading

Uh oh!

friendlymatthew left a comment

Uh oh!

friendlymatthew Dec 17, 2025

Uh oh!

friendlymatthew Dec 17, 2025

Uh oh!

friendlymatthew Dec 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add from_json func #103

Add from_json func #103

Uh oh!

Conversation

adriangb commented Dec 17, 2025

Uh oh!

codecov-commenter commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

friendlymatthew left a comment

Choose a reason for hiding this comment

Uh oh!

friendlymatthew Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

friendlymatthew Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

friendlymatthew Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Dec 17, 2025 •

edited

Loading