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: Add fuzz testing for arithmetic expressions #519

Merged
merged 5 commits into from
Jun 7, 2024

Conversation

andygrove
Copy link
Member

Which issue does this PR close?

N/A

Rationale for this change

Increasing coverage of fuzz testing tool.

What changes are included in this PR?

Add fuzz testing for arithmetic expressions.

How are these changes tested?

Manually.

Example of compatibility issue found by these new tests:

SQL

SELECT c90, c1, c90 % c1 FROM test1 ORDER BY c90, c1;

First difference at row 31:
Spark: -21840,-0.0,NULL
Comet: -21840,-0.0,NaN

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.07%. Comparing base (7ba5693) to head (449cb92).
Report is 18 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #519      +/-   ##
============================================
+ Coverage     34.05%   34.07%   +0.01%     
+ Complexity      859      795      -64     
============================================
  Files           116      104      -12     
  Lines         38679    38470     -209     
  Branches       8567     8564       -3     
============================================
- Hits          13173    13107      -66     
+ Misses        22745    22606     -139     
+ Partials       2761     2757       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@huaxingao
Copy link
Contributor

Thanks @andygrove for the PR! For binary arithmetic expressions, shall we also include bitwise operation such as BitwiseAnd, BitwiseOr, BitwiseXor?

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

Looks good. We could add BitwiseAnd etc like @huaxingao mentioned.

Copy link
Contributor

@kazuyukitanimura kazuyukitanimura left a comment

Choose a reason for hiding this comment

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

+1 for adding more operations

@andygrove
Copy link
Member Author

Thanks for the review @huaxingao @kazuyukitanimura @viirya I have added the bitwise expressions.

@@ -106,4 +106,8 @@ object Meta {
Function("stddev_samp", 1),
Function("corr", 2))

val unaryArithmeticOps: Seq[String] = Seq("+", "-")

val binaryArithmeticOps: Seq[String] = Seq("+", "-", "*", "/", "%", "&", "|", "^")
Copy link
Contributor

Choose a reason for hiding this comment

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

any plans to support<, <=, ==, <=>, >, >=, =, ! ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thsi PR was focussing on arithmetic because it is related to the ongoing work to support ANSI mode, but yet, we should add tests for comparison operators as well

@andygrove andygrove merged commit c6d387c into apache:main Jun 7, 2024
43 checks passed
@andygrove andygrove deleted the fuzz-test-arithmetic branch June 7, 2024 05:12
himadripal pushed a commit to himadripal/datafusion-comet that referenced this pull request Sep 7, 2024
* Add fuzz tests for aritmetic expressions

* add unary math

* add bit-wise expressions

* bug fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants