Skip to content

Commit 413d6e2

Browse files
andygroveSteve Vaughan Jr
authored andcommitted
chore: Remove COMET_EXPR_ALLOW_INCOMPATIBLE config (apache#2786)
1 parent fc6e0c8 commit 413d6e2

File tree

11 files changed

+26
-38
lines changed

11 files changed

+26
-38
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
@@ -668,14 +668,6 @@ object CometConf extends ShimCometConf {
668668
.booleanConf
669669
.createWithDefault(false)
670670

671-
val COMET_EXPR_ALLOW_INCOMPATIBLE: ConfigEntry[Boolean] =
672-
conf("spark.comet.expression.allowIncompatible")
673-
.category(CATEGORY_EXEC)
674-
.doc("Comet is not currently fully compatible with Spark for all expressions. " +
675-
s"Set this config to true to allow them anyway. $COMPAT_GUIDE.")
676-
.booleanConf
677-
.createWithDefault(false)
678-
679671
val COMET_EXEC_STRICT_FLOATING_POINT: ConfigEntry[Boolean] =
680672
conf("spark.comet.exec.strictFloatingPoint")
681673
.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: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ Comet provides the following configuration settings.
6363
| `spark.comet.exec.enabled` | Whether to enable Comet native vectorized execution for Spark. This controls whether Spark should convert operators into their Comet counterparts and execute them in native space. Note: each operator is associated with a separate config in the format of `spark.comet.exec.<operator_name>.enabled` at the moment, and both the config and this need to be turned on, in order for the operator to be executed in native. | true |
6464
| `spark.comet.exec.replaceSortMergeJoin` | Experimental feature to force Spark to replace SortMergeJoin with ShuffledHashJoin for improved performance. This feature is not stable yet. For more information, refer to the [Comet Tuning Guide](https://datafusion.apache.org/comet/user-guide/tuning.html). | false |
6565
| `spark.comet.exec.strictFloatingPoint` | When enabled, fall back to Spark for floating-point operations that may differ from Spark, such as when comparing or sorting -0.0 and 0.0. For more information, refer to the [Comet Compatibility Guide](https://datafusion.apache.org/comet/user-guide/compatibility.html). | false |
66-
| `spark.comet.expression.allowIncompatible` | Comet is not currently fully compatible with Spark for all expressions. Set this config to true to allow them anyway. For more information, refer to the [Comet Compatibility Guide](https://datafusion.apache.org/comet/user-guide/compatibility.html). | false |
6766
| `spark.comet.maxTempDirectorySize` | The maximum amount of data (in bytes) stored inside the temporary directories. | 107374182400b |
6867
| `spark.comet.metrics.updateInterval` | The interval in milliseconds to update metrics. If interval is negative, metrics will be updated upon task completion. | 3000 |
6968
| `spark.comet.nativeLoadRequired` | Whether to require Comet native library to load successfully when Comet is enabled. If not, Comet will silently fallback to Spark when it fails to load the native lib. Otherwise, an error will be thrown and the Spark job will be aborted. | false |

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)