[GitHub] carbondata pull request #3050: Optimize the upper/lower case problem

[qiuchenjian-2]

GitHub user bbinwang opened a pull request:

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

    Optimize the upper/lower case problem

    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ no] Any interfaces changed?
     
     - [ no] Any backward compatibility impacted?
     
     - [ no] Document update required?
   
     - [ no] Testing done
            Please provide details on
            - Whether new unit test cases have been added or why no new tests are required?
            - How it is tested? Please attach test report.
            - Is it a performance related change? Please attach the performance test report.
            - Any additional information to help reviewers in testing this change.
           
     - [yes ] 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/bbinwang/carbondata master

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

    https://github.com/apache/carbondata/pull/3050.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 #3050
   
----
commit d53a178dea88fc28b3944c443cc3868a9027005b
Author: bbinwang <binw305@...>
Date:   2019-01-02T14:42:10Z

    Merge pull request #1 from apache/master
   
    pull

commit ea3b157638f2dcd0d88c2aa34246a210fbb54d00
Author: [hidden email] <513338github>
Date:   2019-01-03T15:22:51Z

    Optimize the upper/lower case problem

commit b4bb5fba4d9b37a5797d2030e9e916574d4795dd
Author: [hidden email] <513338github>
Date:   2019-01-03T15:29:01Z

    Optimize the upper/lower case problem

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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


---

[qiuchenjian-2]

Github user bbinwang commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    > Can one of the admins verify this patch?
   
    are you machine？


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    @bbinwang Please optimize the title and content of this PR. for example: [CARBONDATA-3226] Remove duplicated and useless files


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3050#discussion_r245244853
 
    --- Diff: datamap/lucene/src/main/java/org/apache/carbondata/datamap/lucene/LuceneFineGrainDataMapFactory.java ---
    @@ -57,7 +57,7 @@ public LuceneFineGrainDataMapFactory(CarbonTable carbonTable, DataMapSchema data
               DataMapWriter.getDefaultDataMapPath(tableIdentifier.getTablePath(),
                   segment.getSegmentNo(), dataMapName), segment.getConfiguration()));
         } catch (MemoryException e) {
    -      LOGGER.error("failed to get lucene datamap , detail is {}" + e.getMessage());
    +      LOGGER.error("failed to get lucene datamap, detail is {}" + e.getMessage());
    --- End diff --
   
    ```suggestion
          LOGGER.error(String.format("failed to get lucene datamap, detail is %s", e.getMessage()));
    ```
    I think this log can't print the exception message


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    @bbinwang Please optimize the title to:[CARBONDATA-3211] Optimize the documentation


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3050#discussion_r245328482
 
    --- Diff: datamap/lucene/src/main/java/org/apache/carbondata/datamap/lucene/LuceneFineGrainDataMapFactory.java ---
    @@ -57,7 +57,7 @@ public LuceneFineGrainDataMapFactory(CarbonTable carbonTable, DataMapSchema data
               DataMapWriter.getDefaultDataMapPath(tableIdentifier.getTablePath(),
                   segment.getSegmentNo(), dataMapName), segment.getConfiguration()));
         } catch (MemoryException e) {
    -      LOGGER.error("failed to get lucene datamap , detail is {}" + e.getMessage());
    +      LOGGER.error("failed to get lucene datamap, detail is {}" + e.getMessage());
    --- End diff --
   
    +1 for @qiuchenjian


---

[qiuchenjian-2]

Github user bbinwang commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    > @bbinwang Please optimize the title to:[CARBONDATA-3211] Optimize the documentation
   
    ok


---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    add to whitelist


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3050#discussion_r245534776
 
    --- Diff: datamap/lucene/src/main/java/org/apache/carbondata/datamap/lucene/LuceneFineGrainDataMapFactory.java ---
    @@ -57,7 +57,7 @@ public LuceneFineGrainDataMapFactory(CarbonTable carbonTable, DataMapSchema data
               DataMapWriter.getDefaultDataMapPath(tableIdentifier.getTablePath(),
                   segment.getSegmentNo(), dataMapName), segment.getConfiguration()));
         } catch (MemoryException e) {
    -      LOGGER.error("failed to get lucene datamap , detail is {}" + e.getMessage());
    +      LOGGER.error("failed to get lucene datamap, detail is {}" + e.getMessage());
    --- End diff --
   
    +1, it should can print the exception message, but String.format is better.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2178/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10434/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2393/



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/3050#discussion_r245696590
 
    --- Diff: datamap/lucene/src/main/java/org/apache/carbondata/datamap/lucene/LuceneFineGrainDataMapFactory.java ---
    @@ -57,7 +57,7 @@ public LuceneFineGrainDataMapFactory(CarbonTable carbonTable, DataMapSchema data
               DataMapWriter.getDefaultDataMapPath(tableIdentifier.getTablePath(),
                   segment.getSegmentNo(), dataMapName), segment.getConfiguration()));
         } catch (MemoryException e) {
    -      LOGGER.error("failed to get lucene datamap , detail is {}" + e.getMessage());
    +      LOGGER.error("failed to get lucene datamap, detail is {}" + e.getMessage());
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2202/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10459/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2419/



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/3050
 
    LGTM! Thanks for you contribution!


---

[qiuchenjian-2]

Github user zzcclp commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---