szehon-ho commented on code in PR #53437:
URL: https://github.com/apache/spark/pull/53437#discussion_r2647532822


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/DescribeProcedureCommand.scala:
##########
@@ -45,34 +42,63 @@ case class DescribeProcedureCommand(
 
   override def run(sparkSession: SparkSession): Seq[Row] = {
     child match {
-      case ResolvedIdentifier(catalog, ident) =>
-        val procedure = load(catalog.asProcedureCatalog, ident)
-        describeV2Procedure(procedure)
+      case ResolvedProcedure(catalog, ident, procedure) =>
+        describeV2Procedure(procedure.asInstanceOf[UnboundProcedure])
       case _ =>
         throw SparkException.internalError(s"Invalid procedure identifier: 
${child.getClass}")
     }
   }
 
-  private def load(catalog: ProcedureCatalog, ident: Identifier): 
UnboundProcedure = {
-    try {
-      catalog.loadProcedure(ident)
-    } catch {
-      case e: Exception if !e.isInstanceOf[SparkThrowable] =>
-        val nameParts = catalog.name +: ident.asMultipartIdentifier
-        throw QueryCompilationErrors.failedToLoadRoutineError(nameParts, e)
-    }
-  }
 
   private def describeV2Procedure(procedure: UnboundProcedure): Seq[Row] = {
     val buffer = new ArrayBuffer[(String, String)]
     append(buffer, "Procedure:", procedure.name())
     append(buffer, "Description:", procedure.description())
 
+    // UnboundProcedure requires binding to retrieve parameters. We try to 
bind with an empty
+    // argument list to get the parameters. If the procedure requires 
arguments, binding might
+    // fail. In that case, we suppress the exception and just show the 
procedure metadata
+    // without parameters.

Review Comment:
   It seems it will work for the new Iceberg procedures (those procedures dont 
require arguments to bind)
   
   I agree the comment sounds a bit confusing.  its not about the procedure 
'requiring arguments' (it sounds like its talking about at runtime), its more 
about whether it can bind succeesfully without arguments?
   
   Also , i am wondering , is the expectation that we do not show anything for 
the procedures that need arguments for binding?



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

Reply via email to