-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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
[SPARK-40850][SQL] Fix test case interpreted queries may execute Codegen #41467
[SPARK-40850][SQL] Fix test case interpreted queries may execute Codegen #41467
Conversation
cc @holdenk |
@Hisoka-X How about other places, like: spark/sql/core/src/test/scala/org/apache/spark/sql/errors/QueryExecutionErrorsSuite.scala Line 81 in f26bdb7
Could you re-check them too, please ... or open an JIRA if you don't have time. |
+1, LGTM. Merging to master. |
Thanks @MaxGekk |
Ok, I will do it. |
…gen.factoryMode` to NO_CODEGEN ### What changes were proposed in this pull request? After #41467 , we fix the `CodegenInterpretedPlanTest ` will execute codeGen even set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`. Before this PR, `spark.sql.codegen.factoryMode` can't disable WholeStageCodegen, many test case want to disable codegen by set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`, but it not work for WholeStageCodegen. So this PR change the `spark.sql.codegen.factoryMode` behavior, when set `NO_CODEGEN`, we will disable `WholeStageCodegen` too. ### Why are the changes needed? Fix the `spark.sql.codegen.factoryMode` config behavior. ### Does this PR introduce _any_ user-facing change? Yes, the config logic changed. ### How was this patch tested? add new test. Closes #41779 from Hisoka-X/SPARK-44236_wholecodegen_disable. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…gen.factoryMode` to NO_CODEGEN ### What changes were proposed in this pull request? After #41467 , we fix the `CodegenInterpretedPlanTest ` will execute codeGen even set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`. Before this PR, `spark.sql.codegen.factoryMode` can't disable WholeStageCodegen, many test case want to disable codegen by set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`, but it not work for WholeStageCodegen. So this PR change the `spark.sql.codegen.factoryMode` behavior, when set `NO_CODEGEN`, we will disable `WholeStageCodegen` too. ### Why are the changes needed? Fix the `spark.sql.codegen.factoryMode` config behavior. ### Does this PR introduce _any_ user-facing change? Yes, the config logic changed. ### How was this patch tested? add new test. Closes #41779 from Hisoka-X/SPARK-44236_wholecodegen_disable. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 74fa07c) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…gen.factoryMode` to NO_CODEGEN ### What changes were proposed in this pull request? After apache#41467 , we fix the `CodegenInterpretedPlanTest ` will execute codeGen even set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`. Before this PR, `spark.sql.codegen.factoryMode` can't disable WholeStageCodegen, many test case want to disable codegen by set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`, but it not work for WholeStageCodegen. So this PR change the `spark.sql.codegen.factoryMode` behavior, when set `NO_CODEGEN`, we will disable `WholeStageCodegen` too. ### Why are the changes needed? Fix the `spark.sql.codegen.factoryMode` config behavior. ### Does this PR introduce _any_ user-facing change? Yes, the config logic changed. ### How was this patch tested? add new test. Closes apache#41779 from Hisoka-X/SPARK-44236_wholecodegen_disable. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…gen.factoryMode` to NO_CODEGEN ### What changes were proposed in this pull request? After apache#41467 , we fix the `CodegenInterpretedPlanTest ` will execute codeGen even set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`. Before this PR, `spark.sql.codegen.factoryMode` can't disable WholeStageCodegen, many test case want to disable codegen by set `spark.sql.codegen.factoryMode` to `NO_CODEGEN`, but it not work for WholeStageCodegen. So this PR change the `spark.sql.codegen.factoryMode` behavior, when set `NO_CODEGEN`, we will disable `WholeStageCodegen` too. ### Why are the changes needed? Fix the `spark.sql.codegen.factoryMode` config behavior. ### Does this PR introduce _any_ user-facing change? Yes, the config logic changed. ### How was this patch tested? add new test. Closes apache#41779 from Hisoka-X/SPARK-44236_wholecodegen_disable. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
What changes were proposed in this pull request?
Fix
CodegenInterpretedPlanTest
always will execute Codegen even setspark.sql.codegen.factoryMode
toNO_CODEGEN
. We should setspark.sql.codegen.wholeStage
tofalse
too.Why are the changes needed?
Fix test case
Does this PR introduce any user-facing change?
No
How was this patch tested?
Add breakpoint on
WholeStageCodegenExec::doConsume
. Even setspark.sql.codegen.factoryMode
toNO_CODEGEN
, theWholeStageCodegenExec::doConsume
also will be executed. When setspark.sql.codegen.wholeStage
tofalse
, the logic will skip.