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/02/14 07:30:15 UTC

[GitHub] [spark] LuciferYang opened a new pull request #35506: [SPARK-38198][SQL] Fix `QueryExecution.debug#toFile` use the passed in `maxFields` when `explainMode` is `CodegenMode`

LuciferYang opened a new pull request #35506:
URL: https://github.com/apache/spark/pull/35506


   ### What changes were proposed in this pull request?
   `QueryExecution.debug#toFile` method supports passing in `maxFields` and this parameter will be passed down when `explainMode` is `SimpleMode`, `ExtendedMode`, or `CostMode`.   But the passed down `maxFields` was ignored when `explainMode` is `CostMode` because `QueryExecution#stringWithStats` overrides it with `SQLConf.get.maxToStringFields` at present,  so this pr removes the override behavior to passed in `maxFields` take effect.
   
   
   ### Why are the changes needed?
   Bug fix
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   Pass GA and add a new test case
   


-- 
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] LuciferYang commented on pull request #35506: [SPARK-38198][SQL] Fix `QueryExecution.debug#toFile` use the passed in `maxFields` when `explainMode` is `CodegenMode`

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35506:
URL: https://github.com/apache/spark/pull/35506#issuecomment-1039057664


   > @LuciferYang Just in case, does branch-3.2 have the same issue? If so, please, backport the fix.
   
   OK ~


-- 
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] LuciferYang commented on a change in pull request #35506: [SPARK-38198][SQL] Fix `QueryExecution.debug#toFile` use the passed in `maxFields` when `explainMode` is `CodegenMode`

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #35506:
URL: https://github.com/apache/spark/pull/35506#discussion_r805569380



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala
##########
@@ -261,4 +261,22 @@ class QueryExecutionSuite extends SharedSparkSession {
     val cmdResultExec = projectQe.executedPlan.asInstanceOf[CommandResultExec]
     assert(cmdResultExec.commandPhysicalPlan.isInstanceOf[ShowTablesExec])
   }
+
+  test("SPARK-38198: check specify maxFields when call toFile method") {

Review comment:
       Before this pr, the test will failed as follows:
   ```
   tableScan.exists(((x$8: String) => x$8.contains("more fields"))) was false Specify maxFields = 3 doesn't take effect when explainMode is cost
   ScalaTestFailureLocation: org.apache.spark.sql.execution.QueryExecutionSuite at (QueryExecutionSuite.scala:162)
   org.scalatest.exceptions.TestFailedException: tableScan.exists(((x$8: String) => x$8.contains("more fields"))) was false Specify maxFields = 3 doesn't take effect when explainMode is cost
   ```




-- 
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] MaxGekk commented on pull request #35506: [SPARK-38198][SQL] Fix `QueryExecution.debug#toFile` use the passed in `maxFields` when `explainMode` is `CodegenMode`

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #35506:
URL: https://github.com/apache/spark/pull/35506#issuecomment-1038917193


   @LuciferYang Just in case, does branch-3.2 have the same issue? If so, please, backport the fix. 


-- 
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] MaxGekk closed pull request #35506: [SPARK-38198][SQL] Fix `QueryExecution.debug#toFile` use the passed in `maxFields` when `explainMode` is `CodegenMode`

Posted by GitBox <gi...@apache.org>.
MaxGekk closed pull request #35506:
URL: https://github.com/apache/spark/pull/35506


   


-- 
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] LuciferYang commented on pull request #35506: [SPARK-38198][SQL] Fix `QueryExecution.debug#toFile` use the passed in `maxFields` when `explainMode` is `CodegenMode`

Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #35506:
URL: https://github.com/apache/spark/pull/35506#issuecomment-1038916548


   thanks @MaxGekk 


-- 
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] MaxGekk commented on pull request #35506: [SPARK-38198][SQL] Fix `QueryExecution.debug#toFile` use the passed in `maxFields` when `explainMode` is `CodegenMode`

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #35506:
URL: https://github.com/apache/spark/pull/35506#issuecomment-1038914495


   +1, LGTM. Merging to master.
   Thank you, @LuciferYang .


-- 
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