[GitHub] incubator-carbondata pull request #512: [CARBONDATA-582]added validation for...

[qiuchenjian-2]

GitHub user anubhav100 opened a pull request:

    https://github.com/apache/incubator-carbondata/pull/512

    [CARBONDATA-582]added validation for bucket number is not negative

    when we create table with bucket number is specified in negative it creates table
   
    CREATE TABLE BucketTest1111352983(name int,ID string)USING org.apache.spark.sql.CarbonSource OPTIONS("bucketnumber"="-1", "bucketcolumns"="name1");
    +---------+--+
    | Result  |
    +---------+--+
    +---------+--+
    No rows selected (0.277 seconds)
   
    this is because there is no check whether the bucket number given is positive or not
   
    in hive it gives a error
   
    here are the logs in hive
   
    hive> CREATE TABLE BucketTest1111352983(name int,ID string)USING org.apache.spark.sql.CarbonSource OPTIONS("bucketnumber"="-1", "bucketcolumns"="name1");
    +---------+--+
    | Result  |
    +---------+--+
    +---------+--+
    No rows selected (0.277 seconds)
     in carbon data as well it should not allow this
   
    in this pr we resolved this issue
     

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

    $ git pull https://github.com/anubhav100/incubator-carbondata CARBONDATA-582

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

    https://github.com/apache/incubator-carbondata/pull/512.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 #512
   
----
commit d60b6a2e3a2775818019c8cce5d34e86641d909b
Author: anubhav100 <[hidden email]>
Date:   2017-01-10T05:31:52Z

    added validation for bucket number is not negative

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/528/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/529/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/530/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/incubator-carbondata/pull/512#discussion_r95524857
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/bucketing/TableBucketingTestCase.scala ---
    @@ -63,6 +65,23 @@ class TableBucketingTestCase extends QueryTest with BeforeAndAfterAll {
         }
       }
     
    +  test("must be unable to create if number of buckets is in negative number"){
    +    try {
    +      sql(
    +        """
    +           CREATE TABLE t9
    +           (ID Int, date Timestamp, country String,
    +           name String, phonetype String, serialname String, salary Int)
    +           USING org.apache.spark.sql.CarbonSource
    +           OPTIONS("bucketnumber"="-1", "bucketcolumns"="name", "tableName"="t9")
    +      """)
    +      assert(false)
    +      }
    --- End diff --
   
    incorrect indentation


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/incubator-carbondata/pull/512#discussion_r95524878
 
    --- Diff: integration/spark2/src/test/scala/org/apache/spark/carbondata/bucketing/TableBucketingTestCase.scala ---
    @@ -63,6 +65,23 @@ class TableBucketingTestCase extends QueryTest with BeforeAndAfterAll {
         }
       }
     
    +  test("must be unable to create if number of buckets is in negative number"){
    +    try {
    +      sql(
    +        """
    +           CREATE TABLE t9
    +           (ID Int, date Timestamp, country String,
    +           name String, phonetype String, serialname String, salary Int)
    +           USING org.apache.spark.sql.CarbonSource
    +           OPTIONS("bucketnumber"="-1", "bucketcolumns"="name", "tableName"="t9")
    +      """)
    +      assert(false)
    +      }
    +    catch {
    --- End diff --
   
    move to previous line


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/incubator-carbondata/pull/512#discussion_r95524989
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -138,7 +138,13 @@ class CarbonSqlAstBuilder(conf: SQLConf) extends SparkSqlAstBuilder(conf) {
           val options = new CarbonOption(properties)
           val bucketFields = {
    --- End diff --
   
    this bracket is not required


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/incubator-carbondata/pull/512#discussion_r95525006
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/parser/CarbonSparkSqlParser.scala ---
    @@ -138,7 +138,13 @@ class CarbonSqlAstBuilder(conf: SQLConf) extends SparkSqlAstBuilder(conf) {
           val options = new CarbonOption(properties)
           val bucketFields = {
             if (options.isBucketingEnabled) {
    -          Some(BucketFields(options.bucketColumns.split(","), options.bucketNumber))
    +          if (options.bucketNumber.toString.contains("-") ||
    +             options.bucketNumber.toString.contains("+")) {
    +            throw new MalformedCarbonCommandException("INVALID NUMBER OF BUCKETS SPECIFIED")
    +          }
    +            else {
    --- End diff --
   
    move to previous line


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

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

    https://github.com/apache/incubator-carbondata/pull/512#discussion_r95525037
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -134,7 +135,14 @@ class CarbonSource extends CreatableRelationProvider
             parameters.foreach { x => map.put(x._1, x._2) }
             val bucketFields = {
    --- End diff --
   
    same as comment below


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/576/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/578/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/580/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user anubhav100 commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    @jackylk done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    LGTM
    @ravipesala please check also


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/512
 
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-carbondata/pull/512


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---