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