Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Fixed encoding of NaN to json #990

Merged

Conversation

SimonSchneider
Copy link
Contributor

fixes: #988

@@ -43,6 +43,46 @@ fn primitive_serializer<'a, T: NativeType + ToLexical>(
))
}

fn f64_serializer<'a>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use generics to have a single implementation;

fn float_serializer<'a, T>(
    array: &'a PrimitiveArray<T>,
) -> Box<dyn StreamingIterator<Item = [u8]> + 'a + Send + Sync>
where T: num_traits::Float + NativeType + lexical_core::ToLexical
{
    Box::new(BufStreamingIterator::new(
        array.iter(),
        |x, buf| {
            if let Some(x) = x {
                if T::is_nan(*x) {
                    buf.extend(b"null")
                } else {
                    lexical_to_bytes_mut(*x, buf)
                }
            } else {
                buf.extend(b"null")
            }
        },
        vec![],
    ))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, didn't know that was possible 🎉

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #990 (19968a3) into main (b9aa8e8) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #990      +/-   ##
==========================================
+ Coverage   71.37%   71.39%   +0.02%     
==========================================
  Files         357      357              
  Lines       19781    19791      +10     
==========================================
+ Hits        14118    14130      +12     
+ Misses       5663     5661       -2     
Impacted Files Coverage Δ
src/io/json/write/serialize.rs 87.14% <100.00%> (+1.75%) ⬆️
src/bitmap/utils/slice_iterator.rs 87.93% <0.00%> (+1.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9aa8e8...19968a3. Read the comment docs.

@jorgecarleitao jorgecarleitao added the bug Something isn't working label May 16, 2022
@jorgecarleitao jorgecarleitao changed the title fix: encode float::NAN as null in json Fixed encoding of float::NAN to json May 16, 2022
Copy link
Owner

@jorgecarleitao jorgecarleitao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the PR. It looks great 💯

Thanks a lot @ritchie46 for the review 🙇

@jorgecarleitao jorgecarleitao merged commit 826a2b8 into jorgecarleitao:main May 16, 2022
@jorgecarleitao jorgecarleitao changed the title Fixed encoding of float::NAN to json Fixed encoding of NaN to json May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Serialising NaN float produces invalid JSON.
3 participants