MaxGekk commented on code in PR #42524:
URL: https://github.com/apache/spark/pull/42524#discussion_r1350588319
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala:
##########
@@ -415,13 +417,21 @@ case class AlterTableChangeColumnCommand(
Seq.empty[Row]
}
- // Find the origin column from schema by column name, throw an
AnalysisException if the column
- // reference is invalid.
+ // Find the origin column from schema by column name
private def findColumnByName(
- schema: StructType, name: String, resolver: Resolver): StructField = {
+ schema: StructType, name: String, resolver: Resolver):
Option[StructField] = {
schema.fields.collectFirst {
case field if resolver(field.name, name) => field
- }.getOrElse(throw QueryCompilationErrors.cannotFindColumnError(name,
schema.fieldNames))
+ }
+ }
+
+ private def verifyNonPartitionColumn(
+ table: CatalogTable, columnName: String, resolver: Resolver): Unit = {
+ findColumnByName(table.partitionSchema, columnName, resolver) match {
+ case Some(_) =>
+ throw
QueryCompilationErrors.cannotAlterPartitionColumn(table.qualifiedName,
columnName)
+ case None =>
+ }
Review Comment:
Could you make it shorter:
```suggestion
if (findColumnByName(table.dataSchema, columnName, resolver).isDefined) {
throw
QueryCompilationErrors.cannotAlterPartitionColumn(table.qualifiedName,
columnName)
}
```
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala:
##########
@@ -415,13 +417,21 @@ case class AlterTableChangeColumnCommand(
Seq.empty[Row]
}
- // Find the origin column from schema by column name, throw an
AnalysisException if the column
- // reference is invalid.
+ // Find the origin column from schema by column name
private def findColumnByName(
- schema: StructType, name: String, resolver: Resolver): StructField = {
+ schema: StructType, name: String, resolver: Resolver):
Option[StructField] = {
schema.fields.collectFirst {
case field if resolver(field.name, name) => field
- }.getOrElse(throw QueryCompilationErrors.cannotFindColumnError(name,
schema.fieldNames))
+ }
+ }
+
+ private def verifyNonPartitionColumn(
Review Comment:
If it is used only once, let's inline it.
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -97,6 +97,13 @@
"The method <methodName> can not be called on streaming
Dataset/DataFrame."
]
},
+ "CANNOT_ALTER_PARTITION_COLUMN" : {
+ "message" : [
+ "ALTER TABLE (ALTER|CHANGE) COLUMN is not supported for partition
columns.",
+ "<columnName> is a partition column in table <tableName>."
Review Comment:
Let's follow other message in `error-classes.json`:
```suggestion
"ALTER TABLE (ALTER|CHANGE) COLUMN is not supported for partition
columns, but found the partition column <columnName> in the table <tableName>."
```
##########
common/utils/src/main/resources/error/error-classes.json:
##########
@@ -97,6 +97,13 @@
"The method <methodName> can not be called on streaming
Dataset/DataFrame."
]
},
+ "CANNOT_ALTER_PARTITION_COLUMN" : {
+ "message" : [
+ "ALTER TABLE (ALTER|CHANGE) COLUMN is not supported for partition
columns.",
+ "<columnName> is a partition column in table <tableName>."
Review Comment:
nit:
```suggestion
"<columnName> is a partition column in the table <tableName>."
```
--
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]
