-
Notifications
You must be signed in to change notification settings - Fork 784
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
Remove JsonEqual #2317
Remove JsonEqual #2317
Conversation
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.
What a fast turn around, love it 👍
Edit: there appear to be some test failures
} | ||
|
||
/// Construct an Arrow array from a partially typed JSON column | ||
fn array_from_json( |
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.
This appears to have just been moved
a49db10
to
e7270ef
Compare
@@ -127,7 +127,7 @@ pub fn timestamp_ns_to_datetime(v: i64) -> NaiveDateTime { | |||
// extract seconds from nanoseconds | |||
v / NANOSECONDS, | |||
// discard extracted seconds | |||
(v % NANOSECONDS) as u32, | |||
if v > 0 { (v % NANOSECONDS) as u32 } else { 0 }, |
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.
When v
is negative, this parameter causes invalid or out-of-range datetime
error. Just temporarily avoid the failure here. Other function like timestamp_us_to_datetime
might have same issue.
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.
@@ -945,22 +1258,25 @@ mod tests { | |||
.len(3) | |||
.add_buffer(value_offsets) | |||
.add_child_data(value_data.into_data()) | |||
.null_bit_buffer(Some(Buffer::from([0b00000011]))) |
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.
This appears to be a bug in previous code. The integration.json
specifies validity like the update, but it doesn't reflect that before.
.len(3) | ||
.add_child_data(structs_int32s.data().clone()) | ||
.add_child_data(structs_utf8s.data().clone()) | ||
.null_bit_buffer(Some(Buffer::from([0b00000011]))) |
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.
same as to reflect the validity data in integration.json
.
badbabb
to
dae94f2
Compare
Benchmark runs are scheduled for baseline = 5166a08 and contender = 2683b06. 2683b06 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2296.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?