[GitHub] [spark] dongjoon-hyun commented on pull request #31766: [SPARK-34596][SPARK-34607][SQL][FOLLOWUP][TESTS] Add `PlanTest. testFallback` and use it

[GitBox <gi...@apache.org>]

dongjoon-hyun commented on pull request #31766:
URL: https://github.com/apache/spark/pull/31766#issuecomment-795018697


   Hi, @rednaxelafx , @cloud-fan , @viirya , @kiszk , @maropu .
   
   Here is a summary of the progress.
   1. https://github.com/apache/spark/pull/31775 (SPARK-34660 `Don't use ParVector with withExistingConf which is not thread-safe`) fixed the UT failure at `branch-3.1` last weekend.
   2. https://github.com/apache/spark/pull/31785 (SPARK-33498 `Remove SQLConf.withExistingConf in CastSuite`) removes the unneeded `withExistingConf` at `master` and will be backported to branch-3.1.
   
   So, the above two were the answer why we hit the UT failure only at `branch-3.1` while we have the patch at `master/3.1/3.0`.
   
   Now, this PR aims to do **only refactoring PlanTest and test cases to have a clear test design**. As we discussed in the previous PR, we need this.
   
   - https://github.com/apache/spark/pull/31764#discussion_r589170423 (@cloud-fan )
   > The current test framework assumes that codegen and non-codegen should have consistent behaviors, while this codegen bug breaks the assumption. The test case fails with codegen but passes with non-codegen.
   > 
   > +1 to add this for such test cases. One thing I'm curious about is why this test works in master...
   
   - https://github.com/apache/spark/pull/31764#discussion_r589179603 (@maropu )
   > +1 to add it, too.
   
   I updated this PR to the master and address the two comments.
   - https://github.com/apache/spark/pull/31764#discussion_r589179924
   - https://github.com/apache/spark/pull/31764#discussion_r589182718
   
   So, could you review this again freshly? And, please let me know if I missed some of your comments, (especially @viirya 's one).


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

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