[GitHub] carbondata pull request #2378: [CARBONDATA-2614] Fix the error when using FG...

[qiuchenjian-2]

GitHub user xubo245 opened a pull request:

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

    [CARBONDATA-2614] Fix the error when using FG in search mode and the prune result is none

   
     the prune result is none, and can not set datamapWritePath, which will not generate bitSegGroup in org.apache.carbondata.core.scan.filter.executer.RowLevelFilterExecuterImpl#applyFilter(org.apache.carbondata.core.scan.processor.RawBlockletColumnChunks, boolean), it mean the bitSetGroup is null. It will throw Fix the error when using FG in search mode and the prune result is none
   
   
     - [ ] Any interfaces changed?
     No
     - [ ] Any backward compatibility impacted?
     No
     - [ ] Document update required?
    No
     - [ ] Testing done
         run the test case
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NO

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

    $ git pull https://github.com/xubo245/carbondata CARBONDATA-2614-searchModeFGError

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

    https://github.com/apache/carbondata/pull/2378.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 #2378
   
----
commit 21168176d6027836b6a7f4fa972513434581e5da
Author: xubo245 <xubo29@...>
Date:   2018-06-15T10:50:26Z

    [CARBONDATA-2614] Fix the error when using FG in search mode and the prune result is none
   
     the prune result is none, and can not set datamapWritePath, which will not generate bitSegGroup in org.apache.carbondata.core.scan.filter.executer.RowLevelFilterExecuterImpl#applyFilter(org.apache.carbondata.core.scan.processor.RawBlockletColumnChunks, boolean), it mean the bitSetGroup is null. It will throw Fix the error when using FG in search mode and the prune result is none

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195890375
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SearchModeTestCase.scala ---
    @@ -111,22 +111,26 @@ class SearchModeTestCase extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test lucene datamap with search mode") {
    +    sql("set carbon.search.enabled = true")
         sql("DROP DATAMAP IF EXISTS dm ON TABLE main")
         sql("CREATE DATAMAP dm ON TABLE main USING 'lucene' DMProperties('INDEX_COLUMNS'='id') ")
    +    sql(s"SELECT * FROM main WHERE id='100000'").show()
    --- End diff --
   
    This is not required, remove it


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195891261
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SearchModeTestCase.scala ---
    @@ -111,22 +111,26 @@ class SearchModeTestCase extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test lucene datamap with search mode") {
    +    sql("set carbon.search.enabled = true")
    --- End diff --
   
    Is this required? Search mode is already started


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195891272
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SearchModeTestCase.scala ---
    @@ -111,22 +111,26 @@ class SearchModeTestCase extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test lucene datamap with search mode") {
    +    sql("set carbon.search.enabled = true")
         sql("DROP DATAMAP IF EXISTS dm ON TABLE main")
         sql("CREATE DATAMAP dm ON TABLE main USING 'lucene' DMProperties('INDEX_COLUMNS'='id') ")
    +    sql(s"SELECT * FROM main WHERE id='100000'").show()
         checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('id:100000')"),
           sql(s"SELECT * FROM main WHERE id='100000'"))
         sql("DROP DATAMAP if exists dm ON TABLE main")
       }
     
       test("test lucene datamap with search mode 2") {
    +    sql("set carbon.search.enabled = true")
    --- End diff --
   
    Is this required? Search mode is already started


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195891284
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SearchModeTestCase.scala ---
    @@ -111,22 +111,26 @@ class SearchModeTestCase extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test lucene datamap with search mode") {
    +    sql("set carbon.search.enabled = true")
         sql("DROP DATAMAP IF EXISTS dm ON TABLE main")
         sql("CREATE DATAMAP dm ON TABLE main USING 'lucene' DMProperties('INDEX_COLUMNS'='id') ")
    +    sql(s"SELECT * FROM main WHERE id='100000'").show()
         checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('id:100000')"),
           sql(s"SELECT * FROM main WHERE id='100000'"))
         sql("DROP DATAMAP if exists dm ON TABLE main")
       }
     
       test("test lucene datamap with search mode 2") {
    +    sql("set carbon.search.enabled = true")
         sql("drop datamap if exists dm3 ON TABLE main")
         sql("CREATE DATAMAP dm3 ON TABLE main USING 'lucene' DMProperties('INDEX_COLUMNS'='city') ")
    -    checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('city:city6')"),
    -      sql("SELECT * FROM main WHERE city='city6'"))
    +    checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('city:city5')"),
    --- End diff --
   
    why modify the filter condition? `city6` can not work?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195891403
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/model/QueryModel.java ---
    @@ -109,6 +109,11 @@
        */
       private boolean requiredRowId;
     
    +  /**
    +   * whether it is FG with search mode
    +   */
    +  private boolean isFG;
    --- End diff --
   
    I think this is not needed, you can change the behavior in `SearchRequestHandler.handleRequest`. If the FGDatamap is used, then do not call `queryModel.setTableBlockInfos(list);` in `handleRequest`


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195934496
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SearchModeTestCase.scala ---
    @@ -111,22 +111,26 @@ class SearchModeTestCase extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test lucene datamap with search mode") {
    +    sql("set carbon.search.enabled = true")
    --- End diff --
   
    Required. Because the previous test case close the search mode by set


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195934512
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SearchModeTestCase.scala ---
    @@ -111,22 +111,26 @@ class SearchModeTestCase extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test lucene datamap with search mode") {
    +    sql("set carbon.search.enabled = true")
         sql("DROP DATAMAP IF EXISTS dm ON TABLE main")
         sql("CREATE DATAMAP dm ON TABLE main USING 'lucene' DMProperties('INDEX_COLUMNS'='id') ")
    +    sql(s"SELECT * FROM main WHERE id='100000'").show()
         checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('id:100000')"),
           sql(s"SELECT * FROM main WHERE id='100000'"))
         sql("DROP DATAMAP if exists dm ON TABLE main")
       }
     
       test("test lucene datamap with search mode 2") {
    +    sql("set carbon.search.enabled = true")
    --- End diff --
   
    this one can be removed


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195934536
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/detailquery/SearchModeTestCase.scala ---
    @@ -111,22 +111,26 @@ class SearchModeTestCase extends QueryTest with BeforeAndAfterAll {
       }
     
       test("test lucene datamap with search mode") {
    +    sql("set carbon.search.enabled = true")
         sql("DROP DATAMAP IF EXISTS dm ON TABLE main")
         sql("CREATE DATAMAP dm ON TABLE main USING 'lucene' DMProperties('INDEX_COLUMNS'='id') ")
    +    sql(s"SELECT * FROM main WHERE id='100000'").show()
         checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('id:100000')"),
           sql(s"SELECT * FROM main WHERE id='100000'"))
         sql("DROP DATAMAP if exists dm ON TABLE main")
       }
     
       test("test lucene datamap with search mode 2") {
    +    sql("set carbon.search.enabled = true")
         sql("drop datamap if exists dm3 ON TABLE main")
         sql("CREATE DATAMAP dm3 ON TABLE main USING 'lucene' DMProperties('INDEX_COLUMNS'='city') ")
    -    checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('city:city6')"),
    -      sql("SELECT * FROM main WHERE city='city6'"))
    +    checkAnswer(sql("SELECT * FROM main WHERE TEXT_MATCH('city:city5')"),
    --- End diff --
   
    before city5 can work, but city6 can not work.  After this PR, city6 can work


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2378#discussion_r195935021
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/scan/model/QueryModel.java ---
    @@ -109,6 +109,11 @@
        */
       private boolean requiredRowId;
     
    +  /**
    +   * whether it is FG with search mode
    +   */
    +  private boolean isFG;
    --- End diff --
   
    but it will call queryModel.setTableBlockInfos(list) in org.apache.carbondata.hadoop.CarbonRecordReader#initialize, how to change it?


---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---