Feat: Add CometLocalTableScanExec operator #2735
Conversation
|
Very cool! Kicking off CI. |
| case op: LocalTableScanExec if CometConf.COMET_EXEC_LOCAL_TABLE_SCAN_ENABLED.get(conf) => | ||
| val nativeOp = QueryPlanSerde.operator2Proto(op) | ||
| val cometOp = CometLocalTableScanExec(op, op.rows, op.output) | ||
| CometScanWrapper(nativeOp.get, cometOp) | ||
|
|
||
| case op: LocalTableScanExec if !CometConf.COMET_EXEC_LOCAL_TABLE_SCAN_ENABLED.get(conf) => | ||
| withInfo(op, "LocalTableScan is not enabled") |
There was a problem hiding this comment.
These could be combined into a single match arm.
| case op: LocalTableScanExec if CometConf.COMET_EXEC_LOCAL_TABLE_SCAN_ENABLED.get(conf) => | |
| val nativeOp = QueryPlanSerde.operator2Proto(op) | |
| val cometOp = CometLocalTableScanExec(op, op.rows, op.output) | |
| CometScanWrapper(nativeOp.get, cometOp) | |
| case op: LocalTableScanExec if !CometConf.COMET_EXEC_LOCAL_TABLE_SCAN_ENABLED.get(conf) => | |
| withInfo(op, "LocalTableScan is not enabled") | |
| case op: LocalTableScanExec => | |
| if (CometConf.COMET_EXEC_LOCAL_TABLE_SCAN_ENABLED.get(conf)) { | |
| val nativeOp = QueryPlanSerde.operator2Proto(op) | |
| val cometOp = CometLocalTableScanExec(op, op.rows, op.output) | |
| CometScanWrapper(nativeOp.get, cometOp) | |
| } else { | |
| withInfo(op, "LocalTableScan is not enabled") | |
| } |
There was a problem hiding this comment.
Thanks, fixed.
|
This is looking great. I suspect there may be some failures in the Spark SQL test suite (see #2714 where I enabled converting LocalTableScan to columnar), so we may want to disable this by default until we have addressed the reasons for those failures. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2735 +/- ##
============================================
+ Coverage 56.12% 58.58% +2.45%
- Complexity 976 1475 +499
============================================
Files 119 161 +42
Lines 11743 13964 +2221
Branches 2251 2383 +132
============================================
+ Hits 6591 8181 +1590
- Misses 4012 4573 +561
- Partials 1140 1210 +70 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks @andygrove! I have set COMET_EXEC_LOCAL_TABLE_SCAN_ENABLED to false as the default |
spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala
Outdated
Show resolved
Hide resolved
|
Thanks @mbutrovich @andygrove for the review |
mbutrovich
left a comment
mbutrovich
left a comment
There was a problem hiding this comment.
This is a great addition, thank you @kazantsev-maksim!
4 participants
Which issue does this PR close?
Closes #2513
Rationale for this change
#2513
What changes are included in this PR?
LocalTableScanExec operator implementation has been added:
How are these changes tested?
Added new unit test