Address review comments

Author: aokolnychyi

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala

@@ -383,12 +383,6 @@ case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with
     }
   }
 
-  override def sql: String = {
-    val valueSQL = child.sql
-    val listSQL = hset.toSeq.map(Literal(_).sql).mkString(", ")
-    s"($valueSQL IN ($listSQL))"
-  }
-
   private def canBeComputedUsingSwitch: Boolean = child.dataType match {
     case ByteType | ShortType | IntegerType | DateType => true
     case _ => false
@@ -409,6 +403,8 @@ case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with
     })
   }
 
+  // spark.sql.optimizer.inSetSwitchThreshold has an appropriate upper limit,
+  // so the code size should not exceed 64KB
   private def genCodeWithSwitch(ctx: CodegenContext, ev: ExprCode): ExprCode = {
     val caseValuesGen = hset.filter(_ != null).map(Literal(_).genCode(ctx))
     val valueGen = child.genCode(ctx)
@@ -427,13 +423,19 @@ case class InSet(child: Expression, hset: Set[Any]) extends UnaryExpression with
         ${CodeGenerator.JAVA_BOOLEAN} ${ev.value} = false;
         if (!${valueGen.isNull}) {
           switch (${valueGen.value}) {
-            ${caseBranches.mkString("")}
+            ${caseBranches.mkString("\n")}
             default:
               ${ev.isNull} = $hasNull;
           }
         }
        """)
   }
+
+  override def sql: String = {
+    val valueSQL = child.sql
+    val listSQL = hset.toSeq.map(Literal(_).sql).mkString(", ")
+    s"($valueSQL IN ($listSQL))"
+  }
 }
 
 @ExpressionDescription(

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

@@ -178,7 +178,7 @@ object SQLConf {
         "switch statements. This is applicable only to bytes, shorts, ints, dates.")
       .intConf
       .checkValue(threshold => threshold >= 0 && threshold <= 600, "The max set size " +
-        "for using switch statements in InSet must be positive and less than or equal to 600")
+        "for using switch statements in InSet must be non-negative and less than or equal to 600")
       .createWithDefault(400)
 
   val OPTIMIZER_PLAN_CHANGE_LOG_LEVEL = buildConf("spark.sql.optimizer.planChangeLog.level")

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/PredicateSuite.scala

@@ -242,7 +242,7 @@ class PredicateSuite extends SparkFunSuite with ExpressionEvalHelper {
     }
   }
 
-  test("SPARK-26205: Optimize InSet for bytes, shorts, ints, dates using switch statements") {
+  test("switch statements in InSet for bytes, shorts, ints, dates") {
     val byteValues = Set[Any](1.toByte, 2.toByte, Byte.MinValue, Byte.MaxValue)
     val shortValues = Set[Any](-10.toShort, 20.toShort, Short.MinValue, Short.MaxValue)
     val intValues = Set[Any](20, -100, 30, Int.MinValue, Int.MaxValue)

sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala

@@ -415,7 +415,7 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
       }
   }
 
-  test("SPARK-26205: Optimize InSet for bytes, shorts, ints, dates") {
+  test("IN/INSET with bytes, shorts, ints, dates") {
     def check(): Unit = {
       val values = Seq(
         (Byte.MinValue, Some(Short.MinValue), Int.MinValue, Date.valueOf("2017-01-01")),

sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala

@@ -310,19 +310,4 @@ class SQLConfSuite extends QueryTest with SharedSQLContext {
     SQLConf.unregister(fallback)
   }
 
-  test("SPARK-26205: spark.sql.optimizer.inSetSwitchThreshold must be handled correctly") {
-    spark.sessionState.conf.clear()
-
-    assert(spark.conf.get(SQLConf.OPTIMIZER_INSET_SWITCH_THRESHOLD) == 400)
-    spark.conf.set(SQLConf.OPTIMIZER_INSET_SWITCH_THRESHOLD.key, 50)
-    assert(spark.conf.get(SQLConf.OPTIMIZER_INSET_SWITCH_THRESHOLD) == 50)
-    intercept[IllegalArgumentException] {
-      spark.conf.set(SQLConf.OPTIMIZER_INSET_SWITCH_THRESHOLD.key, -1)
-    }
-    intercept[IllegalArgumentException] {
-      spark.conf.set(SQLConf.OPTIMIZER_INSET_SWITCH_THRESHOLD.key, 601)
-    }
-
-    spark.sessionState.conf.clear()
-  }
 }