[GitHub] carbondata pull request #1991: [CARBONDATA-2196]carbontable should be taken ...

[qiuchenjian-2]

Github user ravipesala commented on the issue:

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



---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1991#discussion_r171487297
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -117,22 +117,25 @@ public static void createLocations(String[] locations) {
           }
         }
       }
    +
       /**
    +   *
        * This method will form the local data folder store location
        *
    -   * @param databaseName
    -   * @param tableName
    +   * @param carbonTable
        * @param taskId
        * @param partitionId
        * @param segmentId
    +   * @param isCompactionFlow
    +   * @param isAltPartitionFlow
        * @return
        */
    -  public static String[] getLocalDataFolderLocation(String databaseName, String tableName,
    +  public static String[] getLocalDataFolderLocation(CarbonTable carbonTable, String tableName,
    --- End diff --
   
    not require tableName


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1991#discussion_r171489212
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -117,22 +117,25 @@ public static void createLocations(String[] locations) {
           }
         }
       }
    +
       /**
    +   *
        * This method will form the local data folder store location
        *
    -   * @param databaseName
    -   * @param tableName
    +   * @param carbonTable
        * @param taskId
        * @param partitionId
        * @param segmentId
    +   * @param isCompactionFlow
    +   * @param isAltPartitionFlow
        * @return
        */
    -  public static String[] getLocalDataFolderLocation(String databaseName, String tableName,
    +  public static String[] getLocalDataFolderLocation(CarbonTable carbonTable, String tableName,
    --- End diff --
   
     in case of Pre aggregate table, table name can be of child even though carbonTable object is for main table.


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user QiangCai commented on the issue:

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


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1991#discussion_r171748491
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -158,6 +160,27 @@ public static void createLocations(String[] locations) {
         return localDataFolderLocArray;
       }
     
    +  /**
    +   * This method will form the local data folder store location
    +   *
    +   * @param databaseName
    +   * @param tableName
    +   * @param taskId
    +   * @param partitionId
    +   * @param segmentId
    +   * @param isCompactionFlow
    +   * @param isAltPartitionFlow
    +   * @return
    +   */
    +  public static String[] getLocalDataFolderLocation(String databaseName, String tableName,
    +      String taskId, String partitionId, String segmentId, boolean isCompactionFlow,
    +      boolean isAltPartitionFlow) {
    +    CarbonTable carbonTable = CarbonMetadata.getInstance().getCarbonTable(databaseName, tableName);
    +    return getLocalDataFolderLocation(carbonTable, tableName, taskId, partitionId,
    +        segmentId, isCompactionFlow, isAltPartitionFlow);
    +  }
    +
    --- End diff --
   
    not required


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/1991#discussion_r171750725
 
    --- Diff: processing/src/main/java/org/apache/carbondata/processing/util/CarbonDataProcessorUtil.java ---
    @@ -158,6 +160,27 @@ public static void createLocations(String[] locations) {
         return localDataFolderLocArray;
       }
     
    +  /**
    +   * This method will form the local data folder store location
    +   *
    +   * @param databaseName
    +   * @param tableName
    +   * @param taskId
    +   * @param partitionId
    +   * @param segmentId
    +   * @param isCompactionFlow
    +   * @param isAltPartitionFlow
    +   * @return
    +   */
    +  public static String[] getLocalDataFolderLocation(String databaseName, String tableName,
    +      String taskId, String partitionId, String segmentId, boolean isCompactionFlow,
    +      boolean isAltPartitionFlow) {
    +    CarbonTable carbonTable = CarbonMetadata.getInstance().getCarbonTable(databaseName, tableName);
    +    return getLocalDataFolderLocation(carbonTable, tableName, taskId, partitionId,
    +        segmentId, isCompactionFlow, isAltPartitionFlow);
    +  }
    +
    --- End diff --
   
    fixed


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user QiangCai commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user QiangCai commented on the issue:

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


---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

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


---