You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/08/12 21:10:08 UTC
[GitHub] [spark] kazuyukitanimura opened a new pull request, #37500: [SPARK-40049][SQL][TESTS] Add adaptive plan case in ReplaceNullWithFalseInPredicateEndToEndSuite
kazuyukitanimura opened a new pull request, #37500:
URL: https://github.com/apache/spark/pull/37500
### What changes were proposed in this pull request?
This PR proposes to add test cases to `ReplaceNullWithFalseInPredicateEndToEndSuite` with AQE (Adaptive Query Execution) turned on.
### Why are the changes needed?
Currently `ReplaceNullWithFalseInPredicateEndToEndSuite` assumes that AQE is always turned off. We should also test with AQE turned on
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Added both with and without AQE tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on pull request #37500: [SPARK-40049][SQL][TESTS] Add ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #37500:
URL: https://github.com/apache/spark/pull/37500#issuecomment-1213627289
Merged to master. Thank you, @kazuyukitanimura , @sunchao .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #37500: [SPARK-40049][SQL][TESTS] Add adaptive plan case in ReplaceNullWithFalseInPredicateEndToEndSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #37500:
URL: https://github.com/apache/spark/pull/37500#discussion_r944927644
##########
sql/core/src/test/scala/org/apache/spark/sql/ReplaceNullWithFalseInPredicateEndToEndSuite.scala:
##########
@@ -19,16 +19,25 @@ package org.apache.spark.sql
import org.apache.spark.sql.catalyst.expressions.{CaseWhen, If, Literal}
import org.apache.spark.sql.execution.LocalTableScanExec
+import org.apache.spark.sql.execution.adaptive.{
+ AdaptiveSparkPlanHelper,
+ DisableAdaptiveExecutionSuite,
+ EnableAdaptiveExecutionSuite
+}
Review Comment:
Hi, @kazuyukitanimura . This is not Apache Spark style. Please match with the code around your patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kazuyukitanimura commented on pull request #37500: [SPARK-40049][SQL][TESTS] Add adaptive plan case in ReplaceNullWithFalseInPredicateEndToEndSuite
Posted by GitBox <gi...@apache.org>.
kazuyukitanimura commented on PR #37500:
URL: https://github.com/apache/spark/pull/37500#issuecomment-1213525547
cc @aokolnychyi @wangyum @sunchao
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #37500: [SPARK-40049][SQL][TESTS] Add adaptive plan case in ReplaceNullWithFalseInPredicateEndToEndSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #37500:
URL: https://github.com/apache/spark/pull/37500#discussion_r944929632
##########
sql/core/src/test/scala/org/apache/spark/sql/ReplaceNullWithFalseInPredicateEndToEndSuite.scala:
##########
@@ -19,16 +19,25 @@ package org.apache.spark.sql
import org.apache.spark.sql.catalyst.expressions.{CaseWhen, If, Literal}
import org.apache.spark.sql.execution.LocalTableScanExec
+import org.apache.spark.sql.execution.adaptive.{
+ AdaptiveSparkPlanHelper,
+ DisableAdaptiveExecutionSuite,
+ EnableAdaptiveExecutionSuite
+}
import org.apache.spark.sql.functions.{lit, when}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.test.SharedSparkSession
import org.apache.spark.sql.types.BooleanType
-class ReplaceNullWithFalseInPredicateEndToEndSuite extends QueryTest with SharedSparkSession {
+class ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite extends
+ ReplaceNullWithFalseInPredicateEndToEndSuite with EnableAdaptiveExecutionSuite
Review Comment:
Please move this class to the end of this file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #37500: [SPARK-40049][SQL][TESTS] Add ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #37500:
URL: https://github.com/apache/spark/pull/37500#issuecomment-1225085428
Hey, are we going to update every test suite to run with both AQE on and off? I don't think it's worthwhile since AQE is already on by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #37500: [SPARK-40049][SQL][TESTS] Add adaptive plan case in ReplaceNullWithFalseInPredicateEndToEndSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #37500:
URL: https://github.com/apache/spark/pull/37500#discussion_r944928418
##########
sql/core/src/test/scala/org/apache/spark/sql/ReplaceNullWithFalseInPredicateEndToEndSuite.scala:
##########
@@ -19,16 +19,25 @@ package org.apache.spark.sql
import org.apache.spark.sql.catalyst.expressions.{CaseWhen, If, Literal}
import org.apache.spark.sql.execution.LocalTableScanExec
+import org.apache.spark.sql.execution.adaptive.{
+ AdaptiveSparkPlanHelper,
+ DisableAdaptiveExecutionSuite,
+ EnableAdaptiveExecutionSuite
+}
Review Comment:
I'm wondering if this came from IDE setting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kazuyukitanimura commented on pull request #37500: [SPARK-40049][SQL][TESTS] Add ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite
Posted by GitBox <gi...@apache.org>.
kazuyukitanimura commented on PR #37500:
URL: https://github.com/apache/spark/pull/37500#issuecomment-1225161845
Yes AQE is on by default, however `spark.sql.adaptive.forceApply` is off by default. Enabling force apply changes the execution plan. Perhaps I should have been clear for this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kazuyukitanimura commented on a diff in pull request #37500: [SPARK-40049][SQL][TESTS] Add ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite
Posted by GitBox <gi...@apache.org>.
kazuyukitanimura commented on code in PR #37500:
URL: https://github.com/apache/spark/pull/37500#discussion_r944935725
##########
sql/core/src/test/scala/org/apache/spark/sql/ReplaceNullWithFalseInPredicateEndToEndSuite.scala:
##########
@@ -19,16 +19,25 @@ package org.apache.spark.sql
import org.apache.spark.sql.catalyst.expressions.{CaseWhen, If, Literal}
import org.apache.spark.sql.execution.LocalTableScanExec
+import org.apache.spark.sql.execution.adaptive.{
+ AdaptiveSparkPlanHelper,
+ DisableAdaptiveExecutionSuite,
+ EnableAdaptiveExecutionSuite
+}
Review Comment:
Updated
##########
sql/core/src/test/scala/org/apache/spark/sql/ReplaceNullWithFalseInPredicateEndToEndSuite.scala:
##########
@@ -19,16 +19,25 @@ package org.apache.spark.sql
import org.apache.spark.sql.catalyst.expressions.{CaseWhen, If, Literal}
import org.apache.spark.sql.execution.LocalTableScanExec
+import org.apache.spark.sql.execution.adaptive.{
+ AdaptiveSparkPlanHelper,
+ DisableAdaptiveExecutionSuite,
+ EnableAdaptiveExecutionSuite
+}
import org.apache.spark.sql.functions.{lit, when}
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.test.SharedSparkSession
import org.apache.spark.sql.types.BooleanType
-class ReplaceNullWithFalseInPredicateEndToEndSuite extends QueryTest with SharedSparkSession {
+class ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite extends
+ ReplaceNullWithFalseInPredicateEndToEndSuite with EnableAdaptiveExecutionSuite
Review Comment:
Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #37500: [SPARK-40049][SQL][TESTS] Add ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #37500: [SPARK-40049][SQL][TESTS] Add ReplaceNullWithFalseInPredicateWithAQEEndToEndSuite
URL: https://github.com/apache/spark/pull/37500
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org