Skip to content

Conversation

@martin-g
Copy link
Member

@martin-g martin-g commented Aug 7, 2024

AVRO-4024

This PR is a follow up of #3051

What is the purpose of the change

  • It sync the possible String default values for Float/Double to "NaN", "INF", "Infinity", "-INF" and "-Infinity" (case sensitive, without trimming!). This way the Rust SDK will behave the same way as the Java and C# SDKs

Verifying this change

  • New and old tests should pass

Documentation

  • Does this pull request introduce a new feature? yes
  • If yes, how is the feature documented? - The specification will be updated in another PR

…ing into Value::Float/Double

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
…Infinity"

This is what the Java SDK (via Jackson library) supports (#3066).
This is what the C# SDK also would support (#3070)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
@github-actions github-actions bot added the Rust label Aug 7, 2024
@martin-g martin-g merged commit dffc13a into main Aug 7, 2024
@martin-g martin-g deleted the avro-4024-nan-inf-float-defaults branch August 7, 2024 11:42
martin-g added a commit that referenced this pull request Aug 7, 2024
…g numbers with Java and C# (#3073)

* AVRO-4024: [Rust] Improve the error messages when parsing unknown String into Value::Float/Double

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* AVRO-4024: [Rust] Accept only "Nan", "INF", "-INF", "Infinity" and "-Infinity"

This is what the Java SDK (via Jackson library) supports (#3066).
This is what the C# SDK also would support (#3070)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

---------

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
(cherry picked from commit dffc13a)
Comment on lines +3146 to +3156
match value.resolve(&schema) {
Err(err @ Error::GetDouble(_)) => {
assert_eq!(
format!("{err:?}"),
r#"Double expected, got String("unknown")"#
);
}
other => {
panic!("Expected Error::GetDouble, got {other:?}");
}
}
Copy link
Member

@xxchan xxchan Aug 7, 2024

Choose a reason for hiding this comment

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

Just FYI: there's a tool that can make testing a lot more happier https://github.com/rust-analyzer/expect-test

Basically it's just like

assert_eq!(format!("{:?}", value.resolve(&schema)), "balabala")

We don't manually match, but just print out the result.

What's cooler is that expect_test can automatically update the actual result for you. (You can see the video demo in that repo)

opwvhk pushed a commit to opwvhk/avro that referenced this pull request Sep 5, 2025
…g numbers with Java and C# (apache#3073)

* AVRO-4024: [Rust] Improve the error messages when parsing unknown String into Value::Float/Double

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

* AVRO-4024: [Rust] Accept only "Nan", "INF", "-INF", "Infinity" and "-Infinity"

This is what the Java SDK (via Jackson library) supports (apache#3066).
This is what the C# SDK also would support (apache#3070)

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>

---------

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants