Github user mallman commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22394#discussion_r217055207
  
    --- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetSchemaPruningSuite.scala
 ---
    @@ -245,28 +249,32 @@ class ParquetSchemaPruningSuite
         checkAnswer(query.orderBy("id"), Row(1) :: Nil)
       }
     
    -  private def testMixedCasePruning(testName: String)(testThunk: => Unit) {
    -    withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true",
    -      SQLConf.CASE_SENSITIVE.key -> "true") {
    -      test(s"Spark vectorized reader - case-sensitive parser - mixed-case 
schema - $testName") {
    -          withMixedCaseData(testThunk)
    +  private def testExactCasePruning(testName: String)(testThunk: => Unit) {
    +    test(s"Spark vectorized reader - case-sensitive parser - mixed-case 
schema - $testName") {
    +      withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true",
    +        SQLConf.CASE_SENSITIVE.key -> "true") {
    +        withMixedCaseData(testThunk)
           }
         }
    -    withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false",
    -      SQLConf.CASE_SENSITIVE.key -> "false") {
    -      test(s"Parquet-mr reader - case-insensitive parser - mixed-case 
schema - $testName") {
    +    test(s"Parquet-mr reader - case-sensitive parser - mixed-case schema - 
$testName") {
    +      withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false",
    +        SQLConf.CASE_SENSITIVE.key -> "true") {
             withMixedCaseData(testThunk)
           }
         }
    -    withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true",
    -      SQLConf.CASE_SENSITIVE.key -> "false") {
    -      test(s"Spark vectorized reader - case-insensitive parser - 
mixed-case schema - $testName") {
    -          withMixedCaseData(testThunk)
    +    testMixedCasePruning(testName)(testThunk)
    +  }
    +
    +  private def testMixedCasePruning(testName: String)(testThunk: => Unit) {
    --- End diff --
    
    Previously, this method ran `testThunk` with `SQLConf.CASE_SENSITIVE.key` 
set to true and false. That was a mistake and incorrect. For example, the query
    
        select col1, col2.b from mixedcase
    
    will fail if `SQLConf.CASE_SENSITIVE.key` is set to true. That mistake was 
causing 6 test cases to fail. Therefore, I moved the code that tests with a 
case-sensitive parser out of `testMixedCasePruning` into `testExactCasePruning` 
and included a call to `testMixedCasePruning` in `testExactCasePruning`.
    
    I'll push a commit that refactors the method names and add code comments 
that will make this clearer.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to