andygrove commented on code in PR #4799:
URL: https://github.com/apache/datafusion-comet/pull/4799#discussion_r3589254152
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -440,9 +440,29 @@ object CometArrayJoin
}
}
-object CometArrayInsert extends CometExpressionSerde[ArrayInsert] {
+object CometArrayInsert extends CometExpressionSerde[ArrayInsert] with
CodegenDispatchFallback {
- override def getSupportLevel(expr: ArrayInsert): SupportLevel = Compatible()
+ // Spark's `spark.sql.legacy.negativeIndexInArrayInsert=true` changes how a
0-based/negative
+ // position is interpreted. Rather than maintain a parallel native code path
for the legacy
+ // semantics, mark `array_insert` Incompatible when the flag is on so
+ // [[CodegenDispatchFallback]] routes the expression through the JVM codegen
dispatcher
+ // (Spark's own `doGenCode` inside the Comet kernel) -- that gives
Spark-exact results
+ // without duplicating the legacy branch natively.
+ private val legacyNegativeIndexConfig =
"spark.sql.legacy.negativeIndexInArrayInsert"
+
+ private val legacyNegativeIndexReason =
+ s"`$legacyNegativeIndexConfig=true` legacy negative-index semantics are
not implemented" +
+ " natively"
+
+ override def getIncompatibleReasons(): Seq[String] =
Seq(legacyNegativeIndexReason)
+
+ override def getSupportLevel(expr: ArrayInsert): SupportLevel = {
+ if (SQLConf.get.getConfString(legacyNegativeIndexConfig,
"false").toBoolean) {
+ Incompatible(Some(legacyNegativeIndexReason))
+ } else {
+ Compatible()
+ }
+ }
Review Comment:
This change looks good and could really be a standalone PR. This also looks
like a bug fix for an issue where we were lacking tests?
--
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]