[GitHub] carbondata pull request #2748: [CARBONDATA-2959] Added validations for TABLE...

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

[GitHub] carbondata pull request #2748: [CARBONDATA-2959] Added validations for TABLE...

qiuchenjian-2
GitHub user NamanRastogi opened a pull request:

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

    [CARBONDATA-2959] Added validations for TABLE OPTIONS

    create table validations are added for table OPTIONS while table is created using:
    ```
    CREATE TABLE <tableName> ( <columns> )
    USING carbon
    OPTIONS ( <options> )
    ```
    As the validations cannot be done at the table creation, it has to be done while loading data.
   
    Validations are done for LONG_STRING_COLUMNS, DICTIONARY_INCLUDE, DICTIONARY_EXCLUDE, and NO_INVERTED_INDEX
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [x] Testing done
           
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


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

    $ git pull https://github.com/NamanRastogi/carbondata b06

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

    https://github.com/apache/carbondata/pull/2748.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 #2748
   
----
commit 477e6faa3333af619645b5ff6cf66a09926dd6eb
Author: Naman Rastogi <naman.rastogi.52@...>
Date:   2018-09-21T14:49:00Z

    Added validation for 'LONG_STRING_COLUMNS' while using 'USING CARBON' to create table

----


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

[GitHub] carbondata issue #2748: [CARBONDATA-2959] Added validations for TABLE OPTION...

qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2748
 
    Can one of the admins verify this patch?


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

[GitHub] carbondata pull request #2748: [CARBONDATA-2959] Added validations for TABLE...

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

    https://github.com/apache/carbondata/pull/2748#discussion_r219658382
 
    --- Diff: integration/spark-datasource/src/main/scala/org/apache/spark/sql/carbondata/execution/datasources/CarbonSparkDataSourceUtil.scala ---
    @@ -269,4 +273,99 @@ object CarbonSparkDataSourceUtil {
         }
         model
       }
    +
    +  def validateTableOptions(options: Map[String, String], schema: Schema): Unit = {
    +
    +    if (options.contains(CarbonCommonConstants.DICTIONARY_EXCLUDE)) {
    --- End diff --
   
    NO need of this validation. it does not support here. Please remove.


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

[GitHub] carbondata pull request #2748: [CARBONDATA-2959] Added validations for TABLE...

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

    https://github.com/apache/carbondata/pull/2748#discussion_r219748229
 
    --- Diff: integration/spark-datasource/src/main/scala/org/apache/spark/sql/carbondata/execution/datasources/CarbonSparkDataSourceUtil.scala ---
    @@ -269,4 +273,94 @@ object CarbonSparkDataSourceUtil {
         }
         model
       }
    +
    +  def validateTableOptions(options: Map[String, String], schema: Schema): Unit = {
    --- End diff --
   
    I don't expect any validations here. We cannot validate all the properties during load. We can do validation only for the properties which it supports. And also those validations should be inside sdk interfaces not here.


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

[GitHub] carbondata pull request #2748: [CARBONDATA-2959] Added validations for TABLE...

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

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


---