Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/22576#discussion_r226571338
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/SparkSessionExtensions.scala ---
@@ -168,4 +173,21 @@ class SparkSessionExtensions {
def injectParser(builder: ParserBuilder): Unit = {
parserBuilders += builder
}
+
+ private[this] val injectedFunctions =
mutable.Buffer.empty[FunctionDescription]
+
+ private[sql] def registerFunctions(functionRegistry: FunctionRegistry) =
{
+ for ((name, expressionInfo, function) <- injectedFunctions) {
--- End diff --
Can you move the stuff that changes the `FunctionRegistry` into the
`BaseSessionStateBuilder` and just make this return the
`Seq[FunctionDescription]`? The return type of this function a
`FunctionRegistry` sort of implies that you are getting back a new registry
instead of a mutated one. If we are mutating then I prefer to do that in the
BaseSessionBuilder so it is obvious that this is safe to do because we mutating
a clone. It also makes this code more inline with the rest of the extension
class (not mutating). Sorry for the late change of heart.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
