-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix issue#2058 file_format/json.rs attempt to subtract with overflow #2066
Conversation
@alamb Please check it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for your contribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll be better to add a test for the fix :)
Thank you for the contribution @silence-coding
I agree with @xudong963 -- can you please provide a test (both to demonstrate what is wrong and help ensure we don't break it again in the future)? Perhaps you can adapt the reproducer on #2058 |
@alamb @xudong963 That's my problem. I forgot to write test cases. |
Sadly I think #2023 added 3.json already -- can you fix this PR to use the name like: --- a/datafusion/src/datasource/file_format/json.rs
+++ b/datafusion/src/datasource/file_format/json.rs
@@ -236,7 +236,7 @@ mod tests {
#[tokio::test]
async fn infer_schema_with_limit() {
- let filename = "tests/jsons/3.json";
+ let filename = "tests/jsons/4.json";
let format = JsonFormat::default()
.with_schema_infer_max_rec(Some(3));
let file_schema = format
diff --git a/datafusion/tests/jsons/3.json b/datafusion/tests/jsons/4.json
similarity index 100%
rename from datafusion/tests/jsons/3.json
rename to datafusion/tests/jsons/4.json to resolve the conflict? |
@alamb fix with schema_infer_limit.json |
When will the next version be released? |
I think we can open an issue to discuss the next version release. |
I created #2095 to start this conversation -- @silence-coding it would be great to hear your perspective if you are thinking of a release directly from master, or a release that has only bug fixes, or something else |
Thanks again @silence-coding |
fix #2058