-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Move ScalarValue tests alongside implementation, move from_slice to datafusion_core
#2914
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
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.
Github makes this look like a massive diff, but it has only about 5 lines of substance
| ScalarValue::TimestampNanosecond(r, None) | ||
| } | ||
| } | ||
|
|
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.
these tests are moved, verbatim. The only change is that this line
use arrow::{array::*, datatypes::*};was removed at the direction of the rust compiler as it was no longer used
andygrove
left a comment
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.
Thanks @alamb. The current state was likely a result of the recent refactoring of crates and modules.
|
Benchmark runs are scheduled for baseline = fb2221c and contender = d8a2f58. d8a2f58 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 #2913
Rationale for this change
I am working on #2891 and it was not clear there were existing
ScalarValuetests because they were in a different module than their implementationImplementation: https://github.com/apache/arrow-datafusion/blob/fd64e6f2e5ca5ba8c33c431b75a3ba9441091970/datafusion/common/src/scalar.rs
Tests https://github.com/apache/arrow-datafusion/blob/85ca8be669dc1fbba4b2eefbf99fc2ed1546cc0c/datafusion/core/src/scalar.rs
🤯
What changes are included in this PR?
ScalarValuetests alongside implementationfrom_slicetodatafusion_core(as it is used in the ScalarValue implementation)Are there any user-facing changes?
from_sliceis now in thedatafusion_corecrate (used to be indatafusion_physical_exprcrate), but their is apub useso no existing code needs to be changed (they can still importfrom_slicefromdatafusion_physical_expr)