[GitHub] carbondata pull request #2187: [CARBONDATA-2362] Changing the Cacheable obje...

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

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


---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4529/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5395/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4531/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4534/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5398/



---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    Retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2187#discussion_r184278866
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/datastore/DataRefNode.java ---
    @@ -141,5 +141,5 @@ DimensionRawColumnChunk getDimensionChunk(FileHolder fileReader, int blockIndexe
        * @param pageNumber
        * @return
        */
    -  int getPageRowCount(int pageNumber);
    +  int getPageRowCount(int pageNumber) throws IOException;
    --- End diff --
   
    Avoid throwing IOException from this interface methods. Instead catch the IO exception in the caller methods and wrap it in RuntimeException and throw


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2187#discussion_r184280492
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/UnsafeMemoryDMStore.java ---
    @@ -101,6 +102,7 @@ public void addIndexRow(DataMapRow indexRow) throws MemoryException {
       }
     
       private void addToUnsafe(CarbonRowSchema schema, DataMapRow row, int index) {
    +    schema.setDataType(DataTypeUtil.valueOf(schema.getDataType(), 0, 0));
    --- End diff --
   
    Remove this logic from here and add a method in AbstractMemoryDMStore called setSchemaDataType and set the datatypes at once in that method


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2187#discussion_r184281016
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockletDataMapFactory.java ---
    @@ -121,7 +123,7 @@ public DataMapWriter createWriter(Segment segment) {
             segmentMap.get(segment.getSegmentNo());
         if (tableBlockIndexUniqueIdentifiers == null) {
           tableBlockIndexUniqueIdentifiers =
    -          BlockletDataMapUtil.getTableBlockUniqueIdentifiers(segment, identifier.getTablePath());
    +           BlockletDataMapUtil.getTableBlockUniqueIdentifiers(segment, identifier.getTablePath());
    --- End diff --
   
    check for proper identation for this line


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2187#discussion_r184281751
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/TableInfo.java ---
    @@ -97,6 +102,14 @@ public TableSchema getFactTable() {
       public void setFactTable(TableSchema factTable) {
         this.factTable = factTable;
         updateParentRelationIdentifier();
    +    updateIsSchemaModified();
    +  }
    +
    +  private void updateIsSchemaModified() {
    +    if (null != factTable.getSchemaEvalution()) {
    +      isSchemaModified =
    --- End diff --
   
    Add a comment here as below
    If schema evolution entry list size  is > 1 that means n alter operation is performed which has added the new schema entry in the schema evolution list. Currently apart from create table schema evolution entries are getting added only in the alter operations.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5413/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4550/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4552/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2187
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4246/



---

[qiuchenjian-2]

Github user dhatchayani commented on the issue:

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


---

[qiuchenjian-2]

Github user manishgupta88 commented on the issue:

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


---