test: Follow up on Spark 3.4 diff#209
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
============================================
- Coverage 33.54% 33.54% -0.01%
- Complexity 769 770 +1
============================================
Files 107 107
Lines 35470 35517 +47
Branches 7723 7751 +28
============================================
+ Hits 11899 11913 +14
- Misses 21079 21100 +21
- Partials 2492 2504 +12 ☔ View full report in Codecov by Sentry. |
|
cc @viirya |
| new file mode 100644 | ||
| index 00000000000..4b31bea33de | ||
| --- /dev/null | ||
| +++ b/sql/core/src/test/scala/org/apache/spark/sql/IgnoreComet.scala |
There was a problem hiding this comment.
This is main change? DisableCometSuite -> IgnoreCometSuite?
| + op.asInstanceOf[CometExec].originalPlan.find(_.isInstanceOf[SortExec]).isDefined) | ||
| + }.isDefined == sortLeft, | ||
| + | ||
| + joinOperator.left.exists(op => op.isInstanceOf[ShuffleExchangeExec] || |
There was a problem hiding this comment.
For op.isInstanceOf[ShuffleExchangeExec], don't we need to check if it equals to shuffleLeft?
There was a problem hiding this comment.
hmm? it is checked in the code:
exists(op => op.isInstanceOf[ShuffleExchangeExec] || op.isInstanceOf[CometShuffleExchangeExec]) == shuffleLeft
| + op.isInstanceOf[SortExec] || | ||
| + (op.isInstanceOf[CometExec] && | ||
| + op.asInstanceOf[CometExec].originalPlan.find(_.isInstanceOf[SortExec]).isDefined) | ||
| + }.isDefined == sortLeft, |
There was a problem hiding this comment.
I forgot why we changed to check sort instead of shuffle here.
advancedxy
left a comment
advancedxy
left a comment
There was a problem hiding this comment.
LGTM.
Hmm, I remembered that I checked these parts.
|
Thanks, merged |
4 participants
Which issue does this PR close?
Closes #.
Rationale for this change
Certain changes to address comments from #166 were somehow excluded in the final commit. This adds them back.
What changes are included in this PR?
How are these changes tested?