test: enable ignored 4.0 tests, enable ansi mode#3454
test: enable ignored 4.0 tests, enable ansi mode#3454parthchandra merged 4 commits intoapache:mainfrom
Conversation
parthchandra
changed the title
parthchandra
force-pushed
the
spark-4.0-support
branch
2 times, most recently
from
eedc50a to
e08426c
Compare
parthchandra
changed the title
| + */ | ||
| + protected def enableCometAnsiMode: Boolean = { | ||
| + val v = System.getenv("ENABLE_COMET_ANSI_MODE") | ||
| + v != null && v.toBoolean | ||
| + if (v != null) v.toBoolean else true | ||
| + } |
There was a problem hiding this comment.
This method can be removed. We no longer have a config in Comet for this.
There was a problem hiding this comment.
(of course, can be done in a separate PR)
There was a problem hiding this comment.
Actually, nm, it looks like this is for enabling Spark ANSI mode, not Comet ANSI mode, so maybe the method name and env var name are just confusing. I will take another look at this tomorrow.
There was a problem hiding this comment.
Spark 4 Ansi mode is the default. This may be leftover from 3.5.
kazuyukitanimura
left a comment
kazuyukitanimura
left a comment
There was a problem hiding this comment.
Thank you @parthchandra
|
@kazuyukitanimura @andygrove thank you for looking at this PR. I found an issue with this diff. Let me change this to draft. |
parthchandra
force-pushed
the
spark-4.0-support
branch
from
513946c to
6c4ba5c
Compare
|
@andygrove @kazuyukitanimura this is ready for review again. |
| - val aqePlanRoot = findNodeInSparkPlanInfo(inMemoryScanNode.get, | ||
| - _.nodeName.contains("ResultQueryStage")) | ||
| - aqePlanRoot.get.children.head.nodeName == "AQEShuffleRead" | ||
| + aqeNode.get.children.head.nodeName == "AQEShuffleRead" || | ||
| + (aqeNode.get.children.head.nodeName.contains("WholeStageCodegen") && | ||
| + aqeNode.get.children.head.children.head.nodeName == "ColumnarToRow" && | ||
| + aqeNode.get.children.head.children.head.children.head.nodeName == "InputAdapter" && | ||
| + aqeNode.get.children.head.children.head.children.head.children.head.nodeName == | ||
| + "AQEShuffleRead") | ||
| + // Spark 4.0 wraps results in ResultQueryStage. The coalescing indicator is AQEShuffleRead | ||
| + // as the direct child of InputAdapter. | ||
| + // AdaptiveSparkPlan -> ResultQueryStage -> WholestageCodegen -> | ||
| + // CometColumnarToRow -> InputAdapter -> AQEShuffleRead (if coalesced) | ||
| + val resultStage = aqeNode.get.children.head // ResultQueryStage | ||
| + val wsc = resultStage.children.head // WholeStageCodegen | ||
| + val c2r = wsc.children.head // ColumnarToRow or CometColumnarToRow | ||
| + val inputAdapter = c2r.children.head // InputAdapter | ||
| + inputAdapter.children.head.nodeName == "AQEShuffleRead" |
There was a problem hiding this comment.
This new test is not equivalent because some of the nodeName tests are skipped.
Should the change be something like
...
(aqeNode.get.children.head.children.head.nodeName == "ColumnarToRow" ||
aqeNode.get.children.head.children.head.nodeName == "CometColumnarToRow") &&
...
There was a problem hiding this comment.
Added the check for the node names. Apart from the CometColumnarToRow, the test was failing because there is an additional ResultQuerryStage. in Spark 4.0
kazuyukitanimura
left a comment
kazuyukitanimura
left a comment
There was a problem hiding this comment.
Thank you @parthchandra
|
Merged. Thank you @andygrove @kazuyukitanimura |
3 participants
enabling tests that fail in 4.0, and with ansi mode enabled to see what still fails