[GitHub] carbondata pull request #2211: [CARBONDATA-2380][DataMap] Support visible/in...

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

Github user jackylk commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2211#discussion_r189491069
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datamap/CGDataMapTestCase.scala ---
    @@ -383,6 +384,39 @@ class CGDataMapTestCase extends QueryTest with BeforeAndAfterAll {
           sql("select * from normal_test where name='n502670' and city='c2670'"))
       }
     
    +  test("test invisible datamap during query") {
    --- End diff --
   
    This test case shouldn't test the invisible/visible table directly, can you add some test case for checking invisible/visible table? Are there any performance report for this?


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2211#discussion_r189502015
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datamap/CGDataMapTestCase.scala ---
    @@ -383,6 +384,39 @@ class CGDataMapTestCase extends QueryTest with BeforeAndAfterAll {
           sql("select * from normal_test where name='n502670' and city='c2670'"))
       }
     
    +  test("test invisible datamap during query") {
    --- End diff --
   
    What's your proposal?
    The invisible datamap is a temporary status for a datamap. It indicates that the query will not use the datamap for now.
    See Line404 for usage.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2211#discussion_r189502270
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datamap/CGDataMapTestCase.scala ---
    @@ -383,6 +384,39 @@ class CGDataMapTestCase extends QueryTest with BeforeAndAfterAll {
           sql("select * from normal_test where name='n502670' and city='c2670'"))
       }
     
    +  test("test invisible datamap during query") {
    --- End diff --
   
    There is no need to add a performance report here.
    You can explain the query before/after change the visibility of the datamap, the output should be different.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2211#discussion_r189502836
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datamap/CGDataMapTestCase.scala ---
    @@ -383,6 +384,39 @@ class CGDataMapTestCase extends QueryTest with BeforeAndAfterAll {
           sql("select * from normal_test where name='n502670' and city='c2670'"))
       }
     
    +  test("test invisible datamap during query") {
    --- End diff --
   
    I think we should add some validate in test case, which ensure the function correct.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2211#discussion_r189556850
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datamap/CGDataMapTestCase.scala ---
    @@ -383,6 +384,39 @@ class CGDataMapTestCase extends QueryTest with BeforeAndAfterAll {
           sql("select * from normal_test where name='n502670' and city='c2670'"))
       }
     
    +  test("test invisible datamap during query") {
    --- End diff --
   
    Yes. We can validate it by validating the output of explain.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2211#discussion_r189557032
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/datamap/CGDataMapTestCase.scala ---
    @@ -383,6 +384,39 @@ class CGDataMapTestCase extends QueryTest with BeforeAndAfterAll {
           sql("select * from normal_test where name='n502670' and city='c2670'"))
       }
     
    +  test("test invisible datamap during query") {
    --- End diff --
   
    @xubo245 Will you work on it or just raise a jira?


---