[GitHub] incubator-carbondata pull request #511: [CARBONDATA-584]added validation for...

[qiuchenjian-2]

GitHub user anubhav100 opened a pull request:

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

    [CARBONDATA-584]added validation for table is not empty

   
       

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

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

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

    https://github.com/apache/incubator-carbondata/pull/511.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 #511
   
----
commit 05bd1ea17c1f088cbcd6e829f561543913ad5fba
Author: anubhav100 <[hidden email]>
Date:   2017-01-09T15:58:26Z

    added validation for table is not empty and don not contain any spaces

----


---
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/511
 
    Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/526/



---
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/511
 
    Please provide description to this PR. Are you using CarbonSession or SparkSession? How to reproduce this bug?


---
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/511
 
    Build Success with Spark 1.5.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/531/



---
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/511#discussion_r95702215
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +111,9 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(tableName.isEmpty || tableName.contains("")) {
    --- End diff --
   
    I think you can use `StringUtils.isBlank` utility function


---
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/511
 
    @jackylk sure 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 CarbonDataQA commented on the issue:

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



---
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/511#discussion_r96005860
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    --- End diff --
   
    add space after `if`
    import this class at the beginning of file


---
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/511#discussion_r96005973
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    --- End diff --
   
    I think `isBlank` will check whether it is space, so this 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/511#discussion_r96006080
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    +      throw new MalformedCarbonCommandException("INVALID TABLE NAME")
    --- End diff --
   
    You can say it is empty table name


---
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/511#discussion_r96006298
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    +      throw new MalformedCarbonCommandException("INVALID TABLE NAME")
    --- End diff --
   
    You can say it is empty table name


---
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 a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/511#discussion_r96021165
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    +      throw new MalformedCarbonCommandException("INVALID TABLE NAME")
    --- End diff --
   
    @jackylk  i think emptyTableName will make less sense because it is not gurranted that it will be empty


---
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 a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/511#discussion_r96021180
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    +      throw new MalformedCarbonCommandException("INVALID TABLE NAME")
    --- End diff --
   
    @jackylk sure i am correcting it


---
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 a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/511#discussion_r96021932
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    --- End diff --
   
    @jackylk  but what if table name is "a n u b h a v" i think it will not check for its validation


---
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/511#discussion_r96022013
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    +      throw new MalformedCarbonCommandException("INVALID TABLE NAME")
    --- End diff --
   
    if the if check is true, it means the table name passing by user is blank


---
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/511#discussion_r96022199
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonSource.scala ---
    @@ -108,6 +110,10 @@ class CarbonSource extends CreatableRelationProvider
     
         val dbName: String = parameters.getOrElse("dbName", CarbonCommonConstants.DATABASE_DEFAULT_NAME)
         val tableName: String = parameters.getOrElse("tableName", "default_table")
    +    if(org.apache.commons.lang.StringUtils.isBlank(tableName) ||
    +       org.apache.commons.lang.StringUtils.isWhitespace(tableName)) {
    --- End diff --
   
    In this case, it will fail in parser. It will not come to here


---
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/511
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/593/



---
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/511
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/594/



---
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/511
 
    @jackylk all issues are resolved


---
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/511
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/595/



---
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.
---