nastra commented on code in PR #9675: URL: https://github.com/apache/iceberg/pull/9675#discussion_r1484192464
########## spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/RewriteViewCommands.scala: ########## @@ -149,4 +167,20 @@ case class RewriteViewCommands(spark: SparkSession) extends Rule[LogicalPlan] wi None } } + + /** + * Collect the names of all temporary functions. + */ + private def collectTemporaryFunctions(child: LogicalPlan): Seq[String] = { + val tempFunctions = new ArrayBuffer[String]() + child.resolveExpressionsWithPruning(_.containsAnyPattern(UNRESOLVED_FUNCTION)) { + case f@UnresolvedFunction(nameParts, _, _, _, _) if isTempFunction(nameParts.asFunctionIdentifier) => Review Comment: > This has a bug that is the same as one I pointed out for checking temp table references: if the identifier is too long then asFunctionIdentifier is going to throw an exception. isTempFunction should be passed Seq[String] and should return false if it is too many parts to be a temporary function name. You're right, I missed this and added a fix + tests > In addition, this needs to handle names that include the catalog, like tabular.system.bucket. Can you elaborate what you mean by `this needs to handle names that include the catalog`? According to https://spark.apache.org/docs/latest/sql-ref-syntax-ddl-create-function.html the `function_name` is defined as `[ database_name. ] function_name`. I checked the behavior in Spark and a `database_name` can only be specified for non-temp functions, meaning that temp functions are only allowed to have a single-part identifier when they are being created. At read time, only the simple function name without the `catalog` / `database` part are allowed for temporary functions, otherwise the temp function can't be found. Or did you mean to add some test that use catalog+schema in the identifier to make sure this doesn't fail here when detecting temp functions (I've added such tests btw)? -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org