andygrove commented on code in PR #4799:
URL: https://github.com/apache/datafusion-comet/pull/4799#discussion_r3658991546
##########
spark/src/main/scala/org/apache/comet/CometSparkSessionExtensions.scala:
##########
@@ -150,6 +150,27 @@ object CometSparkSessionExtensions extends Logging {
return false
}
+ // Some spark.sql.legacy.* configs affect execution semantics for queries
Comet accelerates
+ // but are not tied to a specific expression that Comet's serdes can gate
on (parquet
+ // datetime rebase modes, decimal-precision analyzer rules, type-coercion
policies, etc.).
+ // When any such config is set to a non-default value we disable Comet for
the session so
+ // Spark's own execution provides the legacy semantics. The list is
intentionally narrow --
+ // legacy configs whose consumers ARE Comet-supported expressions (Cast,
ArrayInsert, In,
+ // etc.) are handled per-expression via [[CodegenDispatchFallback]] and
are NOT in this set.
+ if (COMET_LEGACY_CONF_FALLBACK_ENABLED.get(conf)) {
+ val triggered = LegacyConfFallback.triggeredConfigs(conf)
+ if (triggered.nonEmpty) {
+ val keys = triggered.mkString(", ")
+ logWarning(
+ "Comet extension is disabled because the following
execution-affecting " +
+ s"spark.sql.legacy.* configs are set to non-default values: $keys.
Comet does not " +
+ "implement these legacy execution semantics. To keep Comet enabled
anyway, set " +
+ s"${COMET_LEGACY_CONF_FALLBACK_ENABLED.key}=false (Spark
compatibility is not " +
+ "guaranteed in that case).")
+ return false
Review Comment:
Do we still need this? I thought we were moving the checks to the affected
expressions/scans now? Disabling the plugin seems like overkill, IMO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
