imback82 commented on a change in pull request #33200:
URL: https://github.com/apache/spark/pull/33200#discussion_r671595283
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala
##########
@@ -229,22 +228,13 @@ case class ReplaceTableAsSelectStatement(
* Column data as parsed by ALTER TABLE ... ADD COLUMNS.
*/
case class QualifiedColType(
- name: Seq[String],
+ fieldName: FieldName,
Review comment:
For `AlterTableAddColumns`, we need to 1) resolve the "parent" name if
the column being added is a nested one, and 2) check if the column name already
exists.
For `AlterTableReplaceColumns`, it seems that we do not need to check
anything with this new change (I removed it in the recent
[commit](https://github.com/apache/spark/pull/33200/commits/1eab11f13030c76a500d6d2b46fc8e39a3617d71))
The reason I was using `FieldName` is so that I can check whether
`QualifiedColType` is resolved or not so that rule doesn't run if already
resolved:
```
private def hasUnresolvedColumns(cols: Seq[QualifiedColType]): Boolean = {
cols.exists(col => !col.fieldName.resolved ||
col.position.exists(!_.resolved))
```
, but I agree that using `ResolvedFieldName` is a bit weird since the field
name is being "added". Maybe turn `QualifiedColType` into an `Expression`?
--
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: reviews-unsubscr...@spark.apache.org
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
