[GitHub] carbondata pull request #1665: [CARBONDATA-1884] Add CTAS support to carbond...

classic Classic list List threaded Threaded
39 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1665: [CARBONDATA-1884] Add CTAS support to carbond...

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

    https://github.com/apache/carbondata/pull/1665#discussion_r157405351
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonMetaStore.scala ---
    @@ -144,6 +145,15 @@ trait CarbonMetaStore {
       def getThriftTableInfo(tablePath: CarbonTablePath)(sparkSession: SparkSession): TableInfo
     
       def getTableFromMetadataCache(database: String, tableName: String): Option[CarbonTable]
    +
    +  def getCarbonDataSourceHadoopRelation(sparkSession: SparkSession,
    +      tableIdentifier: TableIdentifier): CarbonDatasourceHadoopRelation
    +
    +  def getSchemaFromUnresolvedRelation(sparkSession: SparkSession,
    --- End diff --
   
    This method is called from the parser and this logic for retrieving schema from unresolved relation, according to me should not return in parser. Please let me know if you have a different perspective


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1665: [CARBONDATA-1884] Add CTAS support to carbond...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user watermen commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1665#discussion_r157408233
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/table/CarbonCreateTableAsSelectCommand.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.sql.execution.command.table
    +
    +import scala.util.control.NonFatal
    +
    +import org.apache.spark.sql.{CarbonEnv, Row, SparkSession}
    +import org.apache.spark.sql.catalyst.TableIdentifier
    +import org.apache.spark.sql.catalyst.analysis.TableAlreadyExistsException
    +import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
    +import org.apache.spark.sql.execution.command.MetadataCommand
    +import org.apache.spark.sql.execution.command.management.CarbonInsertIntoCommand
    +
    +import org.apache.carbondata.common.logging.LogServiceFactory
    +import org.apache.carbondata.core.metadata.schema.table.TableInfo
    +
    +/**
    + * Create table and insert the query result into it.
    + *
    + * @param query the query whose result will be insert into the new relation
    + * @param tableInfo the Table Describe, which may contains serde, storage handler etc.
    + * @param ifNotExistsSet allow continue working if it's already exists, otherwise
    + *                      raise exception
    + * @param tableLocation store location where the table need to be created
    + */
    +case class CarbonCreateTableAsSelectCommand(query: LogicalPlan,
    +    tableInfo: TableInfo,
    +    ifNotExistsSet: Boolean = false,
    +    tableLocation: Option[String] = None) extends MetadataCommand {
    +
    +  override def processMetadata(sparkSession: SparkSession): Seq[Row] = {
    +    val LOGGER = LogServiceFactory.getLogService(this.getClass.getCanonicalName)
    +    val tableName = tableInfo.getFactTable.getTableName
    +    var databaseOpt: Option[String] = None
    +    if (tableInfo.getDatabaseName != null) {
    +      databaseOpt = Some(tableInfo.getDatabaseName)
    +    }
    +    val dbName = CarbonEnv.getDatabaseName(databaseOpt)(sparkSession)
    +    LOGGER.audit(s"Request received for CTAS for $dbName.$tableName")
    +    lazy val carbonDataSourceHadoopRelation = {
    --- End diff --
   
    Why you use lazy here?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1665: [CARBONDATA-1884] Add CTAS support to carbond...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user watermen commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1665#discussion_r157409856
 
    --- Diff: integration/spark2/src/main/spark2.1/CarbonSessionState.scala ---
    @@ -259,25 +259,26 @@ object CarbonOptimizerUtil {
       }
     }
     
    -class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser) extends
    -  SparkSqlAstBuilder(conf) {
    +class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSession: SparkSession)
    +  extends SparkSqlAstBuilder(conf) {
     
    -  val helper = new CarbonHelperSqlAstBuilder(conf, parser)
    +  val helper = new CarbonHelperSqlAstBuilder(conf, parser, sparkSession)
     
       override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
         val fileStorage = helper.getFileStorage(ctx.createFileFormat)
     
         if (fileStorage.equalsIgnoreCase("'carbondata'") ||
             fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) {
           helper.createCarbonTable(ctx.createTableHeader,
    --- End diff --
   
    move `ctx.createTableHeader` to next line


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1665: [CARBONDATA-1884] Add CTAS support to carbond...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user watermen commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1665#discussion_r157410277
 
    --- Diff: integration/spark2/src/main/spark2.2/CarbonSessionState.scala ---
    @@ -280,25 +280,26 @@ class CarbonOptimizer(
       }
     }
     
    -class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser) extends
    -  SparkSqlAstBuilder(conf) {
    +class CarbonSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser, sparkSession: SparkSession)
    +  extends SparkSqlAstBuilder(conf) {
     
    -  val helper = new CarbonHelperSqlAstBuilder(conf, parser)
    +  val helper = new CarbonHelperSqlAstBuilder(conf, parser, sparkSession)
     
       override def visitCreateHiveTable(ctx: CreateHiveTableContext): LogicalPlan = {
         val fileStorage = helper.getFileStorage(ctx.createFileFormat)
     
         if (fileStorage.equalsIgnoreCase("'carbondata'") ||
             fileStorage.equalsIgnoreCase("'org.apache.carbondata.format'")) {
           helper.createCarbonTable(ctx.createTableHeader,
    -          ctx.skewSpec,
    --- End diff --
   
    @jackylk We should use the right format at the first time, I mean use 2 blanks instead of 4 blanks. We should avoid unrelated modify, so we can keep each MR with less diff. Better to review and better to rebase.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/867/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    retest this please


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2093/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1665: [CARBONDATA-1884] Add CTAS support to carbond...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/1665#discussion_r157473214
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -210,13 +208,40 @@ class CarbonHelperSqlAstBuilder(conf: SQLConf, parser: CarbonSpark2SqlParser)
           }
         }
     
    -    val fields = parser.getFields(cols ++ partitionByStructFields)
    +    var fields = parser.getFields(cols ++ partitionByStructFields)
         val options = new CarbonOption(properties)
         // validate tblProperties
         val bucketFields = parser.getBucketFields(tableProperties, fields, options)
     
         validateStreamingProperty(options)
     
    +    // validate for create table as select
    +    val selectQuery = Option(query).map(plan)
    +    selectQuery match {
    +      case Some(q) =>
    +        // create table as select does not allow creation of partitioned table
    +        if (partitionFields.nonEmpty) {
    +          val errorMessage = "A Create Table As Select (CTAS) statement is not allowed to " +
    +                             "create a partitioned table using Carbondata file formats."
    +          operationNotAllowed(errorMessage, partitionColumns)
    +        }
    +        // create table as select does not allow to explicitly specify schema
    +        if (fields.nonEmpty) {
    +          operationNotAllowed(
    +            "Schema may not be specified in a Create Table As Select (CTAS) statement", columns)
    +        }
    +        // create table as select does not allow specifying any table properties
    +        if (tableProperties.nonEmpty) {
    --- End diff --
   
    I think this can be allowed, it is also in Hive syntax


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

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



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/873/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2098/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/890/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

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



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2115/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/891/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

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



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2402/



---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata issue #1665: [CARBONDATA-1884] Add CTAS support to carbondata

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user jackylk commented on the issue:

    https://github.com/apache/carbondata/pull/1665
 
    LGTM, thanks for working on this


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] carbondata pull request #1665: [CARBONDATA-1884] Add CTAS support to carbond...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user asfgit closed the pull request at:

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


---
12