You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2018/08/01 05:52:25 UTC

[GitHub] spark pull request #19449: [SPARK-22219][SQL] Refactor code to get a value f...

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

    https://github.com/apache/spark/pull/19449#discussion_r206760031
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/internal/ExecutorSideSQLConfSuite.scala ---
    @@ -82,4 +84,22 @@ class ExecutorSideSQLConfSuite extends SparkFunSuite with SQLTestUtils {
           assert(checks.forall(_ == true))
         }
       }
    +
    +  test("SPARK-22219: refactor to control to generate comment") {
    +    withSQLConf(StaticSQLConf.CODEGEN_COMMENTS.key -> "false") {
    +      val res = codegenStringSeq(spark.range(10).groupBy(col("id") * 2).count()
    +        .queryExecution.executedPlan)
    +      assert(res.length == 2)
    +      assert(res.forall{ case (_, code) =>
    +        !code.contains("* Codegend pipeline") && !code.contains("// input[")})
    +    }
    +
    +    withSQLConf(StaticSQLConf.CODEGEN_COMMENTS.key -> "true") {
    +      val res = codegenStringSeq(spark.range(10).groupBy(col("id") * 2).count()
    +        .queryExecution.executedPlan)
    +      assert(res.length == 2)
    +      assert(res.forall{ case (_, code) =>
    +        code.contains("* Codegend pipeline") && code.contains("// input[")})
    +    }
    --- End diff --
    
    combine these two?
    ```
    Seq(true, false).foreach { flag =>
      ...
      if (flag) {
         ...
      } else {
        ...
      }
    }
    ```


---

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