Skip to content
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

feat(frontend): Trigonometric functions in degrees #8884

Closed
wants to merge 24 commits into from
Closed

Conversation

CAJan93
Copy link
Contributor

@CAJan93 CAJan93 commented Mar 30, 2023

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

Implement trigonometric functions in degree. Merge after original one #8838

#8806

  • sind / cosd / tand / cotd / asind / acosd / atand / atan2d

Checklist For Contributors

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • I have demonstrated that backward compatibility is not broken by breaking changes and created issues to track deprecated features to be removed in the future. (Please refer to the issue)
  • All checks passed in ./risedev check (or alias, ./risedev c)

Checklist For Reviewers

  • I have requested macro/micro-benchmarks as this PR can affect performance substantially, and the results are shown.

Documentation

Documents are here: #9678

@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Merging #8884 (0f16e47) into main (07dbb8a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 0f16e47 differs from pull request most recent head c4363f0. Consider uploading reports for the commit c4363f0 to get more accurate results

@@           Coverage Diff            @@
##             main    #8884    +/-   ##
========================================
  Coverage   70.80%   70.80%            
========================================
  Files        1181     1177     -4     
  Lines      195755   195234   -521     
========================================
- Hits       138602   138234   -368     
+ Misses      57153    57000   -153     
Flag Coverage Δ
rust 70.80% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/expr/src/vector_op/trigonometric.rs 100.00% <100.00%> (+5.63%) ⬆️
src/frontend/src/binder/expr/function.rs 89.45% <100.00%> (+0.14%) ⬆️

... and 65 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines +59 to +62
#[function("sind(float64) -> float64")]
pub fn sind_f64(input: F64) -> F64 {
f64::sin(f64::to_radians(input.0)).into()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The PostgreSQL implementation is nontrivial, as it is intended to be more accurate than conversion first.

Another way to work with angles measured in degrees is to use the unit transformation functions radians() and degrees() shown earlier. However, using the degree-based trigonometric functions is preferred, as that way avoids round-off error for special cases such as sind(30).

For reference, see this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for pointing that out. Sorry, I did not have time to continue working on this. I will try to introduce the function one by one whenever I have time for it. My first short at Sind: #9056

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cosd: #9064

@neverchanje neverchanje added the user-facing-changes Contains changes that are visible to users label May 9, 2023
@neverchanje neverchanje linked an issue May 9, 2023 that may be closed by this pull request
1 task
@CAJan93
Copy link
Contributor Author

CAJan93 commented May 11, 2023

Closing this one. Will break it up in small PRs. Next up is asind

@CAJan93 CAJan93 closed this May 11, 2023
@CharlieSYH CharlieSYH added the 📖✗ No user documentation is needed. label May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature user-facing-changes Contains changes that are visible to users 📖✗ No user documentation is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement sind / cosd / tand / cotd / asind / acosd / atand / atan2d
4 participants