ulysses-you commented on a change in pull request #28840:
URL: https://github.com/apache/spark/pull/28840#discussion_r454162497



##########
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala
##########
@@ -236,6 +236,45 @@ case class ShowFunctionsCommand(
   }
 }
 
+
+/**
+ * A command for users to refresh the persistent function.
+ * The syntax of using this command in SQL is:
+ * {{{
+ *    REFRESH FUNCTION functionName
+ * }}}
+ */
+case class RefreshFunctionCommand(
+    databaseName: Option[String],
+    functionName: String)
+  extends RunnableCommand {
+
+  override def run(sparkSession: SparkSession): Seq[Row] = {
+    val catalog = sparkSession.sessionState.catalog
+    if 
(FunctionRegistry.builtin.functionExists(FunctionIdentifier(functionName))) {
+      throw new AnalysisException(s"Cannot refresh builtin function 
$functionName")
+    }
+    if (catalog.isTemporaryFunction(FunctionIdentifier(functionName, 
databaseName))) {
+      throw new AnalysisException(s"Cannot refresh temporary function 
$functionName")
+    }
+
+    val identifier = FunctionIdentifier(
+      functionName, Some(databaseName.getOrElse(catalog.getCurrentDatabase)))
+    // we only refresh the permanent function.
+    if (catalog.isPersistentFunction(identifier)) {
+      // register overwrite function.
+      val func = catalog.getFunctionMetadata(identifier)
+      catalog.registerFunction(func, true)
+    } else {
+      // function is not exists, clear cached function.
+      catalog.unregisterFunction(identifier, true)
+      throw new NoSuchFunctionException(identifier.database.get, functionName)

Review comment:
       `REFRESH TABLE` doesn't do the side-effects, it always check the table 
if exist first.
   
   I think it's necessary to have both of invalid cache and throw exception.
   * It's confused that we can still use or desc a not exist function if we 
just throw exception. 
   * It's also confused that we can refresh any function name without an 
exception if we just clear cache.
   
   BTW current `REFRESH TABLE` exists a minor memory leak in this case
   ```
   -- client a execute
   create table t1(c1 int);
   cache table t1;
   
   -- client b execute
   drop table t1;
   create table t1(c1 int, c2 int);
   uncache table t1.
   
   -- client a.t1 produce a memory leak
   -- the reason is spark think it's a plan cache but user may think it's a 
table cache
   ```




----------------------------------------------------------------
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.

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



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

Reply via email to