[GitHub] carbondata pull request #2747: [CARBONDATA-2960] SDK Reader fix with project...

[qiuchenjian-2]

GitHub user manishnalla1994 opened a pull request:

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

    [CARBONDATA-2960] SDK Reader fix with projection columns

    SDK Reader was not working when all projection columns were given.
    Added exception for Complex child projections too.
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     
     - [ ] Any backward compatibility impacted?
     
     - [ ] Document update required?
   
     - [x] Testing done
           
     - [ ] 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/manishnalla1994/carbondata reader_fix

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

    https://github.com/apache/carbondata/pull/2747.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 #2747
   
----
commit 1c458df5ca3c274a3028bc8263c9b2758588c4e6
Author: Manish Nalla <manishnalla1994@...>
Date:   2018-09-21T13:54:01Z

    SDK Reader with Default Projection

commit b684ab22d42d749364de8a46317b4c43a2d67d2c
Author: Manish Nalla <manishnalla1994@...>
Date:   2018-09-21T14:05:38Z

    SDK Reader with Default Projection(1)

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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


---

[qiuchenjian-2]

Github user brijoobopanna commented on the issue:

    https://github.com/apache/carbondata/pull/2747
 
    retest this please



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2747
 
    Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/600/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2747
 
    Build Failed  with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8670/



---

[qiuchenjian-2]

Github user manishnalla1994 commented on the issue:

    https://github.com/apache/carbondata/pull/2747
 
    retest this please


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2747#discussion_r219754646
 
    --- Diff: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java ---
    @@ -775,9 +775,18 @@ public static boolean getAccessStreamingSegments(Configuration configuration) {
       public String[] projectAllColumns(CarbonTable carbonTable) {
         List<ColumnSchema> colList = carbonTable.getTableInfo().getFactTable().getListOfColumns();
         List<String> projectColumn = new ArrayList<>();
    +    int childDimCount = 0;
         for (ColumnSchema cols : colList) {
           if (cols.getSchemaOrdinal() != -1) {
    -        projectColumn.add(cols.getColumnName());
    +        if (childDimCount == 0) {
    --- End diff --
   
    please add comment


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2747#discussion_r219757736
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonReaderBuilder.java ---
    @@ -205,6 +205,13 @@ public CarbonReaderBuilder setEndPoint(String value) {
     
         if (projectionColumns != null) {
           // set the user projection
    +      int len = projectionColumns.length;
    +      for (int i = 0; i < len; i++) {
    +        if (projectionColumns[i].contains(".")) {
    --- End diff --
   
    Add some comments here


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2747#discussion_r219758599
 
    --- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/createTable/TestNonTransactionalCarbonTableForMapType.scala ---
    @@ -401,6 +406,89 @@ class TestNonTransactionalCarbonTableForMapType extends QueryTest with BeforeAnd
         dropSchema
       }
     
    +  test("SDK Reader Without Projection Columns"){
    +    deleteDirectory(writerPath)
    +    val mySchema =
    +      """
    +        |{
    +        |  "name": "address",
    +        |  "type": "record",
    +        |  "fields": [
    +        |    {
    +        |      "name": "name",
    +        |      "type": "string"
    +        |    },
    +        |    {
    +        |      "name": "age",
    +        |      "type": "int"
    +        |    },
    +        |    {
    +        |      "name": "arrayRecord",
    +        |      "type": {
    +        |        "type": "array",
    +        |        "items": {
    +        |           "name": "houseDetails",
    +        |           "type": "map",
    +        |           "values": "string"
    +        |        }
    +        |      }
    +        |    }
    +        |  ]
    +        |}
    +      """.stripMargin
    +    val json = """ {"name":"bob", "age":10, "arrayRecord": [{"101": "Rahul", "102": "Pawan"}]} """.stripMargin
    +    nonTransactionalCarbonTable.WriteFilesWithAvroWriter(2, mySchema, json)
    +
    +    val reader = CarbonReader.builder(writerPath, "_temp").isTransactionalTable(false).build(conf)
    +    reader.close()
    +    println("Done test")
    +  }
    +
    +  test("SDK Reader Without Projection Columns1"){
    +    deleteDirectory(writerPath)
    +    val mySchema =
    +      """
    +        |{
    +        |  "name": "address",
    +        |  "type": "record",
    +        |  "fields": [
    +        |    {
    +        |      "name": "name",
    +        |      "type": "string"
    +        |    },
    +        |    {
    +        |      "name": "age",
    +        |      "type": "int"
    +        |    },
    +        |    {
    +        |      "name": "arrayRecord",
    +        |      "type": {
    +        |        "type": "array",
    +        |        "items": {
    +        |           "name": "houseDetails",
    +        |           "type": "map",
    +        |           "values": "string"
    +        |        }
    +        |      }
    +        |    }
    +        |  ]
    +        |}
    +      """.stripMargin
    +    val json = """ {"name":"bob", "age":10, "arrayRecord": [{"101": "Rahul", "102": "Pawan"}]} """.stripMargin
    +    nonTransactionalCarbonTable.WriteFilesWithAvroWriter(2, mySchema, json)
    +
    +    val exception1 = intercept[Exception] {
    --- End diff --
   
    Handle this in above test case itself. As we are testing reader. No need to write again and read it.


---

[qiuchenjian-2]

Github user shardul-cr7 commented on the issue:

    https://github.com/apache/carbondata/pull/2747
 
    retest this please


---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

    https://github.com/apache/carbondata/pull/2747
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2747
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/8695/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2747#discussion_r220062362
 
    --- Diff: hadoop/src/main/java/org/apache/carbondata/hadoop/api/CarbonInputFormat.java ---
    @@ -775,9 +775,18 @@ public static boolean getAccessStreamingSegments(Configuration configuration) {
       public String[] projectAllColumns(CarbonTable carbonTable) {
         List<ColumnSchema> colList = carbonTable.getTableInfo().getFactTable().getListOfColumns();
         List<String> projectColumn = new ArrayList<>();
    +    int childDimCount = 0;
         for (ColumnSchema cols : colList) {
           if (cols.getSchemaOrdinal() != -1) {
    -        projectColumn.add(cols.getColumnName());
    +        if (childDimCount == 0) {
    --- End diff --
   
    added and updated


---

[qiuchenjian-2]

Github user kumarvishal09 commented on the issue:

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


---

[qiuchenjian-2]

Github user manishnalla1994 closed the pull request at:

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


---