akashrn5 commented on a change in pull request #4121:
URL: https://github.com/apache/carbondata/pull/4121#discussion_r619957409
##########
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableDropColumnCommand.scala
##########
@@ -147,19 +143,30 @@ private[sql] case class CarbonAlterTableDropColumnCommand(
metastore.getThriftTableInfo(carbonTable)
// maintain the deleted columns for schema evolution history
var deletedColumnSchema =
ListBuffer[org.apache.carbondata.format.ColumnSchema]()
+ var deletedParentColumns =
ListBuffer[org.apache.carbondata.format.ColumnSchema]()
val columnSchemaList = tableInfo.fact_table.table_columns.asScala
alterTableDropColumnModel.columns.foreach { column =>
columnSchemaList.foreach { columnSchema =>
- if (!columnSchema.invisible &&
column.equalsIgnoreCase(columnSchema.column_name)) {
- deletedColumnSchema += columnSchema.deepCopy
- columnSchema.invisible = true
+ if (!columnSchema.invisible) {
+ if (column.equalsIgnoreCase(columnSchema.column_name)) {
+ val columnSchemaCopy = columnSchema.deepCopy
+ deletedParentColumns += columnSchemaCopy
+ deletedColumnSchema += columnSchemaCopy
+ columnSchema.invisible = true
+ }
+ // complex children are prefixed with -> parent_name + '.'
+ if (columnSchema.column_name.toLowerCase.startsWith(
Review comment:
make it `else if`
##########
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableDropColumnCommand.scala
##########
@@ -147,19 +143,30 @@ private[sql] case class CarbonAlterTableDropColumnCommand(
metastore.getThriftTableInfo(carbonTable)
// maintain the deleted columns for schema evolution history
var deletedColumnSchema =
ListBuffer[org.apache.carbondata.format.ColumnSchema]()
+ var deletedParentColumns =
ListBuffer[org.apache.carbondata.format.ColumnSchema]()
val columnSchemaList = tableInfo.fact_table.table_columns.asScala
alterTableDropColumnModel.columns.foreach { column =>
columnSchemaList.foreach { columnSchema =>
- if (!columnSchema.invisible &&
column.equalsIgnoreCase(columnSchema.column_name)) {
- deletedColumnSchema += columnSchema.deepCopy
- columnSchema.invisible = true
+ if (!columnSchema.invisible) {
+ if (column.equalsIgnoreCase(columnSchema.column_name)) {
+ val columnSchemaCopy = columnSchema.deepCopy
+ deletedParentColumns += columnSchemaCopy
+ deletedColumnSchema += columnSchemaCopy
+ columnSchema.invisible = true
+ }
+ // complex children are prefixed with -> parent_name + '.'
+ if (columnSchema.column_name.toLowerCase.startsWith(
+ column + CarbonCommonConstants.POINT)) {
+ deletedColumnSchema += columnSchema.deepCopy
Review comment:
during add columns, you were adding only parent columns to
schemaEvolution right, here dropping includes all columns, there will be
confusions, can we bring both in sync? what do you think @Indhumathi27
##########
File path:
integration/spark/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableDropColumnCommand.scala
##########
@@ -147,19 +143,30 @@ private[sql] case class CarbonAlterTableDropColumnCommand(
metastore.getThriftTableInfo(carbonTable)
// maintain the deleted columns for schema evolution history
var deletedColumnSchema =
ListBuffer[org.apache.carbondata.format.ColumnSchema]()
+ var deletedParentColumns =
ListBuffer[org.apache.carbondata.format.ColumnSchema]()
val columnSchemaList = tableInfo.fact_table.table_columns.asScala
alterTableDropColumnModel.columns.foreach { column =>
columnSchemaList.foreach { columnSchema =>
- if (!columnSchema.invisible &&
column.equalsIgnoreCase(columnSchema.column_name)) {
- deletedColumnSchema += columnSchema.deepCopy
- columnSchema.invisible = true
+ if (!columnSchema.invisible) {
+ if (column.equalsIgnoreCase(columnSchema.column_name)) {
+ val columnSchemaCopy = columnSchema.deepCopy
+ deletedParentColumns += columnSchemaCopy
+ deletedColumnSchema += columnSchemaCopy
+ columnSchema.invisible = true
+ }
+ // complex children are prefixed with -> parent_name + '.'
+ if (columnSchema.column_name.toLowerCase.startsWith(
+ column + CarbonCommonConstants.POINT)) {
+ deletedColumnSchema += columnSchema.deepCopy
Review comment:
during add columns, you were adding only parent columns to
schemaEvolution right, here dropping includes both parent and child columns,
there will be confusions, can we bring both in sync? what do you think
@Indhumathi27
--
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:
[email protected]
