Skip to content

Commit f35d80c

Browse files
andygroveandygrove
authored
chore: Remove COMET_EXPR_ALLOW_INCOMPATIBLE config (#2786)
1 parent 365df8e commit f35d80c

File tree

11 files changed

+43
-55
lines changed

11 files changed

+43
-55
lines changed

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -666,14 +666,6 @@ object CometConf extends ShimCometConf {
666666
.booleanConf
667667
.createWithDefault(false)
668668

669-
val COMET_EXPR_ALLOW_INCOMPATIBLE: ConfigEntry[Boolean] =
670-
conf("spark.comet.expression.allowIncompatible")
671-
.category(CATEGORY_EXEC)
672-
.doc("Comet is not currently fully compatible with Spark for all expressions. " +
673-
s"Set this config to true to allow them anyway. $COMPAT_GUIDE.")
674-
.booleanConf
675-
.createWithDefault(false)
676-
677669
val COMET_EXEC_STRICT_FLOATING_POINT: ConfigEntry[Boolean] =
678670
conf("spark.comet.exec.strictFloatingPoint")
679671
.category(CATEGORY_EXEC)

‎dev/benchmarks/comet-tpcds.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ $SPARK_HOME/bin/spark-submit \
4040
--conf spark.executor.extraClassPath=$COMET_JAR \
4141
--conf spark.plugins=org.apache.spark.CometPlugin \
4242
--conf spark.shuffle.manager=org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager \
43-
--conf spark.comet.expression.allowIncompatible=true \
43+
--conf spark.comet.expression.Cast.allowIncompatible=true \
4444
--conf spark.hadoop.fs.s3a.impl=org.apache.hadoop.fs.s3a.S3AFileSystem \
4545
--conf spark.hadoop.fs.s3a.aws.credentials.provider=com.amazonaws.auth.DefaultAWSCredentialsProviderChain \
4646
tpcbench.py \

‎dev/benchmarks/comet-tpch.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ $SPARK_HOME/bin/spark-submit \
4141
--conf spark.plugins=org.apache.spark.CometPlugin \
4242
--conf spark.shuffle.manager=org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager \
4343
--conf spark.comet.exec.replaceSortMergeJoin=true \
44-
--conf spark.comet.expression.allowIncompatible=true \
44+
--conf spark.comet.expression.Cast.allowIncompatible=true \
4545
--conf spark.hadoop.fs.s3a.impl=org.apache.hadoop.fs.s3a.S3AFileSystem \
4646
--conf spark.hadoop.fs.s3a.aws.credentials.provider=com.amazonaws.auth.DefaultAWSCredentialsProviderChain \
4747
tpcbench.py \

‎docs/source/user-guide/latest/compatibility.md‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ Comet has the following limitations when reading Parquet files:
3232

3333
## ANSI Mode
3434

35-
Comet will fall back to Spark for the following expressions when ANSI mode is enabled, unless
36-
`spark.comet.expression.allowIncompatible=true`.
35+
Comet will fall back to Spark for the following expressions when ANSI mode is enabled. Thes expressions can be enabled by setting
36+
`spark.comet.expression.EXPRNAME.allowIncompatible=true`, where `EXPRNAME` is the Spark expression class name. See
37+
the [Comet Supported Expressions Guide](expressions.md) for more information on this configuration setting.
3738

3839
- Average
3940
- Sum
@@ -58,9 +59,6 @@ Expressions that are not 100% Spark-compatible will fall back to Spark by defaul
5859
`spark.comet.expression.EXPRNAME.allowIncompatible=true`, where `EXPRNAME` is the Spark expression class name. See
5960
the [Comet Supported Expressions Guide](expressions.md) for more information on this configuration setting.
6061

61-
It is also possible to specify `spark.comet.expression.allowIncompatible=true` to enable all
62-
incompatible expressions.
63-
6462
## Regular Expressions
6563

6664
Comet uses the Rust regexp crate for evaluating regular expressions, and this has different behavior from Java's

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

Lines changed: 17 additions & 18 deletions
Large diffs are not rendered by default.

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@ of expressions that be disabled.
3131
Expressions that are not Spark-compatible will fall back to Spark by default and can be enabled by setting
3232
`spark.comet.expression.EXPRNAME.allowIncompatible=true`.
3333

34-
It is also possible to specify `spark.comet.expression.allowIncompatible=true` to enable all
35-
incompatible expressions.
36-
3734
## Conditional Expressions
3835

3936
| Expression | SQL | Spark-Compatible? |

‎docs/source/user-guide/latest/kubernetes.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ spec:
7979
"spark.plugins": "org.apache.spark.CometPlugin"
8080
"spark.comet.enabled": "true"
8181
"spark.comet.exec.enabled": "true"
82-
"spark.comet.expression.allowIncompatible": "true"
8382
"spark.comet.exec.shuffle.enabled": "true"
8483
"spark.comet.exec.shuffle.mode": "auto"
8584
"spark.shuffle.manager": "org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager"

‎spark/src/main/scala/org/apache/comet/expressions/CometCast.scala‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,9 @@ object CometCast extends CometExpressionSerde[Cast] with CometExprShim {
9393
castBuilder.setDatatype(dataType)
9494
castBuilder.setEvalMode(evalModeToProto(evalMode))
9595
castBuilder.setAllowIncompat(
96-
CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.get() ||
97-
SQLConf.get
98-
.getConfString(CometConf.getExprAllowIncompatConfigKey(classOf[Cast]), "false")
99-
.toBoolean)
96+
SQLConf.get
97+
.getConfString(CometConf.getExprAllowIncompatConfigKey(classOf[Cast]), "false")
98+
.toBoolean)
10099
castBuilder.setTimezone(timeZoneId.getOrElse("UTC"))
101100
Some(
102101
ExprOuterClass.Expr

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -404,10 +404,11 @@ object QueryPlanSerde extends Logging with CometExprShim {
404404
None
405405
case Incompatible(notes) =>
406406
val exprAllowIncompat = CometConf.isExprAllowIncompat(exprConfName)
407-
if (exprAllowIncompat || CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.get()) {
407+
if (exprAllowIncompat) {
408408
if (notes.isDefined) {
409409
logWarning(
410-
s"Comet supports $fn when ${CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.key}=true " +
410+
s"Comet supports $fn when " +
411+
s"${CometConf.getExprAllowIncompatConfigKey(exprConfName)}=true " +
411412
s"but has notes: ${notes.get}")
412413
}
413414
aggHandler.convert(aggExpr, fn, inputs, binding, conf)
@@ -416,9 +417,8 @@ object QueryPlanSerde extends Logging with CometExprShim {
416417
withInfo(
417418
fn,
418419
s"$fn is not fully compatible with Spark$optionalNotes. To enable it anyway, " +
419-
s"set ${CometConf.getExprAllowIncompatConfigKey(exprConfName)}=true, or set " +
420-
s"${CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.key}=true to enable all " +
421-
s"incompatible expressions. ${CometConf.COMPAT_GUIDE}.")
420+
s"set ${CometConf.getExprAllowIncompatConfigKey(exprConfName)}=true. " +
421+
s"${CometConf.COMPAT_GUIDE}.")
422422
None
423423
}
424424
case Compatible(notes) =>
@@ -509,10 +509,11 @@ object QueryPlanSerde extends Logging with CometExprShim {
509509
None
510510
case Incompatible(notes) =>
511511
val exprAllowIncompat = CometConf.isExprAllowIncompat(exprConfName)
512-
if (exprAllowIncompat || CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.get()) {
512+
if (exprAllowIncompat) {
513513
if (notes.isDefined) {
514514
logWarning(
515-
s"Comet supports $expr when ${CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.key}=true " +
515+
s"Comet supports $expr when " +
516+
s"${CometConf.getExprAllowIncompatConfigKey(exprConfName)}=true " +
516517
s"but has notes: ${notes.get}")
517518
}
518519
handler.convert(expr, inputs, binding)
@@ -521,9 +522,8 @@ object QueryPlanSerde extends Logging with CometExprShim {
521522
withInfo(
522523
expr,
523524
s"$expr is not fully compatible with Spark$optionalNotes. To enable it anyway, " +
524-
s"set ${CometConf.getExprAllowIncompatConfigKey(exprConfName)}=true, or set " +
525-
s"${CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.key}=true to enable all " +
526-
s"incompatible expressions. ${CometConf.COMPAT_GUIDE}.")
525+
s"set ${CometConf.getExprAllowIncompatConfigKey(exprConfName)}=true. " +
526+
s"${CometConf.COMPAT_GUIDE}.")
527527
None
528528
}
529529
case Compatible(notes) =>

‎spark/src/test/scala/org/apache/comet/exec/CometAggregateSuite.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import scala.util.Random
2323

2424
import org.apache.hadoop.fs.Path
2525
import org.apache.spark.sql.{CometTestBase, DataFrame, Row}
26+
import org.apache.spark.sql.catalyst.expressions.Cast
2627
import org.apache.spark.sql.catalyst.optimizer.EliminateSorts
2728
import org.apache.spark.sql.comet.CometHashAggregateExec
2829
import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper
@@ -1073,7 +1074,7 @@ class CometAggregateSuite extends CometTestBase with AdaptiveSparkPlanHelper {
10731074
withSQLConf(
10741075
CometConf.COMET_EXEC_SHUFFLE_ENABLED.key -> nativeShuffleEnabled.toString,
10751076
CometConf.COMET_SHUFFLE_MODE.key -> "native",
1076-
CometConf.COMET_EXPR_ALLOW_INCOMPATIBLE.key -> "true") {
1077+
CometConf.getExprAllowIncompatConfigKey(classOf[Cast]) -> "true") {
10771078
withTempDir { dir =>
10781079
val path = new Path(dir.toURI.toString, "test")
10791080
makeParquetFile(path, 1000, 20, dictionaryEnabled)

0 commit comments

Comments
 (0)