QiangCai commented on a change in pull request #3960:
URL:
https://github.com/apache/carbondata/pull/3960#discussion_r495671953##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/carbonTableSchemaCommon.scala
##########
@@ -169,7 +169,8 @@ case class AlterTableDataTypeChangeModel(dataTypeInfo: DataTypeInfo,
tableName: String,
columnName: String,
newColumnName: String,
- isColumnRename: Boolean)
+ isColumnRename: Boolean,
+ newColumnComment: Option[String])
Review comment:
better to provide a default value: None.
it can avoid other unnecessary changes.
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala
##########
@@ -150,45 +140,45 @@ private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
}
val newColumnPrecision = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.precision
val newColumnScale = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.scale
- if (alterTableColRenameAndDataTypeChangeModel.isColumnRename) {
- // validate the columns to be renamed
- validColumnsForRenaming(carbonColumns, oldCarbonColumn.head, carbonTable)
- // if the datatype is source datatype, then it is just a column rename operation, else set
- // the isDataTypeChange flag to true
- if (oldCarbonColumn.head.getDataType.getName
- .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
- val newColumnPrecision =
- alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.precision
- val newColumnScale = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.scale
- // if the source datatype is decimal and there is change in precision and scale, then
- // along with rename, datatype change is also required for the command, so set the
- // isDataTypeChange flag to true in this case
- if (oldCarbonColumn.head.getDataType.getName.equalsIgnoreCase("decimal") &&
- (oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getPrecision !=
- newColumnPrecision ||
- oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getScale !=
- newColumnScale)) {
- isDataTypeChange = true
- }
- } else {
+ // set isDataTypeChange flag
+ if (oldCarbonColumn.head.getDataType.getName
+ .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
+ val newColumnPrecision =
+ alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.precision
+ val newColumnScale = alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.scale
+ // if the source datatype is decimal and there is change in precision and scale, then
+ // along with rename, datatype change is also required for the command, so set the
+ // isDataTypeChange flag to true in this case
+ if (oldCarbonColumn.head.getDataType.getName.equalsIgnoreCase("decimal") &&
+ (oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getPrecision !=
+ newColumnPrecision ||
+ oldCarbonColumn.head.getDataType.asInstanceOf[DecimalType].getScale !=
+ newColumnScale)) {
isDataTypeChange = true
}
} else {
isDataTypeChange = true
}
- if (isDataTypeChange) {
- // if column datatype change operation is on partition column, then fail the datatype change
- // operation
- if (null != carbonTable.getPartitionInfo) {
- val partitionColumns = carbonTable.getPartitionInfo.getColumnSchemaList
- partitionColumns.asScala.foreach {
- col =>
- if (col.getColumnName.equalsIgnoreCase(oldColumnName)) {
- throw new MalformedCarbonCommandException(
- s"Alter datatype of the partition column $newColumnName is not allowed")
- }
- }
+ // if column datatype change operation is on partition column, then fail the
+ // chang column operation
+ if (null != carbonTable.getPartitionInfo) {
+ val partitionColumns = carbonTable.getPartitionInfo.getColumnSchemaList
+ partitionColumns.asScala.foreach {
+ col =>
+ if (col.getColumnName.equalsIgnoreCase(oldColumnName)) {
+ throw new MalformedCarbonCommandException(
+ s"Column Rename Operation failed. Renaming or ChangeType or " +
+ s"ChangeComment the partition column $newColumnName is not " +
Review comment:
how about to support changing comments of partition columns?
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/catalyst/CarbonParserUtil.scala
##########
@@ -1135,11 +1135,7 @@ object CarbonParserUtil {
}
DataTypeInfo("decimal", precision, scale)
case _ =>
- if (isColumnRename) {
- DataTypeInfo(DataTypeConverterUtil.convertToCarbonType(dataType).getName.toLowerCase)
- } else {
- throw new MalformedCarbonCommandException("Data type provided is invalid.")
- }
+ DataTypeInfo(DataTypeConverterUtil.convertToCarbonType(dataType).getName.toLowerCase)
Review comment:
is the parameter isColumnRename of this method needed?
----------------------------------------------------------------
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:
[hidden email]
Locked
