docs: Split configuration guide into different sections (scan, exec, shuffle, etc) (#2568)

Author: andygrove

common/src/main/scala/org/apache/comet/CometConf.scala

@@ -23,9 +23,10 @@ Comet supports the following Spark expressions. Expressions that are marked as S
 natively in Comet and provide the same results as Spark, or will fall back to Spark for cases that would not
 be compatible.
 
-All expressions are enabled by default, but can be disabled by setting
+All expressions are enabled by default, but most can be disabled by setting
 `spark.comet.expression.EXPRNAME.enabled=false`, where `EXPRNAME` is the expression name as specified in
-the following tables, such as `Length`, or `StartsWith`.
+the following tables, such as `Length`, or `StartsWith`. See the [Comet Configuration Guide] for a full list
+of expressions that be disabled.
 
 Expressions that are not Spark-compatible will fall back to Spark by default and can be enabled by setting
 `spark.comet.expression.EXPRNAME.allowIncompatible=true`.
@@ -269,4 +270,5 @@ incompatible expressions.
 | ToPrettyString               | Yes               |                                                                             |
 | UnscaledValue                | Yes               |                                                                             |
 
+[Comet Configuration Guide]: configs.md
 [Comet Compatibility Guide]: compatibility.md

docs/source/user-guide/latest/configs.md

@@ -26,7 +26,7 @@ import scala.collection.mutable.ListBuffer
 import org.apache.spark.sql.catalyst.expressions.Cast
 
 import org.apache.comet.expressions.{CometCast, CometEvalMode}
-import org.apache.comet.serde.{Compatible, Incompatible}
+import org.apache.comet.serde.{Compatible, Incompatible, QueryPlanSerde}
 
 /**
  * Utility for generating markdown documentation from the configs.
@@ -37,29 +37,48 @@ object GenerateDocs {
 
   private def userGuideLocation = "docs/source/user-guide/latest/"
 
+  val publicConfigs: Set[ConfigEntry[_]] = CometConf.allConfs.filter(_.isPublic).toSet
+
   def main(args: Array[String]): Unit = {
     generateConfigReference()
     generateCompatibilityGuide()
   }
 
   private def generateConfigReference(): Unit = {
+
+    val pattern = "<!--BEGIN:CONFIG_TABLE\\[(.*)]-->".r
     val filename = s"$userGuideLocation/configs.md"
     val lines = readFile(filename)
     val w = new BufferedOutputStream(new FileOutputStream(filename))
     for (line <- lines) {
       w.write(s"${line.stripTrailing()}\n".getBytes)
-      if (line.trim == "<!--BEGIN:CONFIG_TABLE-->") {
-        val publicConfigs = CometConf.allConfs.filter(_.isPublic)
-        val confs = publicConfigs.sortBy(_.key)
-        w.write("| Config | Description | Default Value |\n".getBytes)
-        w.write("|--------|-------------|---------------|\n".getBytes)
-        for (conf <- confs) {
-          if (conf.defaultValue.isEmpty) {
-            w.write(s"| ${conf.key} | ${conf.doc.trim} | |\n".getBytes)
-          } else {
-            w.write(s"| ${conf.key} | ${conf.doc.trim} | ${conf.defaultValueString} |\n".getBytes)
+      line match {
+        case pattern(category) =>
+          w.write("| Config | Description | Default Value |\n".getBytes)
+          w.write("|--------|-------------|---------------|\n".getBytes)
+          category match {
+            case "enable_expr" =>
+              for (expr <- QueryPlanSerde.exprSerdeMap.keys.map(_.getSimpleName).toList.sorted) {
+                val config = s"spark.comet.expression.$expr.enabled"
+                w.write(s"| $config | Enable Comet acceleration for $expr | true |\n".getBytes)
+              }
+            case "enable_agg_expr" =>
+              for (expr <- QueryPlanSerde.aggrSerdeMap.keys.map(_.getSimpleName).toList.sorted) {
+                val config = s"spark.comet.expression.$expr.enabled"
+                w.write(s"| $config | Enable Comet acceleration for $expr | true |\n".getBytes)
+              }
+            case _ =>
+              val confs = publicConfigs.filter(_.category == category).toList.sortBy(_.key)
+              for (conf <- confs) {
+                if (conf.defaultValue.isEmpty) {
+                  w.write(s"| ${conf.key} | ${conf.doc.trim} | |\n".getBytes)
+                } else {
+                  w.write(
+                    s"| ${conf.key} | ${conf.doc.trim} | ${conf.defaultValueString} |\n".getBytes)
+                }
+              }
           }
-        }
+        case _ =>
       }
     }
     w.close()

docs/source/user-guide/latest/expressions.md

@@ -239,7 +239,7 @@ object QueryPlanSerde extends Logging with CometExprShim {
   /**
    * Mapping of Spark expression class to Comet expression handler.
    */
-  private val exprSerdeMap: Map[Class[_ <: Expression], CometExpressionSerde[_]] =
+  val exprSerdeMap: Map[Class[_ <: Expression], CometExpressionSerde[_]] =
     mathExpressions ++ hashExpressions ++ stringExpressions ++
       conditionalExpressions ++ mapExpressions ++ predicateExpressions ++
       structExpressions ++ bitwiseExpressions ++ miscExpressions ++ arrayExpressions ++
@@ -248,7 +248,7 @@ object QueryPlanSerde extends Logging with CometExprShim {
   /**
    * Mapping of Spark aggregate expression class to Comet expression handler.
    */
-  private val aggrSerdeMap: Map[Class[_], CometAggregateExpressionSerde[_]] = Map(
+  val aggrSerdeMap: Map[Class[_], CometAggregateExpressionSerde[_]] = Map(
     classOf[Average] -> CometAverage,
     classOf[BitAndAgg] -> CometBitAndAgg,
     classOf[BitOrAgg] -> CometBitOrAgg,
@@ -578,9 +578,16 @@ object QueryPlanSerde extends Logging with CometExprShim {
     val cometExpr = aggrSerdeMap.get(fn.getClass)
     cometExpr match {
       case Some(handler) =>
-        handler
-          .asInstanceOf[CometAggregateExpressionSerde[AggregateFunction]]
-          .convert(aggExpr, fn, inputs, binding, conf)
+        val aggHandler = handler.asInstanceOf[CometAggregateExpressionSerde[AggregateFunction]]
+        val exprConfName = aggHandler.getExprConfigName(fn)
+        if (!CometConf.isExprEnabled(exprConfName)) {
+          withInfo(
+            aggExpr,
+            "Expression support is disabled. Set " +
+              s"${CometConf.getExprEnabledConfigKey(exprConfName)}=true to enable it.")
+          return None
+        }
+        aggHandler.convert(aggExpr, fn, inputs, binding, conf)
       case _ =>
         withInfo(
           aggExpr,
@@ -1832,6 +1839,17 @@ trait CometExpressionSerde[T <: Expression] {
  */
 trait CometAggregateExpressionSerde[T <: AggregateFunction] {
 
+  /**
+   * Get a short name for the expression that can be used as part of a config key related to the
+   * expression, such as enabling or disabling that expression.
+   *
+   * @param expr
+   *   The Spark expression.
+   * @return
+   *   Short name for the expression, defaulting to the Spark class name
+   */
+  def getExprConfigName(expr: T): String = expr.getClass.getSimpleName
+
   /**
    * Convert a Spark expression into a protocol buffer representation that can be passed into
    * native code.

spark/src/main/scala/org/apache/comet/GenerateDocs.scala

@@ -538,32 +538,23 @@ trait CometStddev {
       binding: Boolean,
       conf: SQLConf): Option[ExprOuterClass.AggExpr] = {
     val child = stddev.child
-    if (CometConf.COMET_EXPR_STDDEV_ENABLED.get(conf)) {
-      val childExpr = exprToProto(child, inputs, binding)
-      val dataType = serializeDataType(stddev.dataType)
-
-      if (childExpr.isDefined && dataType.isDefined) {
-        val builder = ExprOuterClass.Stddev.newBuilder()
-        builder.setChild(childExpr.get)
-        builder.setNullOnDivideByZero(nullOnDivideByZero)
-        builder.setDatatype(dataType.get)
-        builder.setStatsTypeValue(statsType)
-
-        Some(
-          ExprOuterClass.AggExpr
-            .newBuilder()
-            .setStddev(builder)
-            .build())
-      } else {
-        withInfo(aggExpr, child)
-        None
-      }
+    val childExpr = exprToProto(child, inputs, binding)
+    val dataType = serializeDataType(stddev.dataType)
+
+    if (childExpr.isDefined && dataType.isDefined) {
+      val builder = ExprOuterClass.Stddev.newBuilder()
+      builder.setChild(childExpr.get)
+      builder.setNullOnDivideByZero(nullOnDivideByZero)
+      builder.setDatatype(dataType.get)
+      builder.setStatsTypeValue(statsType)
+
+      Some(
+        ExprOuterClass.AggExpr
+          .newBuilder()
+          .setStddev(builder)
+          .build())
     } else {
-      withInfo(
-        aggExpr,
-        "stddev disabled by default because it can be slower than Spark. " +
-          s"Set ${CometConf.COMET_EXPR_STDDEV_ENABLED}=true to enable it.",
-        child)
+      withInfo(aggExpr, child)
       None
     }
   }

spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala

@@ -61,9 +61,7 @@ class CometAggregateSuite extends CometTestBase with AdaptiveSparkPlanHelper {
   }
 
   test("stddev_pop should return NaN for some cases") {
-    withSQLConf(
-      CometConf.COMET_EXEC_SHUFFLE_ENABLED.key -> "true",
-      CometConf.COMET_EXPR_STDDEV_ENABLED.key -> "true") {
+    withSQLConf(CometConf.COMET_EXEC_SHUFFLE_ENABLED.key -> "true") {
       Seq(true, false).foreach { nullOnDivideByZero =>
         withSQLConf("spark.sql.legacy.statisticalAggregate" -> nullOnDivideByZero.toString) {
 
@@ -1446,9 +1444,7 @@ class CometAggregateSuite extends CometTestBase with AdaptiveSparkPlanHelper {
   }
 
   test("stddev_pop and stddev_samp") {
-    withSQLConf(
-      CometConf.COMET_EXEC_SHUFFLE_ENABLED.key -> "true",
-      CometConf.COMET_EXPR_STDDEV_ENABLED.key -> "true") {
+    withSQLConf(CometConf.COMET_EXEC_SHUFFLE_ENABLED.key -> "true") {
       Seq("native", "jvm").foreach { cometShuffleMode =>
         withSQLConf(CometConf.COMET_SHUFFLE_MODE.key -> cometShuffleMode) {
           Seq(true, false).foreach { dictionary =>