chore: Create simple fuzz test as part of test suite by andygrove · Pull Request #1610 · apache/datafusion-comet

andygrove · 2025-04-04T18:43:48Z

Which issue does this PR close?

Part of #1373
Closes #1289
Closes #1613
Closes #1611 (actually, I can no longer reproduce this issue)

Rationale for this change

This is a starting point for fuzz testing as part of our standard test suite.

What changes are included in this PR?

How are these changes tested?

andygrove · 2025-04-04T21:23:57Z

common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala

          _: BigIntVector | _: Float4Vector | _: Float8Vector | _: VarCharVector |
          _: DecimalVector | _: DateDayVector | _: TimeStampMicroTZVector | _: VarBinaryVector |
-          _: FixedSizeBinaryVector | _: TimeStampMicroVector | _: StructVector) =>
+          _: FixedSizeBinaryVector | _: TimeStampMicroVector | _: StructVector | _: ListVector) =>


This is the fix for #1289

Nice, need to include MapType as part of #1603

We could add MapVector here as well without any ill effects.

codecov-commenter · 2025-04-04T23:21:30Z

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.97%. Comparing base (f09f8af) to head (bf25dcb).
⚠️ Report is 834 commits behind head on main.

Files with missing lines	Patch %	Lines
...la/org/apache/comet/testing/ParquetGenerator.scala	85.71%	0 Missing and 2 partials ⚠️
.../scala/org/apache/spark/sql/comet/util/Utils.scala	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1610      +/-   ##
============================================
+ Coverage     56.12%   58.97%   +2.84%     
- Complexity      976     1074      +98     
============================================
  Files           119      125       +6     
  Lines         11743    12522     +779     
  Branches       2251     2339      +88     
============================================
+ Hits           6591     7385     +794     
+ Misses         4012     3971      -41     
- Partials       1140     1166      +26

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

comphead

thanks @andygrove

andygrove · 2025-04-05T16:41:04Z

docs/source/user-guide/installation.md

 Make sure `SPARK_HOME` points to the same Spark version as Comet was built for.

-```console
+```shell


This fixes an issue where previously it was not possible to copy and paste the $ in $SPARK_HOME when copying from the published documentation.

andygrove · 2025-04-05T18:12:48Z

spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala

+        if (groupingExprs.exists(_.isEmpty)) {
+          withInfo(op, "Not all grouping expressions are supported")
+          return None
+        }


This fixes #1613

andygrove · 2025-04-07T14:08:18Z

@parthchandra @mbutrovich could you review?

mbutrovich · 2025-04-07T15:01:09Z

spark/src/main/scala/org/apache/comet/testing/ParquetGenerator.scala

        Range(0, numRows).map(_ => new java.sql.Date(1716645600011L + r.nextInt()))
      case DataTypes.TimestampType =>
        Range(0, numRows).map(_ => new Timestamp(1716645600011L + r.nextInt()))
+      case DataTypes.TimestampNTZType =>


Do we know what this magic number is? If so could we define it as a variable?

Updated. It was approximately the current date when I worked on this code. I added a variable with comments.

Or you could just use SimpleDateFormat("YYYY-MM-DD hh:mm:ss").parse("1980-12-08 23:15:00") ?

mbutrovich · 2025-04-07T15:02:35Z

spark/src/test/scala/org/apache/comet/CometFuzzTestSuite.scala

+    val random = new Random(42)
+    withSQLConf(
+      CometConf.COMET_ENABLED.key -> "false",
+      SQLConf.SESSION_LOCAL_TIMEZONE.key -> "Asia/Kathmandu" /* UTC+5:45 */ ) {


We should document that his is a non-arbitrary timezone choice. I know @parthchandra said it's a tricky timezone due to the unique offset from UTC.

And also makes a great trivia question :)

parthchandra

lgtm. Non binding comments.

parthchandra · 2025-04-07T23:30:48Z

spark/src/main/scala/org/apache/comet/testing/ParquetGenerator.scala

        Range(0, numRows).map(_ => new java.sql.Date(1716645600011L + r.nextInt()))
      case DataTypes.TimestampType =>
        Range(0, numRows).map(_ => new Timestamp(1716645600011L + r.nextInt()))
+      case DataTypes.TimestampNTZType =>


Or you could just use SimpleDateFormat("YYYY-MM-DD hh:mm:ss").parse("1980-12-08 23:15:00") ?

parthchandra · 2025-04-07T23:34:23Z

common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala

          _: BigIntVector | _: Float4Vector | _: Float8Vector | _: VarCharVector |
          _: DecimalVector | _: DateDayVector | _: TimeStampMicroTZVector | _: VarBinaryVector |
-          _: FixedSizeBinaryVector | _: TimeStampMicroVector | _: StructVector) =>
+          _: FixedSizeBinaryVector | _: TimeStampMicroVector | _: StructVector | _: ListVector) =>


We could add MapVector here as well without any ill effects.

parthchandra · 2025-04-07T23:35:00Z

spark/src/test/scala/org/apache/comet/CometFuzzTestSuite.scala

+    val random = new Random(42)
+    withSQLConf(
+      CometConf.COMET_ENABLED.key -> "false",
+      SQLConf.SESSION_LOCAL_TIMEZONE.key -> "Asia/Kathmandu" /* UTC+5:45 */ ) {


And also makes a great trivia question :)

andygrove added 3 commits April 4, 2025 12:43

create simple fuzz test

d9499bf

fall back for complex type grouping expr

5475b63

add shuffle mode dimension

7d4ad1f

This was referenced Apr 4, 2025

Unsupported Arrow Vector for export: class org.apache.arrow.vector.complex.ListVector #1289

Closed

[complex types] Unsupported data type org.apache.spark.sql.Row #1611

Closed

andygrove added 5 commits April 4, 2025 13:55

Save

4bb9932

Save

ce85a12

Save

c57ba4f

fix ListVector

fd69e78

fix

c1f6869

andygrove commented Apr 4, 2025

View reviewed changes

andygrove added 3 commits April 4, 2025 15:43

fix

ff263a3

fix

4826cde

join test

30f3aa7

andygrove marked this pull request as ready for review April 4, 2025 21:54

andygrove requested review from comphead and parthchandra April 4, 2025 21:55

more

31603b7

comphead approved these changes Apr 4, 2025

View reviewed changes

comphead mentioned this pull request Apr 4, 2025

feat: add MAP type support for first level #1603

Merged

improve tests

cf4bfba

andygrove commented Apr 5, 2025

View reviewed changes

andygrove added 2 commits April 5, 2025 11:23

improve tests

6063ad9

fix for apache#1613

2695f9c

andygrove commented Apr 5, 2025

View reviewed changes

mbutrovich reviewed Apr 7, 2025

View reviewed changes

address feedback

e509284

parthchandra approved these changes Apr 7, 2025

View reviewed changes

address feedback

bf25dcb

andygrove merged commit 4b577f8 into apache:main Apr 8, 2025
78 checks passed

andygrove deleted the fuzz-complex-types branch April 8, 2025 12:43

coderfender pushed a commit to coderfender/datafusion-comet that referenced this pull request Dec 13, 2025

chore: Create simple fuzz test as part of test suite (apache#1610)

88c6f95

Conversation

andygrove commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

andygrove Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andygrove commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parthchandra left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andygrove commented Apr 4, 2025 •

edited

Loading

codecov-commenter commented Apr 4, 2025 •

edited

Loading

andygrove Apr 5, 2025 •

edited

Loading