thesamet commented on a change in pull request #35378:
URL: https://github.com/apache/spark/pull/35378#discussion_r796349570
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
##########
@@ -371,6 +384,18 @@ case class Invoke(
}
private lazy val encodedFunctionName =
ScalaReflection.encodeFieldNameToIdentifier(functionName)
+ private var isDeterministic: Boolean = true
Review comment:
My suggestion is to keep `isDeterministic: boolean = true` in the
constructor, and add `def this` and an `apply` in a companion object:
```
case class StaticInvoke(
staticObject: Class[_],
dataType: DataType,
functionName: String,
arguments: Seq[Expression] = Nil,
inputTypes: Seq[AbstractDataType] = Nil,
propagateNull: Boolean = true,
returnNullable: Boolean = true,
isDeterministic: Boolean = true) {
def this(
staticObject: Class[_],
dataType: DataType,
functionName: String,
arguments: Seq[Expression],
inputTypes: Seq[AbstractDataType],
propagateNull: Boolean,
returnNullable: Boolean
) = {
this(staticObject, dataType, functionName, arguments, inputTypes,
propagateNull, returnNullable, true)
}
}
object StaticInvoke {
def apply(
staticObject: Class[_],
dataType: DataType,
functionName: String,
arguments: Seq[Expression],
inputTypes: Seq[AbstractDataType],
propagateNull: Boolean,
returnNullable: Boolean
): StaticInvoke = StaticInvoke(staticObject, dataType, functionName,
arguments, inputTypes, propagateNull, returnNullable, true)
}
```
The two methods added without default parameters represent what existing
compiled libraries are linked with. I tested this suggestion in a test project
where I added empty traits for `DataType`, `Expression`, etc. The above
suggestion takes care of the constructor compatibility, but leaves a few
unavoidable binary compatibility issues that should be inconsequential
(pertaining to `tupled`, `curried` and `copy` method):
```
[error] compat: Failed binary compatibility check against
default:compat_2.13:0.1.0! Found 4 potential problems (filtered 2)
[error] * static method tupled()scala.Function1 in class x.StaticInvoke
does not have a correspondent in current version
[error] filter with:
ProblemFilters.exclude[DirectMissingMethodProblem]("x.StaticInvoke.tupled")
[error] * static method curried()scala.Function1 in class x.StaticInvoke
does not have a correspondent in current version
[error] filter with:
ProblemFilters.exclude[DirectMissingMethodProblem]("x.StaticInvoke.curried")
[error] * method
copy(java.lang.Class,x.DataType,java.lang.String,scala.collection.immutable.Seq,scala.collection.immutable.Seq,Boolean,Boolean)x.StaticInvoke
in class x.StaticInvoke does not have a correspondent in current version
[error] filter with:
ProblemFilters.exclude[DirectMissingMethodProblem]("x.StaticInvoke.copy")
[error] * the type hierarchy of object x.StaticInvoke is different in
current version. Missing types {scala.runtime.AbstractFunction7}
[error] filter with:
ProblemFilters.exclude[MissingTypesProblem]("x.StaticInvoke$")
```
--
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]
