rdblue commented on code in PR #9675:
URL: https://github.com/apache/iceberg/pull/9675#discussion_r1501265008


##########
spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/RewriteViewCommands.scala:
##########
@@ -102,20 +113,23 @@ case class RewriteViewCommands(spark: SparkSession) 
extends Rule[LogicalPlan] wi
   private def verifyTemporaryObjectsDontExist(
     name: Identifier,
     child: LogicalPlan): Unit = {
-    import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
-
     val tempViews = collectTemporaryViews(child)
-    tempViews.foreach { nameParts =>
-      throw new AnalysisException(
-        errorClass = "INVALID_TEMP_OBJ_REFERENCE",
-        messageParameters = Map(
-          "obj" -> "VIEW",
-          "objName" -> name.name(),
-          "tempObj" -> "VIEW",
-          "tempObjName" -> nameParts.quoted))
+    if (tempViews.nonEmpty) {
+      throw invalidRefToTempObject(name, tempViews.map(v => 
v.quoted).mkString("[", ", ", "]"), "VIEW")
     }
 
-    // TODO: check for temp function names
+    val tempFunctions = collectTemporaryFunctions(child)
+    if (tempFunctions.nonEmpty) {
+      throw invalidRefToTempObject(name, tempFunctions.mkString("[", ", ", 
"]"), "FUNCTION")
+    }
+  }
+
+  private def invalidRefToTempObject(name: Identifier, tempObjectNames: 
String, tempObjectType: String) = {
+    new AnalysisException(String.format("Cannot create persistent object %s" +
+      " of type VIEW because it references the temporary object(s) %s of" +
+      " type %s. Please make the temporary object(s)" +
+      " persistent, or make the persistent object %s temporary",

Review Comment:
   This isn't a good error message. I think the idea is to match Spark's 
behavior, but I would rather throw good error messages.
   
   I think `Cannot create view %s that references temporary %s: %s` is shorter 
and better. The full identifier should be used (with catalog name) so that it 
is clear that the object is permanent. No need to specify that you can create a 
temporary view instead.



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

Reply via email to