imback82 commented on a change in pull request #30881:
URL: https://github.com/apache/spark/pull/30881#discussion_r547046976



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -344,10 +344,11 @@ case class DescribeRelation(
  */
 case class DescribeColumn(
     relation: LogicalPlan,
-    colNameParts: Seq[String],
+    column: NamedExpression,
     isExtended: Boolean) extends Command {
   override def children: Seq[LogicalPlan] = Seq(relation)
   override def output: Seq[Attribute] = 
DescribeCommandSchema.describeColumnAttributes()
+  override lazy val references: AttributeSet = AttributeSet.empty

Review comment:
       This is required since `column` is an expression, 
`QueryPlan.expressions` will pick up this field.

##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
##########
@@ -149,6 +149,9 @@ trait CheckAnalysis extends PredicateHelper with 
LookupCatalog {
       case AlterTable(_, _, u: UnresolvedV2Relation, _) =>
         failAnalysis(s"Table not found: ${u.originalNameParts.quoted}")
 
+      case DescribeColumn(_: ResolvedTable, UnresolvedAttribute(colNameParts), 
_) =>
+        failAnalysis(s"Column not found: ${colNameParts.quoted}")

Review comment:
       This check should come before expression checks. Otherwise, we will get 
`org.apache.spark.sql.AnalysisException: cannot resolve '`invalid_col`' given 
input columns: []` and this is misleading since input columns are printed as 
empty.

##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala
##########
@@ -344,10 +344,11 @@ case class DescribeRelation(
  */
 case class DescribeColumn(
     relation: LogicalPlan,
-    colNameParts: Seq[String],
+    column: NamedExpression,
     isExtended: Boolean) extends Command {
   override def children: Seq[LogicalPlan] = Seq(relation)
   override def output: Seq[Attribute] = 
DescribeCommandSchema.describeColumnAttributes()
+  override lazy val references: AttributeSet = AttributeSet.empty

Review comment:
       Another way to approach is to use `LogicalPlan` instead of 
`NamedExpression`, but I went with this approach to reuse `UnresolvedAttribute`.
   
   cc @cloud-fan 




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to