[GitHub] carbondata pull request #2804: [CARBONDATA-2996] CarbonSchemaReader support ...

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

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


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2804
 
    @KanakaKumar @kunal642 @ajantha-bhat CI pass,  Please check.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230788337
 
    --- Diff: docs/sdk-guide.md ---
    @@ -685,6 +685,31 @@ Find example code at [CarbonReaderExample](https://github.com/apache/carbondata/
        */
       public static Schema readSchemaInIndexFile(String indexFilePath);
     ```
    +```
    +  /**
    +   * read schema from path,
    +   * path can be folder path,carbonindex file path, and carbondata file path
    +   * and will not check all files schema
    +   *
    +   * @param path file/folder path
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path);
    +```
    +```
    +  /**
    +   * read schema from path,
    +   * path can be folder path,carbonindex file path, and carbondata file path
    +   * and user can decide whether check all files schema
    +   *
    +   * @param path             file/folder path
    +   * @param checkFilesSchema whether check all files schema
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path, boolean checkFilesSchema);
    --- End diff --
   
    checkFilesSchema should be validateSchema


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230799912
 
    --- Diff: store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonSchemaReaderTest.java ---
    @@ -101,18 +104,30 @@ public boolean accept(CarbonFile file) {
           String dataFilePath = carbonFiles[0].getAbsolutePath();
     
           Schema schema = CarbonSchemaReader
    -          .readSchemaInDataFile(dataFilePath)
    +          .readSchema(dataFilePath)
               .asOriginOrder();
     
           assertEquals(schema.getFieldsLength(), 12);
           checkSchema(schema);
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    --- End diff --
   
    should fail


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230801853
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -61,14 +65,121 @@ public static Schema readSchemaInSchemaFile(String schemaFilePath) throws IOExce
         return new Schema(schemaList);
       }
     
    +  /**
    +   * get carbondata/carbonindex file in path
    +   *
    +   * @param path carbon file path
    +   * @return CarbonFile array
    +   */
    +  private static CarbonFile[] getCarbonFile(String path, final String extension) {
    +    String dataFilePath = path;
    +    if (!(dataFilePath.contains(extension))) {
    +      CarbonFile[] carbonFiles = FileFactory
    +          .getCarbonFile(path)
    +          .listFiles(new CarbonFileFilter() {
    +            @Override
    +            public boolean accept(CarbonFile file) {
    +              if (file == null) {
    +                return false;
    +              }
    +              return file.getName().endsWith(extension);
    +            }
    +          });
    +      if (carbonFiles == null || carbonFiles.length < 1) {
    +        throw new RuntimeException("Carbon file not exists.");
    +      }
    +      return carbonFiles;
    +    }
    +    return null;
    +  }
    +
    +  /**
    +   * read schema from path,
    +   * path can be folder path, carbonindex file path, and carbondata file path
    +   * and will not check all files schema
    +   *
    +   * @param path file/folder path
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path) throws IOException {
    +    return readSchema(path, false);
    +  }
    +
    +  /**
    +   * read schema from path,
    +   * path can be folder path, carbonindex file path, and carbondata file path
    +   * and user can decide whether check all files schema
    +   *
    +   * @param path             file/folder path
    +   * @param checkFilesSchema whether check all files schema
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path, boolean checkFilesSchema) throws IOException {
    --- End diff --
   
    readSchema(String path, boolean checkFilesSchema)
    -- Is this schema validation method is required ? If no use case we can skip this..  during query execution anyways schema is validated.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230802487
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -61,14 +65,121 @@ public static Schema readSchemaInSchemaFile(String schemaFilePath) throws IOExce
         return new Schema(schemaList);
       }
     
    +  /**
    +   * get carbondata/carbonindex file in path
    +   *
    +   * @param path carbon file path
    +   * @return CarbonFile array
    +   */
    +  private static CarbonFile[] getCarbonFile(String path, final String extension) {
    +    String dataFilePath = path;
    +    if (!(dataFilePath.contains(extension))) {
    +      CarbonFile[] carbonFiles = FileFactory
    +          .getCarbonFile(path)
    +          .listFiles(new CarbonFileFilter() {
    +            @Override
    +            public boolean accept(CarbonFile file) {
    +              if (file == null) {
    +                return false;
    +              }
    +              return file.getName().endsWith(extension);
    +            }
    +          });
    +      if (carbonFiles == null || carbonFiles.length < 1) {
    +        throw new RuntimeException("Carbon file not exists.");
    --- End diff --
   
    Why RunTimeException, IO related failures should throw IOException


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230982398
 
    --- Diff: docs/sdk-guide.md ---
    @@ -685,6 +685,31 @@ Find example code at [CarbonReaderExample](https://github.com/apache/carbondata/
        */
       public static Schema readSchemaInIndexFile(String indexFilePath);
     ```
    +```
    +  /**
    +   * read schema from path,
    +   * path can be folder path,carbonindex file path, and carbondata file path
    +   * and will not check all files schema
    +   *
    +   * @param path file/folder path
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path);
    +```
    +```
    +  /**
    +   * read schema from path,
    +   * path can be folder path,carbonindex file path, and carbondata file path
    +   * and user can decide whether check all files schema
    +   *
    +   * @param path             file/folder path
    +   * @param checkFilesSchema whether check all files schema
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path, boolean checkFilesSchema);
    --- End diff --
   
    ok, done


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230982638
 
    --- Diff: store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonSchemaReaderTest.java ---
    @@ -101,18 +104,30 @@ public boolean accept(CarbonFile file) {
           String dataFilePath = carbonFiles[0].getAbsolutePath();
     
           Schema schema = CarbonSchemaReader
    -          .readSchemaInDataFile(dataFilePath)
    +          .readSchema(dataFilePath)
               .asOriginOrder();
     
           assertEquals(schema.getFieldsLength(), 12);
           checkSchema(schema);
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    --- End diff --
   
    ok, done,added Assert.fail();


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230982799
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -61,14 +65,121 @@ public static Schema readSchemaInSchemaFile(String schemaFilePath) throws IOExce
         return new Schema(schemaList);
       }
     
    +  /**
    +   * get carbondata/carbonindex file in path
    +   *
    +   * @param path carbon file path
    +   * @return CarbonFile array
    +   */
    +  private static CarbonFile[] getCarbonFile(String path, final String extension) {
    +    String dataFilePath = path;
    +    if (!(dataFilePath.contains(extension))) {
    +      CarbonFile[] carbonFiles = FileFactory
    +          .getCarbonFile(path)
    +          .listFiles(new CarbonFileFilter() {
    +            @Override
    +            public boolean accept(CarbonFile file) {
    +              if (file == null) {
    +                return false;
    +              }
    +              return file.getName().endsWith(extension);
    +            }
    +          });
    +      if (carbonFiles == null || carbonFiles.length < 1) {
    +        throw new RuntimeException("Carbon file not exists.");
    +      }
    +      return carbonFiles;
    +    }
    +    return null;
    +  }
    +
    +  /**
    +   * read schema from path,
    +   * path can be folder path, carbonindex file path, and carbondata file path
    +   * and will not check all files schema
    +   *
    +   * @param path file/folder path
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path) throws IOException {
    +    return readSchema(path, false);
    +  }
    +
    +  /**
    +   * read schema from path,
    +   * path can be folder path, carbonindex file path, and carbondata file path
    +   * and user can decide whether check all files schema
    +   *
    +   * @param path             file/folder path
    +   * @param checkFilesSchema whether check all files schema
    +   * @return schema
    +   * @throws IOException
    +   */
    +  public static Schema readSchema(String path, boolean checkFilesSchema) throws IOException {
    --- End diff --
   
    when user only want to check schema and no need to query data, they can use readSchema. and readSchema also will faster.


---

[qiuchenjian-2]

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

    https://github.com/apache/carbondata/pull/2804#discussion_r230983007
 
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -61,14 +65,121 @@ public static Schema readSchemaInSchemaFile(String schemaFilePath) throws IOExce
         return new Schema(schemaList);
       }
     
    +  /**
    +   * get carbondata/carbonindex file in path
    +   *
    +   * @param path carbon file path
    +   * @return CarbonFile array
    +   */
    +  private static CarbonFile[] getCarbonFile(String path, final String extension) {
    +    String dataFilePath = path;
    +    if (!(dataFilePath.contains(extension))) {
    +      CarbonFile[] carbonFiles = FileFactory
    +          .getCarbonFile(path)
    +          .listFiles(new CarbonFileFilter() {
    +            @Override
    +            public boolean accept(CarbonFile file) {
    +              if (file == null) {
    +                return false;
    +              }
    +              return file.getName().endsWith(extension);
    +            }
    +          });
    +      if (carbonFiles == null || carbonFiles.length < 1) {
    +        throw new RuntimeException("Carbon file not exists.");
    --- End diff --
   
    ok, done


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

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



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2804
 
    @KanakaKumar @kunal642 CI pass, please check it.


---