-
Notifications
You must be signed in to change notification settings - Fork 205
chore: Enable CometFuzzTestSuite int96 test for experimental native scans (without complex types) #1664
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
…ut without complex types.
@@ -206,10 +206,15 @@ class CometFuzzTestSuite extends CometTestBase with AdaptiveSparkPlanHelper { | |||
} | |||
|
|||
private def testParquetTemporalTypes( | |||
outputTimestampType: ParquetOutputTimestampType.Value): Unit = { | |||
outputTimestampType: ParquetOutputTimestampType.Value, | |||
generateArray: Boolean = true, |
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.
should we also generate maps?
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.
It would be good to start testing maps as well in the fuzz suite. Some of the existing tests cannot work with map types though (because maps don't implementing ordering). We may need to add new map-specific tests.
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.
For this PR I'd just like to match existing functionality. We should open an issue to add map coverage to the CometFuzzTestSuite.
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.
I filed an issue: #1665
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.
Filed #1666
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1664 +/- ##
============================================
+ Coverage 56.12% 58.81% +2.68%
- Complexity 976 1082 +106
============================================
Files 119 125 +6
Lines 11743 12592 +849
Branches 2251 2362 +111
============================================
+ Hits 6591 7406 +815
- Misses 4012 4015 +3
- Partials 1140 1171 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Which issue does this PR close?
N/A
Rationale for this change
#1652 added better int96 support for experimental native scans (relying on CometCastSuite and other Parquet tests) but did not enable CometFuzzTestSuite because it uses complex types by default. There's an issue in DF's schema coercion for complex types.
What changes are included in this PR?
This PR modifies the test to get coverage with the int96 data type without complex types.
How are these changes tested?
New test coverage.