[GitHub] carbondata pull request #1469: [WIP] Spark-2.2 Carbon Integration - Phase 1

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061073
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/CarbonReflectionUtils.scala ---
    @@ -0,0 +1,90 @@
    +/*
    + * 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.util
    +
    +import java.util
    +
    +import scala.reflect.runtime._
    +import scala.reflect.runtime.universe._
    +
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +
    +/**
    + * Reflection APIs
    + */
    +
    +object CarbonReflectionUtils {
    +
    +  private val LOGGER = LogServiceFactory.getLogService(this.getClass.getCanonicalName)
    +
    +  private val rm = universe.runtimeMirror(getClass.getClassLoader)
    +
    +  /**
    +   * Returns the field val from a object through reflection.
    +   * @param name - name of the field being retrieved.
    +   * @param obj - Object from which the field has to be retrieved.
    +   * @tparam T
    +   * @return
    +   */
    +  def getField[T: TypeTag: reflect.ClassTag](name: String, obj: T): Any = {
    +    val im = rm.reflect(obj)
    +
    +    im.symbol.typeSignature.members.find(
    +      _.name.toString.equals(name)).map(
    +      l => im.reflectField(l.asTerm).get.asInstanceOf[LogicalPlan]
    +    ).getOrElse(null)
    +  }
    +
    +  def hasField[T: TypeTag: reflect.ClassTag](name: String, obj: T): Boolean = {
    +    val hasField : Boolean = if (typeOf[T].members.filter(!_.isMethod).toList.contains(name)) {
    +      true
    +    } else {
    +      false
    +    }
    +    hasField
    +  }
    +
    +  def getUnresolvedRelation(tableIdentifier: TableIdentifier,
    +      tableAlias: Option[String] = None): UnresolvedRelation = {
    +
    +    val clazz = Utils.classForName("org.apache.spark.sql.catalyst.analysis.UnresolvedRelation")
    +    try {
    +      // For 2.1
    +      clazz.getDeclaredField("alias")
    +      val ctor = clazz.getConstructors.head
    +      ctor.setAccessible(true)
    +      val unresolvedrelation = ctor
    +        .newInstance(tableIdentifier,
    +          tableAlias).asInstanceOf[UnresolvedRelation]
    +      unresolvedrelation
    +    } catch {
    +      case ce: NoSuchFieldException =>
    --- End diff --
   
    Please check based on spark version, not through an exception.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061102
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonEnv.scala ---
    @@ -22,6 +22,7 @@ import java.util.concurrent.ConcurrentHashMap
     
     import org.apache.spark.sql.hive.{CarbonMetaStore, CarbonMetaStoreFactory, CarbonSessionCatalog}
     import org.apache.spark.sql.internal.CarbonSQLConf
    +import org.apache.spark.util.Utils
    --- End diff --
   
    remove the unnecessary import


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061126
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonExpressions.scala ---
    @@ -0,0 +1,110 @@
    +/*
    + * 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
    +
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
    +import org.apache.spark.sql.catalyst.expressions.{Attribute, Cast, Expression}
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias}
    +import org.apache.spark.sql.execution.command.DescribeTableCommand
    +import org.apache.spark.sql.types.DataType
    +
    +/**
    + * This class contains the wrappers of all the case classes which are common
    + * across spark version 2.1 and 2.2 but have change in parameter list.
    + * Below are the overriden unapply methods in order to make it work
    + * across both the version of spark2.1 and spark 2.2
    + */
    +object CarbonExpressions {
    +
    +  /**
    +   * unapply method of Cast class.
    +   */
    +  object MatchCast {
    +    def unapply(expr: Expression): Option[(Attribute, DataType)] = {
    +      if (expr.isInstanceOf[Cast] && expr.asInstanceOf[Cast].child.isInstanceOf[Attribute]) {
    --- End diff --
   
    use `match {case }` instead of if else here


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061142
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonExpressions.scala ---
    @@ -0,0 +1,110 @@
    +/*
    + * 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
    +
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
    +import org.apache.spark.sql.catalyst.expressions.{Attribute, Cast, Expression}
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias}
    +import org.apache.spark.sql.execution.command.DescribeTableCommand
    +import org.apache.spark.sql.types.DataType
    +
    +/**
    + * This class contains the wrappers of all the case classes which are common
    + * across spark version 2.1 and 2.2 but have change in parameter list.
    + * Below are the overriden unapply methods in order to make it work
    + * across both the version of spark2.1 and spark 2.2
    + */
    +object CarbonExpressions {
    +
    +  /**
    +   * unapply method of Cast class.
    +   */
    +  object MatchCast {
    +    def unapply(expr: Expression): Option[(Attribute, DataType)] = {
    +      if (expr.isInstanceOf[Cast] && expr.asInstanceOf[Cast].child.isInstanceOf[Attribute]) {
    +        Some((expr.asInstanceOf[Cast].child.asInstanceOf[Attribute], expr.asInstanceOf[Cast].child
    +          .dataType))
    +      } else {
    +        None
    +      }
    +    }
    +  }
    +
    +  /**
    +   * unapply method of Describe Table format.
    +   */
    +  object CarbonDescribeTable {
    +    def unapply(plan: LogicalPlan): Option[(TableIdentifier, TablePartitionSpec, Boolean)] = {
    +      if (plan.isInstanceOf[DescribeTableCommand]) {
    --- End diff --
   
    use `match {case }` instead of if else here


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061155
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonExpressions.scala ---
    @@ -0,0 +1,110 @@
    +/*
    + * 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
    +
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
    +import org.apache.spark.sql.catalyst.expressions.{Attribute, Cast, Expression}
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias}
    +import org.apache.spark.sql.execution.command.DescribeTableCommand
    +import org.apache.spark.sql.types.DataType
    +
    +/**
    + * This class contains the wrappers of all the case classes which are common
    + * across spark version 2.1 and 2.2 but have change in parameter list.
    + * Below are the overriden unapply methods in order to make it work
    + * across both the version of spark2.1 and spark 2.2
    + */
    +object CarbonExpressions {
    +
    +  /**
    +   * unapply method of Cast class.
    +   */
    +  object MatchCast {
    +    def unapply(expr: Expression): Option[(Attribute, DataType)] = {
    +      if (expr.isInstanceOf[Cast] && expr.asInstanceOf[Cast].child.isInstanceOf[Attribute]) {
    +        Some((expr.asInstanceOf[Cast].child.asInstanceOf[Attribute], expr.asInstanceOf[Cast].child
    +          .dataType))
    +      } else {
    +        None
    +      }
    +    }
    +  }
    +
    +  /**
    +   * unapply method of Describe Table format.
    +   */
    +  object CarbonDescribeTable {
    +    def unapply(plan: LogicalPlan): Option[(TableIdentifier, TablePartitionSpec, Boolean)] = {
    +      if (plan.isInstanceOf[DescribeTableCommand]) {
    +        val describeTableCommand = plan.asInstanceOf[DescribeTableCommand]
    +        if (describeTableCommand.table.isInstanceOf[TableIdentifier] &&
    +            describeTableCommand.partitionSpec.isInstanceOf[TablePartitionSpec] &&
    +            describeTableCommand.isExtended.isInstanceOf[Boolean]) {
    +          Some(describeTableCommand.table,
    +            describeTableCommand.partitionSpec,
    +            describeTableCommand.isExtended)
    +        } else {
    +          None
    +        }
    +      } else {
    +        None
    +      }
    +    }
    +  }
    +
    +  /**
    +   * unapply method of SubqueryAlias.
    +   */
    +  object CarbonSubqueryAlias {
    +    def unapply(plan: LogicalPlan): Option[(String, LogicalPlan)] = {
    +      if (plan.isInstanceOf[SubqueryAlias]) {
    --- End diff --
   
    use `match {case }` instead of if else here


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061160
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonExpressions.scala ---
    @@ -0,0 +1,110 @@
    +/*
    + * 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
    +
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
    +import org.apache.spark.sql.catalyst.expressions.{Attribute, Cast, Expression}
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, SubqueryAlias}
    +import org.apache.spark.sql.execution.command.DescribeTableCommand
    +import org.apache.spark.sql.types.DataType
    +
    +/**
    + * This class contains the wrappers of all the case classes which are common
    + * across spark version 2.1 and 2.2 but have change in parameter list.
    + * Below are the overriden unapply methods in order to make it work
    + * across both the version of spark2.1 and spark 2.2
    + */
    +object CarbonExpressions {
    +
    +  /**
    +   * unapply method of Cast class.
    +   */
    +  object MatchCast {
    +    def unapply(expr: Expression): Option[(Attribute, DataType)] = {
    +      if (expr.isInstanceOf[Cast] && expr.asInstanceOf[Cast].child.isInstanceOf[Attribute]) {
    +        Some((expr.asInstanceOf[Cast].child.asInstanceOf[Attribute], expr.asInstanceOf[Cast].child
    +          .dataType))
    +      } else {
    +        None
    +      }
    +    }
    +  }
    +
    +  /**
    +   * unapply method of Describe Table format.
    +   */
    +  object CarbonDescribeTable {
    +    def unapply(plan: LogicalPlan): Option[(TableIdentifier, TablePartitionSpec, Boolean)] = {
    +      if (plan.isInstanceOf[DescribeTableCommand]) {
    +        val describeTableCommand = plan.asInstanceOf[DescribeTableCommand]
    +        if (describeTableCommand.table.isInstanceOf[TableIdentifier] &&
    +            describeTableCommand.partitionSpec.isInstanceOf[TablePartitionSpec] &&
    +            describeTableCommand.isExtended.isInstanceOf[Boolean]) {
    +          Some(describeTableCommand.table,
    +            describeTableCommand.partitionSpec,
    +            describeTableCommand.isExtended)
    +        } else {
    +          None
    +        }
    +      } else {
    +        None
    +      }
    +    }
    +  }
    +
    +  /**
    +   * unapply method of SubqueryAlias.
    +   */
    +  object CarbonSubqueryAlias {
    +    def unapply(plan: LogicalPlan): Option[(String, LogicalPlan)] = {
    +      if (plan.isInstanceOf[SubqueryAlias]) {
    +        val subqueryAlias = plan.asInstanceOf[SubqueryAlias]
    +        if (subqueryAlias.alias.isInstanceOf[String] &&
    +            subqueryAlias.child.isInstanceOf[LogicalPlan]) {
    +          Some(subqueryAlias.alias,
    +            subqueryAlias.child)
    +        } else {
    +          None
    +        }
    +      } else {
    +        None
    +      }
    +    }
    +  }
    +
    +  /**
    +   * uapply method of UnresolvedRelation
    +   */
    +  object CarbonUnresolvedRelation {
    +    def unapply(plan: LogicalPlan): Option[(TableIdentifier)] = {
    +      if (plan.isInstanceOf[UnresolvedRelation]) {
    --- End diff --
   
    use `match {case }` instead of if else here


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061170
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala ---
    @@ -19,10 +19,10 @@ package org.apache.spark.sql
     import java.io.File
     
     import org.apache.hadoop.conf.Configuration
    -import org.apache.spark.{SparkConf, SparkContext}
     import org.apache.spark.scheduler.{SparkListener, SparkListenerApplicationEnd}
    +import org.apache.spark.SparkConf
    +import org.apache.spark.SparkContext
    --- End diff --
   
    Imports order don't change , keep as combined like earlier


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061284
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSession.scala ---
    @@ -43,14 +43,15 @@ class CarbonSession(@transient val sc: SparkContext,
       }
     
       @transient
    -  override lazy val sessionState: SessionState = new CarbonSessionState(this)
    +  override lazy val sessionState: SessionState = CarbonClassReflectionUtils
    +    .getSessionState(sparkContext, this)
    --- End diff --
   
    Change the format like this
    ```
    override lazy val sessionState: SessionState =
        CarbonClassReflectionUtils.getSessionState(sparkContext, this)
    ```


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061299
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/CastExpressionOptimization.scala ---
    @@ -21,12 +21,14 @@ import java.text.{ParseException, SimpleDateFormat}
     import java.util
     import java.util.{Locale, TimeZone}
     
    +import scala.Option
     import scala.collection.JavaConverters._
     
    -import org.apache.spark.sql.catalyst.expressions.{Attribute, Cast, EmptyRow, EqualTo, Expression, GreaterThan, GreaterThanOrEqual, In, LessThan, LessThanOrEqual, Literal, Not}
    +import org.apache.spark.sql.catalyst.expressions.{Attribute, EmptyRow, EqualTo, Expression, GreaterThan, GreaterThanOrEqual, In, LessThan, LessThanOrEqual, Literal, Not}
     import org.apache.spark.sql.CastExpr
     import org.apache.spark.sql.sources
    -import org.apache.spark.sql.types.{DoubleType, IntegerType, StringType, TimestampType}
    +import org.apache.spark.sql.types.{DataType, DoubleType, IntegerType, StringType, TimestampType}
    +import org.apache.spark.sql.CarbonExpressions.{MatchCast => Cast}
    --- End diff --
   
    Don't change the other imports, there are unnecessary imports here


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061334
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/AlterTableDataTypeChangeCommand.scala ---
    @@ -19,9 +19,10 @@ package org.apache.spark.sql.execution.command.schema
     
     import scala.collection.JavaConverters._
     
    +import org.apache.hadoop.hive.ql.session.SessionState
    --- End diff --
   
    Remove the unnecessary import


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061341
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/AlterTableDataTypeChangeCommand.scala ---
    @@ -94,10 +95,7 @@ private[sql] case class AlterTableDataTypeChangeCommand(
           tableInfo.getFact_table.getSchema_evolution.getSchema_evolution_history.get(0)
             .setTime_stamp(System.currentTimeMillis)
           AlterTableUtil
    -        .updateSchemaInfo(carbonTable,
    -          schemaEvolutionEntry,
    -          tableInfo)(sparkSession,
    -          sparkSession.sessionState.asInstanceOf[CarbonSessionState])
    +              .updateSchemaInfo(carbonTable, schemaEvolutionEntry, tableInfo)(sparkSession)
    --- End diff --
   
    move the line up


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061350
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/AlterTableDropColumnCommand.scala ---
    @@ -20,9 +20,10 @@ package org.apache.spark.sql.execution.command.schema
     import scala.collection.JavaConverters._
     import scala.collection.mutable.ListBuffer
     
    +import org.apache.hadoop.hive.ql.session.SessionState
    --- End diff --
   
    remove unused import


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061370
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/AlterTableDropColumnCommand.scala ---
    @@ -120,10 +121,7 @@ private[sql] case class AlterTableDropColumnCommand(
           val schemaEvolutionEntry = new SchemaEvolutionEntry(timeStamp)
           schemaEvolutionEntry.setRemoved(deletedColumnSchema.toList.asJava)
           AlterTableUtil
    -        .updateSchemaInfo(carbonTable,
    -          schemaEvolutionEntry,
    -          tableInfo)(sparkSession,
    -          sparkSession.sessionState.asInstanceOf[CarbonSessionState])
    +              .updateSchemaInfo(carbonTable, schemaEvolutionEntry, tableInfo)(sparkSession)
    --- End diff --
   
    Move the line up


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061398
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/CarbonLateDecodeStrategy.scala ---
    @@ -42,6 +43,7 @@ import org.apache.carbondata.spark.CarbonAliasDecoderRelation
     import org.apache.carbondata.spark.rdd.CarbonScanRDD
     import org.apache.carbondata.spark.util.CarbonScalaUtil
     
    +
    --- End diff --
   
    remove empty line


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061413
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/DDLStrategy.scala ---
    @@ -26,13 +26,19 @@ import org.apache.spark.sql.execution.command.management.{AlterTableCompactionCo
     import org.apache.spark.sql.execution.command.partition.ShowCarbonPartitionsCommand
     import org.apache.spark.sql.execution.command.schema.{AlterTableAddColumnCommand, AlterTableDataTypeChangeCommand, AlterTableDropColumnCommand, AlterTableRenameTableCommand}
     import org.apache.spark.sql.hive.execution.command.{CarbonDropDatabaseCommand, CarbonResetCommand, CarbonSetCommand}
    +import org.apache.spark.sql.CarbonExpressions.{CarbonDescribeTable => DescribeTableCommand}
    +import org.apache.spark.sql.catalyst.catalog.CatalogTypes
     
     import org.apache.carbondata.core.util.CarbonUtil
     import org.apache.carbondata.spark.exception.MalformedCarbonCommandException
     
     /**
      * Carbon strategies for ddl commands
      */
    +case class CarbonDescribeTableCommand (
    --- End diff --
   
    remove unused code


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153061418
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/DDLStrategy.scala ---
    @@ -26,13 +26,19 @@ import org.apache.spark.sql.execution.command.management.{AlterTableCompactionCo
     import org.apache.spark.sql.execution.command.partition.ShowCarbonPartitionsCommand
     import org.apache.spark.sql.execution.command.schema.{AlterTableAddColumnCommand, AlterTableDataTypeChangeCommand, AlterTableDropColumnCommand, AlterTableRenameTableCommand}
     import org.apache.spark.sql.hive.execution.command.{CarbonDropDatabaseCommand, CarbonResetCommand, CarbonSetCommand}
    +import org.apache.spark.sql.CarbonExpressions.{CarbonDescribeTable => DescribeTableCommand}
    +import org.apache.spark.sql.catalyst.catalog.CatalogTypes
    --- End diff --
   
    Remove unused import


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153062031
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/DDLStrategy.scala ---
    @@ -155,6 +161,14 @@ class DDLStrategy(sparkSession: SparkSession) extends SparkStrategy {
             val cmd =
               CreateDataSourceTableCommand(updatedCatalog, ignoreIfExists = mode == SaveMode.Ignore)
             ExecutedCommandExec(cmd) :: Nil
    +      case CreateDataSourceTableCommand(table, ignoreIfExists)
    +        if table.provider.get != DDLUtils.HIVE_PROVIDER
    +           && table.provider.get.equals("org.apache.spark.sql.CarbonSource") =>
    +        val updatedCatalog =
    +          CarbonSource.updateCatalogTableWithCarbonSchema(table, sparkSession)
    --- End diff --
   
    Move line up


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153062036
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/strategy/DDLStrategy.scala ---
    @@ -155,6 +161,14 @@ class DDLStrategy(sparkSession: SparkSession) extends SparkStrategy {
             val cmd =
               CreateDataSourceTableCommand(updatedCatalog, ignoreIfExists = mode == SaveMode.Ignore)
             ExecutedCommandExec(cmd) :: Nil
    +      case CreateDataSourceTableCommand(table, ignoreIfExists)
    +        if table.provider.get != DDLUtils.HIVE_PROVIDER
    +           && table.provider.get.equals("org.apache.spark.sql.CarbonSource") =>
    +        val updatedCatalog =
    +          CarbonSource.updateCatalogTableWithCarbonSchema(table, sparkSession)
    +        val cmd =
    +          CreateDataSourceTableCommand(updatedCatalog, ignoreIfExists)
    --- End diff --
   
    Move line up


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153062059
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonAnalysisRules.scala ---
    @@ -24,17 +24,15 @@ import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute, NamedExpress
     import org.apache.spark.sql.catalyst.plans.Inner
     import org.apache.spark.sql.catalyst.plans.logical._
     import org.apache.spark.sql.catalyst.rules._
    -import org.apache.spark.sql.execution.SparkSqlParser
     import org.apache.spark.sql.execution.command.mutation.ProjectForDeleteCommand
     import org.apache.spark.sql.execution.datasources.LogicalRelation
    +import org.apache.spark.sql.CarbonExpressions.CarbonUnresolvedRelation
     
     import org.apache.carbondata.core.constants.CarbonCommonConstants
     
     
    -/**
    - * Insert into carbon table from other source
    - */
    --- End diff --
   
    Why removed comment


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153062076
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonAnalysisRules.scala ---
    @@ -58,6 +57,8 @@ object CarbonPreInsertionCasts extends Rule[LogicalPlan] {
             )
         }
         if (child.output.size >= relation.carbonRelation.output.size) {
    +      sparkVersion21 = !CarbonClassReflectionUtils.hasField("query", InsertIntoCarbonTable)
    --- End diff --
   
    Don't check the version depends on field, please check spark version


---