You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/02/03 03:16:29 UTC

[GitHub] [spark] HyukjinKwon commented on a change in pull request #35243: [SPARK-37957][SQL] Correctly pass deterministic flag for V2 scalar functions

HyukjinKwon commented on a change in pull request #35243:
URL: https://github.com/apache/spark/pull/35243#discussion_r798182172



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
##########
@@ -248,7 +250,8 @@ case class StaticInvoke(
     arguments: Seq[Expression] = Nil,
     inputTypes: Seq[AbstractDataType] = Nil,
     propagateNull: Boolean = true,
-    returnNullable: Boolean = true) extends InvokeLike {
+    returnNullable: Boolean = true,
+    isDeterministic: Boolean = true) extends InvokeLike {

Review comment:
       I think this might be controversial to backport .. as
   1. V2 expressions are unstable yet.
   2. It could lead to different results in maintenance version upgrade if a user sets `isDeterministic` to `false`
   3. Maybe performance regression if a user sets `isDeterministic` to `false`.
   4. We haven't heard an actual complaint from user mailing list.
   5. This makes an API (`isDeterministic`) working that has never been working in a way (is this a bug fix?)
   
   While I agree with this being merged in 3.3.0, and I don't feel strongly on this, maybe we can consider reverting this out of `branch-3.2` because it has a good and bad thing. If we're worried about this change, we could issue a warning instead when `isDeterministic` from V2 Scalar Function returns `false`.
   
   I will leave it to you @sunchao.




-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org