[GitHub] carbondata pull request #2990: [CARBONDATA-3149]Support alter table column r...

[qiuchenjian-2]

GitHub user akashrn5 opened a pull request:

    https://github.com/apache/carbondata/pull/2990

    [CARBONDATA-3149]Support alter table column rename

    ### Why this PR?
    This PR is to support column rename feature in carbondata. Carbon already supports datatype change, alter table add column and drop column. This PR uses same DDL as datatype change and supports the column rename.
   
    Any column canbe
   
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [ ] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/akashrn5/incubator-carbondata alter_rename

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/2990.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2990
   
----
commit e84b09dd244803746fcacd3e9c6eda4105dd7bef
Author: akashrn5 <akashnilugal@...>
Date:   2018-12-14T11:20:09Z

    Support alter table column rename

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1762/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1974/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10022/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1766/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/1981/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10028/



---

[qiuchenjian-2]

Github user qiuchenjian commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r241935024
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala ---
    @@ -1464,31 +1464,46 @@ abstract class CarbonDDLSqlParser extends AbstractCarbonSparkSQLParser {
        * @param values
        * @return
        */
    -  def parseDataType(dataType: String, values: Option[List[(Int, Int)]]): DataTypeInfo = {
    +  def parseDataType(
    +      dataType: String,
    +      values: Option[List[(Int, Int)]],
    +      isColumnRename: Boolean): DataTypeInfo = {
    +    def validateAndGetDecimalDatatype: DataTypeInfo = {
    +      var precision: Int = 0
    +      var scale: Int = 0
    +      if (values.isDefined) {
    +        precision = values.get(0)._1
    +        scale = values.get(0)._2
    +      } else {
    +        throw new MalformedCarbonCommandException("Decimal format provided is invalid")
    +      }
    +      // precision should be > 0 and <= 38 and scale should be >= 0 and <= 38
    +      if (precision < 1 || precision > 38) {
    --- End diff --
   
    Magic number 38 should be defined in constant,


---

[qiuchenjian-2]

Github user qiuchenjian commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r241935317
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala ---
    @@ -286,12 +286,16 @@ class CarbonFileMetastore extends CarbonMetaStore {
           newTableIdentifier: CarbonTableIdentifier,
           oldTableIdentifier: CarbonTableIdentifier,
           thriftTableInfo: org.apache.carbondata.format.TableInfo,
    -      schemaEvolutionEntry: SchemaEvolutionEntry,
    +      schemaEvolutionEntry: List[SchemaEvolutionEntry],
    --- End diff --
   
    schemaEvolutionEntries or schemaEvolutionEntryList is better


---

[qiuchenjian-2]

Github user qiuchenjian commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r242007987
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala ---
    @@ -0,0 +1,345 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.execution.command.schema
    +
    +import scala.collection.JavaConverters._
    +import scala.collection.mutable
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.sql.{CarbonEnv, Row, SparkSession}
    +import org.apache.spark.sql.execution.command.{AlterTableColRenameAndDataTypeChangeModel, DataTypeInfo, MetadataCommand}
    +import org.apache.spark.sql.hive.CarbonSessionCatalog
    +import org.apache.spark.util.AlterTableUtil
    +
    +import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +import org.apache.carbondata.core.features.TableOperation
    +import org.apache.carbondata.core.locks.{ICarbonLock, LockUsage}
    +import org.apache.carbondata.core.metadata.converter.ThriftWrapperSchemaConverterImpl
    +import org.apache.carbondata.core.metadata.schema.table.CarbonTable
    +import org.apache.carbondata.core.metadata.schema.table.column.CarbonColumn
    +import org.apache.carbondata.events.{AlterTableColRenameAndDataTypeChangePostEvent, AlterTableColRenameAndDataTypeChangePreEvent, OperationContext, OperationListenerBus}
    +import org.apache.carbondata.format.{ColumnSchema, SchemaEvolutionEntry, TableInfo}
    +import org.apache.carbondata.spark.util.DataTypeConverterUtil
    +
    +private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
    +    alterTableColRenameAndDataTypeChangeModel: AlterTableColRenameAndDataTypeChangeModel)
    +  extends MetadataCommand {
    +
    +  override def processMetadata(sparkSession: SparkSession): Seq[Row] = {
    +    val LOGGER = LogServiceFactory.getLogService(this.getClass.getCanonicalName)
    +    val tableName = alterTableColRenameAndDataTypeChangeModel.tableName
    +    val dbName = alterTableColRenameAndDataTypeChangeModel.databaseName
    +      .getOrElse(sparkSession.catalog.currentDatabase)
    +    var isColumnRenameOnly = false
    +    var isDataTypeChangeOnly = false
    +    var isBothColRenameAndDataTypeChange = false
    +    setAuditTable(dbName, tableName)
    +    setAuditInfo(Map(
    +      "column" -> alterTableColRenameAndDataTypeChangeModel.columnName,
    +      "newColumn" -> alterTableColRenameAndDataTypeChangeModel.newColumnName,
    +      "newType" -> alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType))
    +    val locksToBeAcquired = List(LockUsage.METADATA_LOCK, LockUsage.COMPACTION_LOCK)
    +    var locks = List.empty[ICarbonLock]
    +    // get the latest carbon table and check for column existence
    +    var carbonTable: CarbonTable = null
    +    var timeStamp = 0L
    +    try {
    +      locks = AlterTableUtil
    +        .validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
    +      val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
    +      carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
    +      if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_COL_RENAME_AND_CHANGE_DATATYPE,
    +        alterTableColRenameAndDataTypeChangeModel.columnName)) {
    +        throw new MalformedCarbonCommandException(
    +          "alter table change datatype or column rename is not supported for index datamap")
    +      }
    +      val operationContext = new OperationContext
    +      val alterTableColRenameAndDataTypeChangePreEvent =
    +        AlterTableColRenameAndDataTypeChangePreEvent(sparkSession, carbonTable,
    +          alterTableColRenameAndDataTypeChangeModel)
    +      OperationListenerBus.getInstance()
    +        .fireEvent(alterTableColRenameAndDataTypeChangePreEvent, operationContext)
    +      val newColumnName = alterTableColRenameAndDataTypeChangeModel.newColumnName
    +      val oldColumnName = alterTableColRenameAndDataTypeChangeModel.columnName
    +      val carbonColumns = carbonTable.getCreateOrderColumn(tableName).asScala.filter(!_.isInvisible)
    +      if (!carbonColumns.exists(_.getColName.equalsIgnoreCase(oldColumnName))) {
    --- End diff --
   
    Should it be judged that the new column name is not in the old columns?  if new column name is the same as one of old column, ti rhown exception


---

[qiuchenjian-2]

Github user qiuchenjian commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r242010398
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala ---
    @@ -0,0 +1,345 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.execution.command.schema
    +
    +import scala.collection.JavaConverters._
    +import scala.collection.mutable
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.sql.{CarbonEnv, Row, SparkSession}
    +import org.apache.spark.sql.execution.command.{AlterTableColRenameAndDataTypeChangeModel, DataTypeInfo, MetadataCommand}
    +import org.apache.spark.sql.hive.CarbonSessionCatalog
    +import org.apache.spark.util.AlterTableUtil
    +
    +import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +import org.apache.carbondata.core.features.TableOperation
    +import org.apache.carbondata.core.locks.{ICarbonLock, LockUsage}
    +import org.apache.carbondata.core.metadata.converter.ThriftWrapperSchemaConverterImpl
    +import org.apache.carbondata.core.metadata.schema.table.CarbonTable
    +import org.apache.carbondata.core.metadata.schema.table.column.CarbonColumn
    +import org.apache.carbondata.events.{AlterTableColRenameAndDataTypeChangePostEvent, AlterTableColRenameAndDataTypeChangePreEvent, OperationContext, OperationListenerBus}
    +import org.apache.carbondata.format.{ColumnSchema, SchemaEvolutionEntry, TableInfo}
    +import org.apache.carbondata.spark.util.DataTypeConverterUtil
    +
    +private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
    +    alterTableColRenameAndDataTypeChangeModel: AlterTableColRenameAndDataTypeChangeModel)
    +  extends MetadataCommand {
    +
    +  override def processMetadata(sparkSession: SparkSession): Seq[Row] = {
    +    val LOGGER = LogServiceFactory.getLogService(this.getClass.getCanonicalName)
    +    val tableName = alterTableColRenameAndDataTypeChangeModel.tableName
    +    val dbName = alterTableColRenameAndDataTypeChangeModel.databaseName
    +      .getOrElse(sparkSession.catalog.currentDatabase)
    +    var isColumnRenameOnly = false
    +    var isDataTypeChangeOnly = false
    +    var isBothColRenameAndDataTypeChange = false
    +    setAuditTable(dbName, tableName)
    +    setAuditInfo(Map(
    +      "column" -> alterTableColRenameAndDataTypeChangeModel.columnName,
    +      "newColumn" -> alterTableColRenameAndDataTypeChangeModel.newColumnName,
    +      "newType" -> alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType))
    +    val locksToBeAcquired = List(LockUsage.METADATA_LOCK, LockUsage.COMPACTION_LOCK)
    +    var locks = List.empty[ICarbonLock]
    +    // get the latest carbon table and check for column existence
    +    var carbonTable: CarbonTable = null
    +    var timeStamp = 0L
    +    try {
    +      locks = AlterTableUtil
    +        .validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
    +      val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
    +      carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
    +      if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_COL_RENAME_AND_CHANGE_DATATYPE,
    +        alterTableColRenameAndDataTypeChangeModel.columnName)) {
    +        throw new MalformedCarbonCommandException(
    +          "alter table change datatype or column rename is not supported for index datamap")
    +      }
    +      val operationContext = new OperationContext
    +      val alterTableColRenameAndDataTypeChangePreEvent =
    +        AlterTableColRenameAndDataTypeChangePreEvent(sparkSession, carbonTable,
    +          alterTableColRenameAndDataTypeChangeModel)
    +      OperationListenerBus.getInstance()
    +        .fireEvent(alterTableColRenameAndDataTypeChangePreEvent, operationContext)
    +      val newColumnName = alterTableColRenameAndDataTypeChangeModel.newColumnName
    +      val oldColumnName = alterTableColRenameAndDataTypeChangeModel.columnName
    +      val carbonColumns = carbonTable.getCreateOrderColumn(tableName).asScala.filter(!_.isInvisible)
    +      if (!carbonColumns.exists(_.getColName.equalsIgnoreCase(oldColumnName))) {
    +        throwMetadataException(dbName, tableName, s"Column does not exist: $oldColumnName")
    +      }
    +
    +      val carbonColumn = carbonColumns.filter(_.getColName.equalsIgnoreCase(oldColumnName))
    +      if (carbonColumn.size != 1) {
    +        throwMetadataException(dbName, tableName, s"Invalid Column: $oldColumnName")
    +      }
    +      if (alterTableColRenameAndDataTypeChangeModel.isColumnRename) {
    +        // check whether new column name is already an existing column name
    +        if (carbonColumns.exists(_.getColName.equalsIgnoreCase(newColumnName))) {
    +          throw new MalformedCarbonCommandException(s"Column Rename Operation failed. New " +
    +                                                    s"column name $newColumnName already exists" +
    +                                                    s" in table $tableName")
    +        }
    +
    +        // if the datatype is source datatype, then it is just a column rename operation, else do
    +        // the datatype validation for not source datatype
    +        if (carbonColumn.head.getDataType.getName
    +          .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
    +          isColumnRenameOnly = true
    +        } else {
    +          isBothColRenameAndDataTypeChange = true
    +        }
    +
    +        // if column rename operation is on partition column, then fail the rename 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 " +
    +                  s"the partition column $newColumnName is not " +
    +                  s"allowed")
    +              }
    +          }
    +        }
    +      } else {
    +        isDataTypeChangeOnly = true
    --- End diff --
   
    Why isDataTypeChangeOnly is true, when it is not partition table


---

[qiuchenjian-2]

Github user qiuchenjian commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r242010744
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala ---
    @@ -0,0 +1,345 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.execution.command.schema
    +
    +import scala.collection.JavaConverters._
    +import scala.collection.mutable
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.sql.{CarbonEnv, Row, SparkSession}
    +import org.apache.spark.sql.execution.command.{AlterTableColRenameAndDataTypeChangeModel, DataTypeInfo, MetadataCommand}
    +import org.apache.spark.sql.hive.CarbonSessionCatalog
    +import org.apache.spark.util.AlterTableUtil
    +
    +import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +import org.apache.carbondata.core.features.TableOperation
    +import org.apache.carbondata.core.locks.{ICarbonLock, LockUsage}
    +import org.apache.carbondata.core.metadata.converter.ThriftWrapperSchemaConverterImpl
    +import org.apache.carbondata.core.metadata.schema.table.CarbonTable
    +import org.apache.carbondata.core.metadata.schema.table.column.CarbonColumn
    +import org.apache.carbondata.events.{AlterTableColRenameAndDataTypeChangePostEvent, AlterTableColRenameAndDataTypeChangePreEvent, OperationContext, OperationListenerBus}
    +import org.apache.carbondata.format.{ColumnSchema, SchemaEvolutionEntry, TableInfo}
    +import org.apache.carbondata.spark.util.DataTypeConverterUtil
    +
    +private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
    +    alterTableColRenameAndDataTypeChangeModel: AlterTableColRenameAndDataTypeChangeModel)
    +  extends MetadataCommand {
    +
    +  override def processMetadata(sparkSession: SparkSession): Seq[Row] = {
    +    val LOGGER = LogServiceFactory.getLogService(this.getClass.getCanonicalName)
    +    val tableName = alterTableColRenameAndDataTypeChangeModel.tableName
    +    val dbName = alterTableColRenameAndDataTypeChangeModel.databaseName
    +      .getOrElse(sparkSession.catalog.currentDatabase)
    +    var isColumnRenameOnly = false
    +    var isDataTypeChangeOnly = false
    +    var isBothColRenameAndDataTypeChange = false
    +    setAuditTable(dbName, tableName)
    +    setAuditInfo(Map(
    +      "column" -> alterTableColRenameAndDataTypeChangeModel.columnName,
    +      "newColumn" -> alterTableColRenameAndDataTypeChangeModel.newColumnName,
    +      "newType" -> alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType))
    +    val locksToBeAcquired = List(LockUsage.METADATA_LOCK, LockUsage.COMPACTION_LOCK)
    +    var locks = List.empty[ICarbonLock]
    +    // get the latest carbon table and check for column existence
    +    var carbonTable: CarbonTable = null
    +    var timeStamp = 0L
    +    try {
    +      locks = AlterTableUtil
    +        .validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
    +      val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
    +      carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
    +      if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_COL_RENAME_AND_CHANGE_DATATYPE,
    +        alterTableColRenameAndDataTypeChangeModel.columnName)) {
    +        throw new MalformedCarbonCommandException(
    +          "alter table change datatype or column rename is not supported for index datamap")
    +      }
    +      val operationContext = new OperationContext
    +      val alterTableColRenameAndDataTypeChangePreEvent =
    +        AlterTableColRenameAndDataTypeChangePreEvent(sparkSession, carbonTable,
    +          alterTableColRenameAndDataTypeChangeModel)
    +      OperationListenerBus.getInstance()
    +        .fireEvent(alterTableColRenameAndDataTypeChangePreEvent, operationContext)
    +      val newColumnName = alterTableColRenameAndDataTypeChangeModel.newColumnName
    +      val oldColumnName = alterTableColRenameAndDataTypeChangeModel.columnName
    +      val carbonColumns = carbonTable.getCreateOrderColumn(tableName).asScala.filter(!_.isInvisible)
    +      if (!carbonColumns.exists(_.getColName.equalsIgnoreCase(oldColumnName))) {
    +        throwMetadataException(dbName, tableName, s"Column does not exist: $oldColumnName")
    +      }
    +
    +      val carbonColumn = carbonColumns.filter(_.getColName.equalsIgnoreCase(oldColumnName))
    +      if (carbonColumn.size != 1) {
    +        throwMetadataException(dbName, tableName, s"Invalid Column: $oldColumnName")
    +      }
    +      if (alterTableColRenameAndDataTypeChangeModel.isColumnRename) {
    +        // check whether new column name is already an existing column name
    +        if (carbonColumns.exists(_.getColName.equalsIgnoreCase(newColumnName))) {
    +          throw new MalformedCarbonCommandException(s"Column Rename Operation failed. New " +
    +                                                    s"column name $newColumnName already exists" +
    +                                                    s" in table $tableName")
    +        }
    +
    +        // if the datatype is source datatype, then it is just a column rename operation, else do
    +        // the datatype validation for not source datatype
    +        if (carbonColumn.head.getDataType.getName
    +          .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
    +          isColumnRenameOnly = true
    +        } else {
    +          isBothColRenameAndDataTypeChange = true
    +        }
    +
    +        // if column rename operation is on partition column, then fail the rename 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 " +
    +                  s"the partition column $newColumnName is not " +
    +                  s"allowed")
    +              }
    +          }
    +        }
    +      } else {
    +        isDataTypeChangeOnly = true
    +      }
    +      if (isBothColRenameAndDataTypeChange || isDataTypeChangeOnly) {
    +        validateColumnDataType(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo,
    +          carbonColumn.head)
    +      }
    +      // read the latest schema file
    +      val tableInfo: TableInfo =
    --- End diff --
   
    Why not use carbonTable.getTableInfo


---

[qiuchenjian-2]

Github user akashrn5 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r242026723
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala ---
    @@ -0,0 +1,345 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.execution.command.schema
    +
    +import scala.collection.JavaConverters._
    +import scala.collection.mutable
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.sql.{CarbonEnv, Row, SparkSession}
    +import org.apache.spark.sql.execution.command.{AlterTableColRenameAndDataTypeChangeModel, DataTypeInfo, MetadataCommand}
    +import org.apache.spark.sql.hive.CarbonSessionCatalog
    +import org.apache.spark.util.AlterTableUtil
    +
    +import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +import org.apache.carbondata.core.features.TableOperation
    +import org.apache.carbondata.core.locks.{ICarbonLock, LockUsage}
    +import org.apache.carbondata.core.metadata.converter.ThriftWrapperSchemaConverterImpl
    +import org.apache.carbondata.core.metadata.schema.table.CarbonTable
    +import org.apache.carbondata.core.metadata.schema.table.column.CarbonColumn
    +import org.apache.carbondata.events.{AlterTableColRenameAndDataTypeChangePostEvent, AlterTableColRenameAndDataTypeChangePreEvent, OperationContext, OperationListenerBus}
    +import org.apache.carbondata.format.{ColumnSchema, SchemaEvolutionEntry, TableInfo}
    +import org.apache.carbondata.spark.util.DataTypeConverterUtil
    +
    +private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
    +    alterTableColRenameAndDataTypeChangeModel: AlterTableColRenameAndDataTypeChangeModel)
    +  extends MetadataCommand {
    +
    +  override def processMetadata(sparkSession: SparkSession): Seq[Row] = {
    +    val LOGGER = LogServiceFactory.getLogService(this.getClass.getCanonicalName)
    +    val tableName = alterTableColRenameAndDataTypeChangeModel.tableName
    +    val dbName = alterTableColRenameAndDataTypeChangeModel.databaseName
    +      .getOrElse(sparkSession.catalog.currentDatabase)
    +    var isColumnRenameOnly = false
    +    var isDataTypeChangeOnly = false
    +    var isBothColRenameAndDataTypeChange = false
    +    setAuditTable(dbName, tableName)
    +    setAuditInfo(Map(
    +      "column" -> alterTableColRenameAndDataTypeChangeModel.columnName,
    +      "newColumn" -> alterTableColRenameAndDataTypeChangeModel.newColumnName,
    +      "newType" -> alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType))
    +    val locksToBeAcquired = List(LockUsage.METADATA_LOCK, LockUsage.COMPACTION_LOCK)
    +    var locks = List.empty[ICarbonLock]
    +    // get the latest carbon table and check for column existence
    +    var carbonTable: CarbonTable = null
    +    var timeStamp = 0L
    +    try {
    +      locks = AlterTableUtil
    +        .validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
    +      val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
    +      carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
    +      if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_COL_RENAME_AND_CHANGE_DATATYPE,
    +        alterTableColRenameAndDataTypeChangeModel.columnName)) {
    +        throw new MalformedCarbonCommandException(
    +          "alter table change datatype or column rename is not supported for index datamap")
    +      }
    +      val operationContext = new OperationContext
    +      val alterTableColRenameAndDataTypeChangePreEvent =
    +        AlterTableColRenameAndDataTypeChangePreEvent(sparkSession, carbonTable,
    +          alterTableColRenameAndDataTypeChangeModel)
    +      OperationListenerBus.getInstance()
    +        .fireEvent(alterTableColRenameAndDataTypeChangePreEvent, operationContext)
    +      val newColumnName = alterTableColRenameAndDataTypeChangeModel.newColumnName
    +      val oldColumnName = alterTableColRenameAndDataTypeChangeModel.columnName
    +      val carbonColumns = carbonTable.getCreateOrderColumn(tableName).asScala.filter(!_.isInvisible)
    +      if (!carbonColumns.exists(_.getColName.equalsIgnoreCase(oldColumnName))) {
    +        throwMetadataException(dbName, tableName, s"Column does not exist: $oldColumnName")
    +      }
    +
    +      val carbonColumn = carbonColumns.filter(_.getColName.equalsIgnoreCase(oldColumnName))
    +      if (carbonColumn.size != 1) {
    +        throwMetadataException(dbName, tableName, s"Invalid Column: $oldColumnName")
    +      }
    +      if (alterTableColRenameAndDataTypeChangeModel.isColumnRename) {
    +        // check whether new column name is already an existing column name
    +        if (carbonColumns.exists(_.getColName.equalsIgnoreCase(newColumnName))) {
    +          throw new MalformedCarbonCommandException(s"Column Rename Operation failed. New " +
    +                                                    s"column name $newColumnName already exists" +
    +                                                    s" in table $tableName")
    +        }
    +
    +        // if the datatype is source datatype, then it is just a column rename operation, else do
    +        // the datatype validation for not source datatype
    +        if (carbonColumn.head.getDataType.getName
    +          .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
    +          isColumnRenameOnly = true
    +        } else {
    +          isBothColRenameAndDataTypeChange = true
    +        }
    +
    +        // if column rename operation is on partition column, then fail the rename 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 " +
    +                  s"the partition column $newColumnName is not " +
    +                  s"allowed")
    +              }
    +          }
    +        }
    +      } else {
    +        isDataTypeChangeOnly = true
    --- End diff --
   
    i think you are looking into wrong if and else case, there is a partition table validation in if condition, but what you are asking is wrong, this else is for other if, when it is not column rename


---

[qiuchenjian-2]

Github user akashrn5 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r242026814
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableColRenameDataTypeChangeCommand.scala ---
    @@ -0,0 +1,345 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.execution.command.schema
    +
    +import scala.collection.JavaConverters._
    +import scala.collection.mutable
    +import scala.collection.mutable.ArrayBuffer
    +
    +import org.apache.spark.sql.{CarbonEnv, Row, SparkSession}
    +import org.apache.spark.sql.execution.command.{AlterTableColRenameAndDataTypeChangeModel, DataTypeInfo, MetadataCommand}
    +import org.apache.spark.sql.hive.CarbonSessionCatalog
    +import org.apache.spark.util.AlterTableUtil
    +
    +import org.apache.carbondata.common.exceptions.sql.MalformedCarbonCommandException
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +import org.apache.carbondata.core.features.TableOperation
    +import org.apache.carbondata.core.locks.{ICarbonLock, LockUsage}
    +import org.apache.carbondata.core.metadata.converter.ThriftWrapperSchemaConverterImpl
    +import org.apache.carbondata.core.metadata.schema.table.CarbonTable
    +import org.apache.carbondata.core.metadata.schema.table.column.CarbonColumn
    +import org.apache.carbondata.events.{AlterTableColRenameAndDataTypeChangePostEvent, AlterTableColRenameAndDataTypeChangePreEvent, OperationContext, OperationListenerBus}
    +import org.apache.carbondata.format.{ColumnSchema, SchemaEvolutionEntry, TableInfo}
    +import org.apache.carbondata.spark.util.DataTypeConverterUtil
    +
    +private[sql] case class CarbonAlterTableColRenameDataTypeChangeCommand(
    +    alterTableColRenameAndDataTypeChangeModel: AlterTableColRenameAndDataTypeChangeModel)
    +  extends MetadataCommand {
    +
    +  override def processMetadata(sparkSession: SparkSession): Seq[Row] = {
    +    val LOGGER = LogServiceFactory.getLogService(this.getClass.getCanonicalName)
    +    val tableName = alterTableColRenameAndDataTypeChangeModel.tableName
    +    val dbName = alterTableColRenameAndDataTypeChangeModel.databaseName
    +      .getOrElse(sparkSession.catalog.currentDatabase)
    +    var isColumnRenameOnly = false
    +    var isDataTypeChangeOnly = false
    +    var isBothColRenameAndDataTypeChange = false
    +    setAuditTable(dbName, tableName)
    +    setAuditInfo(Map(
    +      "column" -> alterTableColRenameAndDataTypeChangeModel.columnName,
    +      "newColumn" -> alterTableColRenameAndDataTypeChangeModel.newColumnName,
    +      "newType" -> alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType))
    +    val locksToBeAcquired = List(LockUsage.METADATA_LOCK, LockUsage.COMPACTION_LOCK)
    +    var locks = List.empty[ICarbonLock]
    +    // get the latest carbon table and check for column existence
    +    var carbonTable: CarbonTable = null
    +    var timeStamp = 0L
    +    try {
    +      locks = AlterTableUtil
    +        .validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
    +      val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
    +      carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
    +      if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_COL_RENAME_AND_CHANGE_DATATYPE,
    +        alterTableColRenameAndDataTypeChangeModel.columnName)) {
    +        throw new MalformedCarbonCommandException(
    +          "alter table change datatype or column rename is not supported for index datamap")
    +      }
    +      val operationContext = new OperationContext
    +      val alterTableColRenameAndDataTypeChangePreEvent =
    +        AlterTableColRenameAndDataTypeChangePreEvent(sparkSession, carbonTable,
    +          alterTableColRenameAndDataTypeChangeModel)
    +      OperationListenerBus.getInstance()
    +        .fireEvent(alterTableColRenameAndDataTypeChangePreEvent, operationContext)
    +      val newColumnName = alterTableColRenameAndDataTypeChangeModel.newColumnName
    +      val oldColumnName = alterTableColRenameAndDataTypeChangeModel.columnName
    +      val carbonColumns = carbonTable.getCreateOrderColumn(tableName).asScala.filter(!_.isInvisible)
    +      if (!carbonColumns.exists(_.getColName.equalsIgnoreCase(oldColumnName))) {
    +        throwMetadataException(dbName, tableName, s"Column does not exist: $oldColumnName")
    +      }
    +
    +      val carbonColumn = carbonColumns.filter(_.getColName.equalsIgnoreCase(oldColumnName))
    +      if (carbonColumn.size != 1) {
    +        throwMetadataException(dbName, tableName, s"Invalid Column: $oldColumnName")
    +      }
    +      if (alterTableColRenameAndDataTypeChangeModel.isColumnRename) {
    +        // check whether new column name is already an existing column name
    +        if (carbonColumns.exists(_.getColName.equalsIgnoreCase(newColumnName))) {
    +          throw new MalformedCarbonCommandException(s"Column Rename Operation failed. New " +
    +                                                    s"column name $newColumnName already exists" +
    +                                                    s" in table $tableName")
    +        }
    +
    +        // if the datatype is source datatype, then it is just a column rename operation, else do
    +        // the datatype validation for not source datatype
    +        if (carbonColumn.head.getDataType.getName
    +          .equalsIgnoreCase(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo.dataType)) {
    +          isColumnRenameOnly = true
    +        } else {
    +          isBothColRenameAndDataTypeChange = true
    +        }
    +
    +        // if column rename operation is on partition column, then fail the rename 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 " +
    +                  s"the partition column $newColumnName is not " +
    +                  s"allowed")
    +              }
    +          }
    +        }
    +      } else {
    +        isDataTypeChangeOnly = true
    +      }
    +      if (isBothColRenameAndDataTypeChange || isDataTypeChangeOnly) {
    +        validateColumnDataType(alterTableColRenameAndDataTypeChangeModel.dataTypeInfo,
    +          carbonColumn.head)
    +      }
    +      // read the latest schema file
    +      val tableInfo: TableInfo =
    --- End diff --
   
    this is thrift tableInfo object as we will be changing the schema thrift, so this is used


---

[qiuchenjian-2]

Github user akashrn5 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r242026928
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala ---
    @@ -286,12 +286,16 @@ class CarbonFileMetastore extends CarbonMetaStore {
           newTableIdentifier: CarbonTableIdentifier,
           oldTableIdentifier: CarbonTableIdentifier,
           thriftTableInfo: org.apache.carbondata.format.TableInfo,
    -      schemaEvolutionEntry: SchemaEvolutionEntry,
    +      schemaEvolutionEntry: List[SchemaEvolutionEntry],
    --- End diff --
   
    yes, can change to schemaEvolutionEntryList


---

[qiuchenjian-2]

Github user akashrn5 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2990#discussion_r242030254
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala ---
    @@ -1464,31 +1464,46 @@ abstract class CarbonDDLSqlParser extends AbstractCarbonSparkSQLParser {
        * @param values
        * @return
        */
    -  def parseDataType(dataType: String, values: Option[List[(Int, Int)]]): DataTypeInfo = {
    +  def parseDataType(
    +      dataType: String,
    +      values: Option[List[(Int, Int)]],
    +      isColumnRename: Boolean): DataTypeInfo = {
    +    def validateAndGetDecimalDatatype: DataTypeInfo = {
    +      var precision: Int = 0
    +      var scale: Int = 0
    +      if (values.isDefined) {
    +        precision = values.get(0)._1
    +        scale = values.get(0)._2
    +      } else {
    +        throw new MalformedCarbonCommandException("Decimal format provided is invalid")
    +      }
    +      // precision should be > 0 and <= 38 and scale should be >= 0 and <= 38
    +      if (precision < 1 || precision > 38) {
    --- End diff --
   
    since the number is fixed, i havent changed the old code, no need to define constants, so i kept teh same as old


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1784/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1785/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1787/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2990
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1789/



---