Skip to content

Commit

Permalink
fix: modify tests which compare partition values due to breakage in k…
Browse files Browse the repository at this point in the history
…ernel

as of delta-kernel-rs 0.7.0 a Vec<Scalar> will no longer be comparable
if there are nulls in it, which breaks a couple of our tests which
perform said check.

I feel that this is likely going to cause downstream issues with users
of delta-rs, in which case we will have to bring it up with the kernel
developers who assert that Null(String) will never equal Null(String).

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>
  • Loading branch information
rtyler committed Mar 8, 2025
1 parent fdecab7 commit 19fb4a1
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 16 deletions.
20 changes: 10 additions & 10 deletions crates/core/src/operations/convert_to_delta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,16 +605,16 @@ mod tests {
.collect::<Vec<_>>();
partition_values.sort_by_key(|(k, v)| (k.clone(), v.serialize()));

for it in expected_partition_values.iter().zip(partition_values.iter()) {
let (expected, found) = it;

match (expected, found) {
// No-oipping the comparison of null. delta-kernel-rs introduces a regression where
// 0.7.0 cannot perform equivalency of typed null Scalars.
(Scalar::Null(_), Scalar::Null(_)) => {},
_others => {
assert_eq!(expected, found);
},
for (position, expected) in expected_partition_values.iter().enumerate() {
let (key, value) = expected;
let (found_key, found_value) = partition_values[position].clone();
assert_eq!(key, &found_key);

match (value, found_value) {
// no-op the null comparison due to a breaking change in delta-kernel-rs 0.7.0
// which changes null comparables
(Scalar::Null(_), Scalar::Null(_)) => {}
(v, fv) => assert_eq!(v, &fv),
}
}
}
Expand Down
7 changes: 1 addition & 6 deletions crates/core/src/writer/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,18 +542,13 @@ mod tests {

assert_eq!(
extract_partition_values(
&[
String::from("col1"),
String::from("col2"),
String::from("col3")
],
&[String::from("col1"), String::from("col2"),],
&record_batch
)
.unwrap(),
IndexMap::from([
(String::from("col1"), Scalar::Integer(1)),
(String::from("col2"), Scalar::Integer(2)),
(String::from("col3"), Scalar::Null(DataType::INTEGER)),
])
);
assert_eq!(
Expand Down

0 comments on commit 19fb4a1

Please sign in to comment.