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


##########
spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveViews.scala:
##########
@@ -59,6 +61,32 @@ case class ResolveViews(spark: SparkSession) extends 
Rule[LogicalPlan] with Look
       loadView(catalog, ident)
         .map(_ => ResolvedV2View(catalog.asViewCatalog, ident))
         .getOrElse(u)
+
+    case c@CreateIcebergView(ResolvedIdentifier(_, ident), _, query, 
columnAliases, columnComments, _, _, _, _, _,
+    rewritten)
+      if query.resolved && !rewritten =>
+      val rewritten = rewriteIdentifiers(query, ident.asMultipartIdentifier)
+      val aliasedPlan = aliasPlan(rewritten, columnAliases, columnComments)
+      c.copy(query = aliasedPlan, queryColumnNames = query.schema.fieldNames, 
rewritten = true)
+  }
+
+  private def aliasPlan(
+    analyzedPlan: LogicalPlan,
+    columnAliases: Seq[String],
+    columnComments: Seq[Option[String]]): LogicalPlan = {
+    if (columnAliases.isEmpty || columnAliases.length != 
analyzedPlan.output.length) {
+      analyzedPlan

Review Comment:
   I think we should throw an exception here. I definitely prefer having the 
length check done in `CheckViews`, but I think the right convention is to make 
the `CreateIcebergView` node unresolved if this doesn't happen. The problem 
with using `CreateIcebergView.resolved` is that I suspect that Spark will throw 
a generic error about an unresolved node in the plan _before_ our `CheckViews` 
rule runs. Since I'd rather have a good error message (like the ones you have 
in that validation rule) I think the only place to throw is here.
   
   The alternative is to do what you've already done and keep `rewritten` 
instead of using `resolved`. The plan would technically be unresolved since the 
columns don't align, but that would be okay too.



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