-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Simplify math expression code (use unary kernel) #309
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
| let sql = "SELECT avg(sqrt(c11)) FROM aggregate_test_100"; | ||
| let actual = execute(&mut ctx, sql).await; | ||
| let expected = vec![vec!["0.6584408485889435"]]; | ||
| let expected = vec![vec!["0.6584407806396484"]]; |
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.
Small diff, since we avoid as f64 now.
| Some(array) => compute_op!(array, $FUNC, $TYPE), | ||
| Some(array) => { | ||
| let res: $TYPE = | ||
| arrow::compute::kernels::arity::unary(array, |x| x.$FUNC()); |
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 avoids creating intermediate Vec and uses the (efficient) unary kernel
jorgecarleitao
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.
Looks great. 👍
|
🎉 |
* HashMergeJoin metrics * HashMergeJoin metrics test * Fix test * Fix format * Fix descriptions * Fix imports * Update spark/src/test/scala/org/apache/comet/exec/CometExecSuite.scala Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com> * delete conf * Fix --------- Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com>
* Fix broken use of environment variables in GitHub actions * Fix broken workflow file and add actionlint pre-commit check to prevent future errors * Install protoc * Add rustup-components * fix maturin-action bugs * add explanation * add protoc to sdist and manylinux * Update .github/workflows/build.yml Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com> * install protobuf compiler * add protoc * fix invalid build yaml * try set protoc path * try suggestion from ChatGPT * experiment * revert change --------- Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com> Co-authored-by: Ian Alexander Joiner <14581281+iajoiner@users.noreply.github.com>
Which issue does this PR close?
Closes #308
Rationale for this change
Simplifies code, probably is faster too. Also involves one cast less which causes more rounding errors.
What changes are included in this PR?
Use
arity::unaryfunction to simplify math expression codeAre there any user-facing changes?
No