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

extended log.rs tests for unary/binary and f32/f64 casting #13034

Merged
merged 3 commits into from
Oct 23, 2024

Conversation

buraksenn
Copy link
Contributor

Which issue does this PR close?

Closes #13020.

Rationale for this change

Unit tests in log.rs do not cover unary case and failure case. Also added explicit casting to slt tests for f32 and f64 with log(x) and log(x,y) to cover all cases

What changes are included in this PR?

-added tests

Are these changes tested?

changes are only tests and all tests pass

Are there any user-facing changes?

no

@github-actions github-actions bot added sqllogictest SQL Logic Tests (.slt) functions labels Oct 21, 2024
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks like an improvement to me -- thank you @buraksenn

@berkaysynnada
Copy link
Contributor

It seems that these tests don't cover the intended case yet. Can you find a unit or slt test failing with the change 58d3d86

@buraksenn
Copy link
Contributor Author

buraksenn commented Oct 22, 2024

It seems that these tests don't cover the intended case yet. Can you find a unit or slt test failing with the change 58d3d86

Yes @berkaysynnada , scalar tests were missing. I've also added them

@berkaysynnada
Copy link
Contributor

LGTM, thank you @buraksenn

@berkaysynnada berkaysynnada merged commit 521966a into apache:main Oct 23, 2024
24 checks passed
@berkaysynnada berkaysynnada deleted the extend-log-rs-scalar-test branch October 23, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
functions sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate why log.rs bug was not caught by tests
3 participants