[GitHub] carbondata pull request #1813: [CARBONDATA-2015] Restricted maximum length o...

[qiuchenjian-2]

GitHub user dhatchayani opened a pull request:

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

    [CARBONDATA-2015] Restricted maximum length of bytes per column

    Validation for number of bytes for a column is added.
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [x] Testing done
            Manual Testing
   
     - [ ] 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/dhatchayani/incubator-carbondata 32000bytes_1

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

    https://github.com/apache/carbondata/pull/1813.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 #1813
   
----
commit 4378d9f700b6edb0510adc54aa22f73c70a820f8
Author: dhatchayani <dhatcha.official@...>
Date:   2018-01-16T10:18:32Z

    [CARBONDATA-2015] Restricted maximum length of bytes per column

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/1813
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1604/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1813#discussion_r161963254
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/converter/impl/NonDictionaryFieldConverterImpl.java ---
    @@ -70,13 +70,14 @@ public NonDictionaryFieldConverterImpl(DataField dataField, String nullformat, i
           try {
             byte[] value = DataTypeUtil
                 .getBytesBasedOnDataTypeForNoDictionaryColumn(dimensionValue, dataType, dateFormat);
    -        if (dataType == DataTypes.STRING) {
    -          assert value.length <= CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT;
    +        if (dataType == DataTypes.STRING
    +            && value.length > CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT) {
    +          throw new CarbonDataLoadingException("Dataload failed, String size cannot exceed "
    +              + CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT + " bytes");
             }
             row.update(value, index);
    -      } catch (AssertionError ae) {
    -        throw new CarbonDataLoadingException("Dataload failed, String size cannot exceed "
    -            + CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT + " bytes");
    +      } catch (CarbonDataLoadingException e) {
    --- End diff --
   
    Kindly check and confirm if CarbonDataLoadingException can come from any other place also in this flow


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1813#discussion_r161963513
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/loading/converter/impl/NonDictionaryFieldConverterImpl.java ---
    @@ -70,13 +70,14 @@ public NonDictionaryFieldConverterImpl(DataField dataField, String nullformat, i
           try {
             byte[] value = DataTypeUtil
                 .getBytesBasedOnDataTypeForNoDictionaryColumn(dimensionValue, dataType, dateFormat);
    -        if (dataType == DataTypes.STRING) {
    -          assert value.length <= CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT;
    +        if (dataType == DataTypes.STRING
    +            && value.length > CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT) {
    +          throw new CarbonDataLoadingException("Dataload failed, String size cannot exceed "
    +              + CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT + " bytes");
             }
             row.update(value, index);
    -      } catch (AssertionError ae) {
    -        throw new CarbonDataLoadingException("Dataload failed, String size cannot exceed "
    -            + CarbonCommonConstants.MAX_CHARS_PER_COLUMN_DEFAULT + " bytes");
    +      } catch (CarbonDataLoadingException e) {
    --- End diff --
   
    No. There are possibilities of only ParseException and NumberFormatException


---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

    https://github.com/apache/carbondata/pull/1813
 
    LGTM


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---

[qiuchenjian-2]

Github user shining1989 commented on the issue:

    https://github.com/apache/carbondata/pull/1813
 
    Why Validation for number of bytes for a column is added?
   
    Now I have a column more than 32000 character, insert will failed with error  "String length cannot exceed 32000 characters".
   
    How can i avoid this problem?


---