feat: Implement `Scalar::From<HashMap<K, V>>` by Fokko · Pull Request #1541 · delta-io/delta-kernel-rs

Fokko · 2025-12-04T23:10:13Z

This PR extends the IntoEngineData derive macro to work with structs containing HashMap and Option<Vec<T>> fields by implementing the necessary TryFrom conversions for Scalar. This is foundational work to support reading arbitrary fields from CommitInfo.

This resolves #1083.

What changes are proposed in this pull request?

How was this change tested?

Resolves delta-io#1083

codecov · 2025-12-04T23:12:39Z

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.92%. Comparing base (f8106c5) to head (9dc8ecf).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
kernel/src/expressions/scalars.rs	75.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1541      +/-   ##
==========================================
- Coverage   83.93%   83.92%   -0.02%     
==========================================
  Files         120      120              
  Lines       32962    32930      -32     
  Branches    32962    32930      -32     
==========================================
- Hits        27668    27637      -31     
- Misses       3925     3927       +2     
+ Partials     1369     1366       -3

☔ 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.

…love-macros

emkornfield · 2025-12-08T23:29:47Z

@Fokko could you fill in the description of the PR please when you get a chance.

emkornfield · 2025-12-08T23:34:21Z

    }
 }
+
+impl From<Infallible> for Error {


I'm not exactly clear what this is doing here, or why we need it? Could you add a comment?

Good one, I've added a comment, let me know if it makes sense 👍

emkornfield · 2025-12-08T23:35:57Z

+    fn try_from(opt: Option<Vec<T>>) -> Result<Self, Self::Error> {
+        match opt {
+            Some(vec) => vec.try_into(),
+            None => Ok(Self::Null(ArrayType::new(T::to_data_type(), false).into())),


what is the false here? Not nullable?

would this depend on if T is an Option?

Yes, exactly! So, if T is wrapped with an Option, then it needs to be set to true. For example, here:

delta-kernel-rs/kernel/src/expressions/scalars.rs

Lines 565 to 576 in 87d2844

impl<T> TryFrom<Vec<Option<T>>> for Scalar

where

T: Into<Scalar> + ToDataType,

{

type Error = Error;

fn try_from(vec: Vec<Option<T>>) -> Result<Self, Self::Error> {

let array_type = ArrayType::new(T::to_data_type(), true);

let array_data = ArrayData::try_new(array_type, vec)?;

Ok(array_data.into())

}

}

T itself cannot be an option, since there is no corresponding Null scalar, so we can assume that they are all not-null.

Hrmm, we already have:

impl<T> TryFrom<Vec<T>> for Scalar

impl<K, V> TryFrom<HashMap<K, Option<V>>> for Scalar

impl<T: Into<Scalar> + ToDataType> From<Option<T>> for Scalar

If we just added impl<T:TryInto<Scalar> + ToDataType> TryFrom<Option<T>> for Scalar would that get us what we want without having to special case Vec and HashMap?

…love-macros

nicklan

thanks, one more suggestion

nicklan · 2025-12-16T00:34:45Z

            in_commit_timestamp,
            operation: Some(operation.unwrap_or_else(|| UNKNOWN_OPERATION.to_string())),
-            operation_parameters: None,
+            operation_parameters: Some(HashMap::new()),


why this change?

nicklan · 2025-12-16T00:47:40Z

+    fn try_from(opt: Option<Vec<T>>) -> Result<Self, Self::Error> {
+        match opt {
+            Some(vec) => vec.try_into(),
+            None => Ok(Self::Null(ArrayType::new(T::to_data_type(), false).into())),


Hrmm, we already have:

impl<T> TryFrom<Vec<T>> for Scalar

impl<K, V> TryFrom<HashMap<K, Option<V>>> for Scalar

impl<T: Into<Scalar> + ToDataType> From<Option<T>> for Scalar

If we just added impl<T:TryInto<Scalar> + ToDataType> TryFrom<Option<T>> for Scalar would that get us what we want without having to special case Vec and HashMap?

Fokko · 2026-01-08T20:37:13Z

@nicklan

Unfortunately, this approach doesn't work due to Rust's trait coherence rules. The standard library has a blanket impl<T, U> TryFrom<U> for T where U: Into<T> which already provides TryFrom<Option<T>> for any T that has From<Option<T>> implemented (i.e., the primitive types via our existing impl<T: Into<Scalar> + ToDataType> From<Option<T>> for Scalar).

Adding a generic impl<T: TryInto<Scalar> + ToDataType> TryFrom<Option<T>> for Scalar conflicts with this blanket impl because the compiler can't distinguish which implementation to use for types where both bounds apply.

The special-case implementations for Option<Vec<T>> and Option<HashMap<K, V>> work precisely because Vec<T> and HashMap<K, V> only implement TryInto<Scalar> (not Into<Scalar>), so there's no conflict with the blanket impl.

nicklan · 2026-01-08T22:23:39Z

The special-case implementations for Option<Vec<T>> and Option<HashMap<K, V>> work precisely because Vec<T> and HashMap<K, V> only implement TryInto<Scalar> (not Into<Scalar>), so there's no conflict with the blanket impl.

Got it, thanks for the explanation

…love-macros

feat: Implement Scalar::From<HashMap<K, V>>

d0d36f5

Resolves delta-io#1083

github-actions Bot assigned Fokko Dec 4, 2025

Fokko mentioned this pull request Dec 4, 2025

feat: Implement Scalar for HashMap<K,V> #1468

Closed

Fokko added 2 commits December 8, 2025 11:26

Merge branch 'main' of github.com:delta-io/delta-kernel-rs into fd-i-…

2181f8f

…love-macros

cargo fmt

00d074f

nicklan requested review from emkornfield and nicklan December 8, 2025 19:33

emkornfield reviewed Dec 8, 2025

View reviewed changes

Fokko added 2 commits December 9, 2025 14:17

Merge branch 'main' of github.com:delta-io/delta-kernel-rs into fd-i-…

2538a19

…love-macros

Add comment

6481861

Fokko requested a review from emkornfield December 9, 2025 17:26

emkornfield approved these changes Dec 9, 2025

View reviewed changes

Fokko added 7 commits December 9, 2025 23:39

Merge branch 'main' into fd-i-love-macros

f97937b

Merge branch 'main' into fd-i-love-macros

ba3778d

Merge branch 'main' into fd-i-love-macros

bf4415e

Merge branch 'main' into fd-i-love-macros

4c2bde6

Merge branch 'main' into fd-i-love-macros

89787fa

Merge branch 'main' into fd-i-love-macros

ec53cea

Merge branch 'main' into fd-i-love-macros

3bce0be

nicklan reviewed Dec 16, 2025

View reviewed changes

Merge branch 'main' into fd-i-love-macros

8667cee

Fokko requested a review from nicklan January 8, 2026 20:37

nicklan approved these changes Jan 8, 2026

View reviewed changes

Fokko added 2 commits January 14, 2026 10:40

Merge branch 'main' of github.com:delta-io/delta-kernel-rs into fd-i-…

65b3336

…love-macros

Make clippy happy

9dc8ecf

Fokko merged commit 2014edb into delta-io:main Jan 14, 2026
21 of 22 checks passed

Fokko deleted the fd-i-love-macros branch January 14, 2026 11:10

	impl<T> TryFrom<Vec<Option<T>>> for Scalar
	where
	T: Into<Scalar> + ToDataType,
	{
	type Error = Error;

	fn try_from(vec: Vec<Option<T>>) -> Result<Self, Self::Error> {
	let array_type = ArrayType::new(T::to_data_type(), true);
	let array_data = ArrayData::try_new(array_type, vec)?;
	Ok(array_data.into())
	}
	}

Uh oh!

Conversation

Fokko commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed in this pull request?

How was this change tested?

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

emkornfield commented Dec 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicklan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fokko commented Jan 8, 2026

Uh oh!

nicklan commented Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fokko commented Dec 4, 2025 •

edited

Loading

codecov Bot commented Dec 4, 2025 •

edited

Loading