feat: Add auto mode for COMET_PARQUET_SCAN_IMPL
#1747
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1747 +/- ##
============================================
+ Coverage 56.12% 58.59% +2.46%
- Complexity 976 1133 +157
============================================
Files 119 130 +11
Lines 11743 12672 +929
Branches 2251 2365 +114
============================================
+ Hits 6591 7425 +834
- Misses 4012 4061 +49
- Partials 1140 1186 +46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Looking good so far. Will do a final review once it is ready. |
| # Compatibility Guide | ||
|
|
||
| Comet aims to provide consistent results with the version of Apache Spark that is being used. | ||
|
|
||
| This guide offers information about areas of functionality where there are known differences. | ||
|
|
There was a problem hiding this comment.
This section appeared twice
|
@parthchandra @mbutrovich This is ready for review now. I don't know if we want to keep in draft until more complete or merge and iterate. I also did not make auto the default yet. |
| if (COMET_EXEC_ENABLED | ||
| .get() && schemaSupported && partitionSchemaSupported && | ||
| !scanExec.bucketedScan && !knownIssues) { | ||
| scanImpl = SCAN_NATIVE_ICEBERG_COMPAT |
There was a problem hiding this comment.
native_iceberg_compat should be able to handle bucketed scans
| } | ||
|
|
||
| if (scanImpl == SCAN_AUTO) { | ||
| scanImpl = SCAN_NATIVE_COMET |
There was a problem hiding this comment.
We would never choose native_datafusion?
|
Not sure why this would cause the ci failures that we see here. Maybe defer this until some more of the known issues are fixed? |
Which issue does this PR close?
N/A
Follows on from #1746
Rationale for this change
Rather than require the user to manually configure the scan implementation, let's add an "auto" option so that we can just pick the best one based on whether the schema is supported.
What changes are included in this PR?
How are these changes tested?