[GitHub] carbondata pull request #1602: [CARBONDATA-1843] Add configuration to enable...

[qiuchenjian-2]

GitHub user jackylk opened a pull request:

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

    [CARBONDATA-1843]  Add configuration to enable dictionary and complex type

    Improvement:
    1. To improve usability, new carbon property is added to support dictionary and complex type, user can disable these features if not required
    2. Block 'external', 'CTAS' syntax
    3. Some other minor fix to catch exceptions
   
     - [X] Any interfaces changed?
     No
     - [X] Any backward compatibility impacted?
     No
     - [X] Document update required?
    No
     - [X] Testing done
    Yes
     - [X] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NA

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jackylk/incubator-carbondata basic

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/1602.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1602
   
----
commit 413fd02d640cfdadf1e39b5d923dc9c8337590cf
Author: Jacky Li <[hidden email]>
Date:   2017-12-02T11:02:10Z

    add config

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1602#discussion_r154514538
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -18,6 +18,7 @@ package org.apache.spark.sql.parser
     
     import scala.collection.mutable
     
    +import org.antlr.v4.runtime.tree.TerminalNode
    --- End diff --
   
    I think it is better handle these validations at precreate table listener so that main logic will be clean.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1602#discussion_r154519644
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -18,6 +18,7 @@ package org.apache.spark.sql.parser
     
     import scala.collection.mutable
     
    +import org.antlr.v4.runtime.tree.TerminalNode
    --- End diff --
   
    You mean to move all validation logic to precreate table listener or just the CTAS validation?
    Anyway, CTAS will be implement soon, so that check is just temporary.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1602#discussion_r154523178
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -18,6 +18,7 @@ package org.apache.spark.sql.parser
     
     import scala.collection.mutable
     
    +import org.antlr.v4.runtime.tree.TerminalNode
    --- End diff --
   
    No, I mean these type of validations to move to listeners, actually we no need to add these validations to carbon layer.  If other projects who want to block these operations can add their listener and just register it. I feel these configurations are not required.


---

[qiuchenjian-2]

Github user jackylk closed the pull request at:

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


---