vikramahuja1001 commented on a change in pull request #4141: URL: https://github.com/apache/carbondata/pull/4141#discussion_r645381006
########## File path: integration/spark/src/main/spark3.1/org/apache/spark/sql/execution/CarbonCodegenSupport.scala ########## @@ -0,0 +1,29 @@ +/* + * 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 + +import org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext +import org.apache.spark.sql.execution.joins.HashJoin + +trait CarbonCodegenSupport extends SparkPlan with HashJoin { + + // TODO: Spark has started supporting Codegen for Join, Carbon needs to implement the same. + override def supportCodegen: Boolean = false Review comment: okay https://issues.apache.org/jira/browse/CARBONDATA-4197 ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/secondaryindex/command/SICreationCommand.scala ########## @@ -704,5 +705,13 @@ private[sql] case class CarbonCreateSecondaryIndexCommand( columnSchema } + override def clone(): LogicalPlan = { + CarbonCreateSecondaryIndexCommand(indexModel, Review comment: this method is called from spark side ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/parser/CarbonSpark2SqlParser.scala ########## @@ -604,11 +605,12 @@ class CarbonSpark2SqlParser extends CarbonDDLSqlParser { protected lazy val alterTableColumnRenameAndModifyDataType: Parser[LogicalPlan] = - ALTER ~> TABLE ~> (ident <~ ".").? ~ ident ~ CHANGE ~ ident ~ ident ~ - ident ~ opt("(" ~> rep1sep(valueOptions, ",") <~ ")") <~ opt(";") ^^ { - case dbName ~ table ~ change ~ columnName ~ columnNameCopy ~ dataType ~ values => + ALTER ~> TABLE ~> (ident <~ ".").? ~ ident ~ (CHANGE ~> ident) ~ ident ~ ident ~ Review comment: it already exists ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DDLStrategy.scala ########## @@ -67,6 +68,8 @@ object DDLStrategy extends SparkStrategy { case changeColumn: AlterTableChangeColumnCommand if isCarbonTable(changeColumn.tableName) => ExecutedCommandExec(DDLHelper.changeColumn(changeColumn, sparkSession)) :: Nil + case changeColumn: CarbonAlterTableColRenameDataTypeChangeCommand => Review comment: Previously spark was parsing the command, now spark has removed the support. We could either alter the command definition to match with newly updated spark or could parse on our own. Since, the method was already in carbondata code, we decided to parse it in carbondata only. ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DDLStrategy.scala ########## @@ -67,6 +68,8 @@ object DDLStrategy extends SparkStrategy { case changeColumn: AlterTableChangeColumnCommand if isCarbonTable(changeColumn.tableName) => ExecutedCommandExec(DDLHelper.changeColumn(changeColumn, sparkSession)) :: Nil + case changeColumn: CarbonAlterTableColRenameDataTypeChangeCommand => Review comment: Previously spark was parsing the alter column command, now spark has removed the support. We could either alter the command definition to match with newly updated spark or could parse on our own. Since, the method was already in carbondata code, we decided to parse it in carbondata only. ########## File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/strategy/DDLStrategy.scala ########## @@ -67,6 +68,8 @@ object DDLStrategy extends SparkStrategy { case changeColumn: AlterTableChangeColumnCommand if isCarbonTable(changeColumn.tableName) => ExecutedCommandExec(DDLHelper.changeColumn(changeColumn, sparkSession)) :: Nil + case changeColumn: CarbonAlterTableColRenameDataTypeChangeCommand => Review comment: Previously spark was parsing the alter column rename command, now spark has removed the support. We could either alter the command definition to match with newly updated spark or could parse on our own. Since, the method was already in carbondata code, we decided to parse it in carbondata only. ########## File path: integration/spark/src/main/scala/org/apache/carbondata/spark/util/CarbonSparkUtil.scala ########## @@ -181,7 +182,7 @@ object CarbonSparkUtil { */ def createHadoopJob(conf: Configuration = FileFactory.getConfiguration): Job = { val jobConf = new JobConf(conf) - SparkHadoopUtil.get.addCredentials(jobConf) + SparkUtil.addCredentials(jobConf) Review comment: spark has made addCredentials method private to spark, can't access it directly from carbondata package -- 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]
