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

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096045
 
    --- Diff: integration/spark2/src/main/spark2.1/CarbonSessionState.scala ---
    @@ -24,16 +24,18 @@ import org.apache.spark.sql.catalyst.catalog.{FunctionResourceLoader, GlobalTemp
     import org.apache.spark.sql.catalyst.expressions.{PredicateSubquery, ScalarSubquery}
     import org.apache.spark.sql.catalyst.optimizer.Optimizer
     import org.apache.spark.sql.catalyst.parser.ParserInterface
    +import org.apache.spark.sql.catalyst.parser.SqlBaseParser.CreateTableContext
     import org.apache.spark.sql.catalyst.plans.logical.{Filter, LogicalPlan, SubqueryAlias}
    -import org.apache.spark.sql.execution.SparkOptimizer
    +import org.apache.spark.sql.execution.{SparkOptimizer, SparkSqlAstBuilder}
     import org.apache.spark.sql.execution.datasources._
     import org.apache.spark.sql.execution.strategy.{CarbonLateDecodeStrategy, DDLStrategy}
     import org.apache.spark.sql.internal.SQLConf
     import org.apache.spark.sql.optimizer.CarbonLateDecodeRule
    -import org.apache.spark.sql.parser.CarbonSparkSqlParser
    +import org.apache.spark.sql.parser.{CarbonHelperqlAstBuilder, CarbonSpark2SqlParser, CarbonSparkSqlParser}
     
     import org.apache.carbondata.core.datamap.DataMapStoreManager
     import org.apache.carbondata.core.metadata.AbsoluteTableIdentifier
    +import org.apache.carbondata.core.util.CarbonProperties
    --- End diff --
   
    Removed


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096069
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/CarbonClassReflectionUtils.scala ---
    @@ -0,0 +1,186 @@
    +/*
    + * 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 scala.reflect.runtime._
    +import scala.reflect.runtime.universe._
    +
    +import org.apache.spark.SparkContext
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.parser.AstBuilder
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project, SubqueryAlias}
    +import org.apache.spark.sql.internal.{SessionState, SQLConf}
    +import org.apache.spark.sql.parser.CarbonSpark2SqlParser
    +import org.apache.spark.util.Utils
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +
    +/**
    + * Reflection APIs
    + */
    +
    +object CarbonClassReflectionUtils {
    --- End diff --
   
    Ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096083
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/CarbonClassReflectionUtils.scala ---
    @@ -0,0 +1,186 @@
    +/*
    + * 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 scala.reflect.runtime._
    +import scala.reflect.runtime.universe._
    +
    +import org.apache.spark.SparkContext
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.parser.AstBuilder
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project, SubqueryAlias}
    +import org.apache.spark.sql.internal.{SessionState, SQLConf}
    +import org.apache.spark.sql.parser.CarbonSpark2SqlParser
    +import org.apache.spark.util.Utils
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +
    +/**
    + * Reflection APIs
    + */
    +
    +object CarbonClassReflectionUtils {
    +
    +  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 = {
    --- End diff --
   
    Done


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096099
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/CarbonClassReflectionUtils.scala ---
    @@ -0,0 +1,186 @@
    +/*
    + * 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 scala.reflect.runtime._
    +import scala.reflect.runtime.universe._
    +
    +import org.apache.spark.SparkContext
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.UnresolvedRelation
    +import org.apache.spark.sql.catalyst.parser.AstBuilder
    +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project, SubqueryAlias}
    +import org.apache.spark.sql.internal.{SessionState, SQLConf}
    +import org.apache.spark.sql.parser.CarbonSpark2SqlParser
    +import org.apache.spark.util.Utils
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +
    +/**
    + * Reflection APIs
    + */
    +
    +object CarbonClassReflectionUtils {
    +
    +  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]
    --- End diff --
   
    Ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096126
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/util/AlterTableUtil.scala ---
    @@ -21,10 +21,11 @@ import scala.collection.JavaConverters._
     import scala.collection.mutable.ListBuffer
     
     import org.apache.spark.SparkConf
    -import org.apache.spark.sql.{CarbonEnv, SparkSession}
    +import org.apache.spark.sql.{CarbonEnv, CarbonSession, SparkSession}
     import org.apache.spark.sql.catalyst.TableIdentifier
    -import org.apache.spark.sql.hive.{CarbonRelation, CarbonSessionState}
    +import org.apache.spark.sql.hive.{CarbonRelation, HiveExternalCatalog}
     import org.apache.spark.sql.hive.HiveExternalCatalog._
    +import org.apache.spark.sql.internal.SessionState
    --- End diff --
   
    Ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096152
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -184,10 +126,86 @@ class CarbonSqlAstBuilder(conf: SQLConf) extends SparkSqlAstBuilder(conf) {
         }
       }
     
    -  private def needToConvertToLowerCase(key: String): Boolean = {
    -    val noConvertList = Array("LIST_INFO", "RANGE_INFO")
    -    !noConvertList.exists(x => x.equalsIgnoreCase(key));
    +  def getPropertyKeyValues(ctx: TablePropertyListContext): Map[String, String]
    +  = {
    +    Option(ctx).map(visitPropertyKeyValues)
    +      .getOrElse(Map.empty)
       }
     
    +  def createCarbontable(tableHeader: CreateTableHeaderContext,
    +      skewSpecContext: SkewSpecContext,
    +      bucketSpecContext: BucketSpecContext,
    +      partitionColumns: ColTypeListContext,
    +      columns : ColTypeListContext,
    +      tablePropertyList : TablePropertyListContext) : LogicalPlan = {
    +    // val parser = new CarbonSpark2SqlParser
    +
    +    val (name, temp, ifNotExists, external) = visitCreateTableHeader(tableHeader)
    +    // TODO: implement temporary tables
    +    if (temp) {
    +      throw new ParseException(
    +        "CREATE TEMPORARY TABLE is not supported yet. " +
    +        "Please use CREATE TEMPORARY VIEW as an alternative.", tableHeader)
    +    }
    +    if (skewSpecContext != null) {
    +      operationNotAllowed("CREATE TABLE ... SKEWED BY", skewSpecContext)
    +    }
    +    if (bucketSpecContext != null) {
    +      operationNotAllowed("CREATE TABLE ... CLUSTERED BY", bucketSpecContext)
    +    }
    +    val partitionByStructFields = Option(partitionColumns).toSeq.flatMap(visitColTypeList)
    +    val partitionerFields = partitionByStructFields.map { structField =>
    +      PartitionerField(structField.name, Some(structField.dataType.toString), null)
    +    }
    +    val cols = Option(columns).toSeq.flatMap(visitColTypeList)
    +    val properties = getPropertyKeyValues(tablePropertyList)
    +
    +    // Ensuring whether no duplicate name is used in table definition
    +    val colNames = cols.map(_.name)
    +    if (colNames.length != colNames.distinct.length) {
    +      val duplicateColumns = colNames.groupBy(identity).collect {
    +        case (x, ys) if ys.length > 1 => "\"" + x + "\""
    +      }
    +      operationNotAllowed(s"Duplicated column names found in table definition of $name: " +
    --- End diff --
   
    Its same like earlier.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096166
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -184,10 +126,86 @@ class CarbonSqlAstBuilder(conf: SQLConf) extends SparkSqlAstBuilder(conf) {
         }
       }
     
    -  private def needToConvertToLowerCase(key: String): Boolean = {
    -    val noConvertList = Array("LIST_INFO", "RANGE_INFO")
    -    !noConvertList.exists(x => x.equalsIgnoreCase(key));
    +  def getPropertyKeyValues(ctx: TablePropertyListContext): Map[String, String]
    +  = {
    +    Option(ctx).map(visitPropertyKeyValues)
    +      .getOrElse(Map.empty)
       }
     
    +  def createCarbontable(tableHeader: CreateTableHeaderContext,
    --- End diff --
   
    Ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096178
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -69,12 +71,11 @@ class CarbonSparkSqlParser(conf: SQLConf, sparkSession: SparkSession) extends Ab
       }
     }
     
    -class CarbonSqlAstBuilder(conf: SQLConf) extends SparkSqlAstBuilder(conf) {
    -
    -  val parser = new CarbonSpark2SqlParser
    +class CarbonHelperqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser)
    --- End diff --
   
    Done


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096184
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -18,16 +18,17 @@ package org.apache.spark.sql.parser
     
     import scala.collection.mutable
     
    -import org.apache.spark.sql.{CarbonEnv, SparkSession}
    -import org.apache.spark.sql.catalyst.parser.{AbstractSqlParser, ParseException, SqlBaseParser}
    +import org.apache.spark.sql.{CarbonClassReflectionUtils, CarbonEnv, SparkSession}
    +import org.apache.spark.sql.catalyst.parser.{AbstractSqlParser, AstBuilder, ParseException, SqlBaseParser}
     import org.apache.spark.sql.catalyst.parser.ParserUtils._
    -import org.apache.spark.sql.catalyst.parser.SqlBaseParser.{CreateTableContext, TablePropertyListContext}
    +import org.apache.spark.sql.catalyst.parser.SqlBaseParser._
     import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
    +import org.apache.spark.sql.execution.command.{CarbonCreateTableCommand, PartitionerField, TableModel}
     import org.apache.spark.sql.execution.SparkSqlAstBuilder
    -import org.apache.spark.sql.execution.command.{BucketFields, CarbonCreateTableCommand, Field, PartitionerField, TableModel}
     import org.apache.spark.sql.internal.{SQLConf, VariableSubstitution}
    +import org.apache.spark.util.Utils
    --- End diff --
   
    Done


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153096877
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/describeTable/TestDescribeTable.scala ---
    @@ -34,7 +34,7 @@ class TestDescribeTable extends QueryTest with BeforeAndAfterAll {
         sql("CREATE TABLE Desc2(Dec2Col1 BigInt, Dec2Col2 String, Dec2Col3 Bigint, Dec2Col4 Decimal) stored by 'carbondata'")
       }
     
    -  test("test describe table") {
    +  ignore("test describe table") {
    --- End diff --
   
    Although describe table is working But it is showing some format mismatch. This is among of few of the pending issues like Subquery processing. Will fix this soon.


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1469
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1892/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1469
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1473/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153102406
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/describeTable/TestDescribeTable.scala ---
    @@ -34,7 +34,7 @@ class TestDescribeTable extends QueryTest with BeforeAndAfterAll {
         sql("CREATE TABLE Desc2(Dec2Col1 BigInt, Dec2Col2 String, Dec2Col3 Bigint, Dec2Col4 Decimal) stored by 'carbondata'")
       }
     
    -  test("test describe table") {
    +  ignore("test describe table") {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153102465
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/CarbonReflectionUtils.scala ---
    @@ -0,0 +1,86 @@
    +/*
    + * 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 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)) {
    --- End diff --
   
    return `typeOf[T].members.filter(!_.isMethod).toList.contains(name)` directly


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153102773
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/CarbonReflectionUtils.scala ---
    @@ -0,0 +1,86 @@
    +/*
    + * 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 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 = {
    --- End diff --
   
    Instead of providing this function, you can provide a function to extract `LogicalPlan` in `InsertIntoTable` direclty, so that CarbonDecoderProcessor does not need to call `getField`, `getField` can be private


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153102889
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/CarbonReflectionUtils.scala ---
    @@ -0,0 +1,86 @@
    +/*
    + * 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 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 = {
    --- End diff --
   
    You can add a function to return the spark version number in this file, getUnresolvedRelation can also use it.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153103018
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/CarbonReflectionUtils.scala ---
    @@ -0,0 +1,86 @@
    +/*
    + * 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 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
    --- End diff --
   
    code style should be like:
    val unresolvedrelation =
       ctor.newInstance(tableIdentifier, tableAlias).asInstanceOf[UnresolvedRelation]


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153103220
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonCatalystOperators.scala ---
    @@ -101,13 +101,15 @@ case class UpdateTable(
         table: UnresolvedRelation,
         columns: List[String],
         selectStmt: String,
    +    alias: Option[String],
    --- End diff --
   
    provide a default value for optional parameter


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153103233
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonCatalystOperators.scala ---
    @@ -101,13 +101,15 @@ case class UpdateTable(
         table: UnresolvedRelation,
         columns: List[String],
         selectStmt: String,
    +    alias: Option[String],
         filer: String) extends LogicalPlan {
       override def children: Seq[LogicalPlan] = Seq.empty
       override def output: Seq[AttributeReference] = Seq.empty
     }
     
     case class DeleteRecords(
         statement: String,
    +    alias: Option[String],
    --- End diff --
   
    provide a default value for optional parameter


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1469#discussion_r153103373
 
    --- Diff: integration/spark-common/src/main/scala/org/apache/spark/sql/optimizer/CarbonDecoderOptimizerHelper.scala ---
    @@ -84,7 +87,16 @@ class CarbonDecoderProcessor {
             }
             nodeList.add(ArrayCarbonNode(nodeListSeq))
           case e: UnaryNode => process(e.child, nodeList)
    -      case i: InsertIntoTable => process(i.child, nodeList)
    +      case i: InsertIntoTable =>
    +        var sparkVersion21: Boolean = false
    --- End diff --
   
    here, please give one comment to explain : sparkVersion21 is for spark
    2.1.0.  because in the future there are may many spark versions.


---