You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by xubo245 <gi...@git.apache.org> on 2018/11/14 09:24:41 UTC

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

GitHub user xubo245 opened a pull request:

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

    [CARBONDATA-3097] Support folder path in getVersionDetails and support getVersionDetails in CSDK

    This PR support folder path in getVersionDetails and support getVersionDetails in CSDK
    
    Be sure to do all of the following checklist to help us incorporate 
    your contribution quickly and easily:
    
     - [ ] Any interfaces changed?
     add
     - [ ] Any backward compatibility impacted?
     no
     - [ ] Document update required?
    Yes
     - [ ] Testing done
         add test case in SDK and CSDK
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA. 
    JIRA2951


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/xubo245/carbondata CARBONDATA-3097_getVersionDetails

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

    https://github.com/apache/carbondata/pull/2919.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 #2919
    
----
commit 22a6bd39e63040761b70fc944a056d9492e5e95e
Author: xubo245 <xu...@...>
Date:   2018-11-14T09:03:17Z

    [CARBONDATA-3097] Support folder path in getVersionDetails and support getVersionDetails in CSDK

----


---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242583087
  
    --- Diff: store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonSchemaReaderTest.java ---
    @@ -236,6 +243,113 @@ public void testReadSchemaWithDifferentSchema() {
         }
       }
     
    +  @Test
    +  public void testGetVersionDetailsAndValidate() {
    +    try {
    +      String versionDetails = CarbonSchemaReader
    +          .getVersionDetails(path);
    +      Assert.assertTrue(versionDetails.contains("CarbonSchemaReaderTest in version: "));
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithoutValidate() {
    +    try {
    +      String versionDetails = CarbonSchemaReader
    +          .getVersionDetails(path);
    +      Assert.assertTrue(versionDetails.contains("CarbonSchemaReaderTest in version: "));
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithCarbonDataFile() {
    +    try {
    +      File[] dataFiles = new File(path).listFiles(new FilenameFilter() {
    +        @Override
    +        public boolean accept(File dir, String name) {
    +          return name.endsWith(CARBON_DATA_EXT);
    +        }
    +      });
    +      String versionDetails = CarbonSchemaReader.getVersionDetails(dataFiles[0].getAbsolutePath());
    +      Assert.assertTrue(versionDetails.contains("CarbonSchemaReaderTest in version: "));
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithCarbonIndexFile() {
    +    try {
    +      File[] indexFiles = new File(path).listFiles(new FilenameFilter() {
    +        @Override
    +        public boolean accept(File dir, String name) {
    +          return name.endsWith(INDEX_FILE_EXT);
    +        }
    +      });
    +      CarbonSchemaReader.getVersionDetails(indexFiles[0].getAbsolutePath());
    +      Assert.fail();
    +    } catch (Throwable e) {
    +      Assert.assertTrue(e.getMessage()
    +          .equalsIgnoreCase("Can't get version details from carbonindex file."));
    +    }
    +  }
    +
    +  public void testGetVersionDetailsWithTheSameSchema() {
    +    try {
    +      writeData();
    +      try {
    +        String versionDetails = CarbonSchemaReader
    +            .getVersionDetails(path);
    +        Assert.assertTrue(versionDetails
    +            .contains("CarbonSchemaReaderTest in version: "));
    +      } catch (Exception e) {
    +        Assert.fail();
    +      }
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithDifferentSchema() {
    +    try {
    +      int num = 10;
    +      Field[] fields = new Field[2];
    +      fields[0] = new Field("name", DataTypes.STRING);
    +      fields[1] = new Field("age", DataTypes.INT);
    +      CarbonWriter writer = CarbonWriter
    +          .builder()
    +          .outputPath(path)
    +          .withCsvInput(new Schema(fields))
    +          .writtenBy("testReadSchemaWithDifferentSchema")
    +          .build();
    +
    +      for (int i = 0; i < num; i++) {
    +        writer.write(new String[]{"robot" + (i % 10), String.valueOf(i)});
    +      }
    +      writer.close();
    +      try {
    +        CarbonSchemaReader
    +            .getVersionDetails(path);
    +      } catch (Exception e) {
    +        Assert.assertTrue(e.getMessage()
    +            .equalsIgnoreCase("Version details is different between different files."));
    +        Assert.fail();
    --- End diff --
    
    ok, removed.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10090/



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by KanakaKumar <gi...@git.apache.org>.
Github user KanakaKumar commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242588569
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -241,12 +241,25 @@ private static Schema readSchemaFromIndexFile(String indexFilePath) throws IOExc
     
       /**
        * This method return the version details in formatted string by reading from carbondata file
    +   * default won't validate the version details between different carbondata files.
        *
    -   * @param dataFilePath
    -   * @return
    +   * @param path carbondata file path or folder path
    +   * @return string with information of who has written this file
    +   * in which carbondata project version
        * @throws IOException
        */
    -  public static String getVersionDetails(String dataFilePath) throws IOException {
    +  public static String getVersionDetails(String path) throws IOException {
    +    if (path.endsWith(INDEX_FILE_EXT)) {
    +      throw new RuntimeException("Can't get version details from carbonindex file.");
    --- End diff --
    
    RuntimeException is mentioned in the signature. Should throw IOException.
    Also some places in this class throws CarbonDataLoadingException but if not intentional we can change them to IOException as per signature instead of a subclass of RuntimeException (if it was not intentional)


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    @KanakaKumar @jackylk @ajantha-bhat @kunal642 Please review it.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10227/



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by KanakaKumar <gi...@git.apache.org>.
Github user KanakaKumar commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242577599
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -241,12 +241,25 @@ private static Schema readSchemaFromIndexFile(String indexFilePath) throws IOExc
     
       /**
        * This method return the version details in formatted string by reading from carbondata file
    +   * default won't validate the version details between different carbondata files.
        *
    -   * @param dataFilePath
    -   * @return
    +   * @param path carbondata file path or folder path
    +   * @return string with information of who has written this file
    +   * in which carbondata project version
        * @throws IOException
        */
    -  public static String getVersionDetails(String dataFilePath) throws IOException {
    +  public static String getVersionDetails(String path) throws IOException {
    +    if (path.endsWith(INDEX_FILE_EXT)) {
    +      throw new RuntimeException("Can't get version details from carbonindex file.");
    +    } else if (path.endsWith(CARBON_DATA_EXT)) {
    +      return getVersionDetailsFromDataFile(path);
    +    } else {
    +      String indexFilePath = getCarbonFile(path, CARBON_DATA_EXT)[0].getAbsolutePath();
    --- End diff --
    
    this is data file


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/1839/



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    @KanakaKumar Please review it again.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242583526
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -241,12 +241,25 @@ private static Schema readSchemaFromIndexFile(String indexFilePath) throws IOExc
     
       /**
        * This method return the version details in formatted string by reading from carbondata file
    +   * default won't validate the version details between different carbondata files.
        *
    -   * @param dataFilePath
    -   * @return
    +   * @param path carbondata file path or folder path
    +   * @return string with information of who has written this file
    +   * in which carbondata project version
        * @throws IOException
        */
    -  public static String getVersionDetails(String dataFilePath) throws IOException {
    +  public static String getVersionDetails(String path) throws IOException {
    +    if (path.endsWith(INDEX_FILE_EXT)) {
    +      throw new RuntimeException("Can't get version details from carbonindex file.");
    +    } else if (path.endsWith(CARBON_DATA_EXT)) {
    +      return getVersionDetailsFromDataFile(path);
    +    } else {
    +      String indexFilePath = getCarbonFile(path, CARBON_DATA_EXT)[0].getAbsolutePath();
    --- End diff --
    
    ok, change the name to dataFilePath


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

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


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10097/



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Rebased and CI pass,@KanakaKumar @jackylk @QiangCai @ajantha-bhat @kunal642 Please review it.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by KanakaKumar <gi...@git.apache.org>.
Github user KanakaKumar commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242035462
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -241,12 +241,52 @@ private static Schema readSchemaFromIndexFile(String indexFilePath) throws IOExc
     
       /**
        * This method return the version details in formatted string by reading from carbondata file
    +   * If validate is true, it will check the version details between different carbondata files.
    +   * And if version details are not the same, it will throw exception
        *
    -   * @param dataFilePath
    -   * @return
    +   * @param path     carbondata file path or folder path
    +   * @param validate whether validate the version details between different carbondata files.
    +   * @return string with information of who has written this file
    +   * in which carbondata project version
        * @throws IOException
        */
    -  public static String getVersionDetails(String dataFilePath) throws IOException {
    +  public static String getVersionDetails(String path, boolean validate) throws IOException {
    --- End diff --
    
    I think it is not correct to validate readability through version details. In general new version jars can read all old version files. 
    
    Please remove this method.
    



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by KanakaKumar <gi...@git.apache.org>.
Github user KanakaKumar commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242577513
  
    --- Diff: store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonSchemaReaderTest.java ---
    @@ -236,6 +243,113 @@ public void testReadSchemaWithDifferentSchema() {
         }
       }
     
    +  @Test
    +  public void testGetVersionDetailsAndValidate() {
    +    try {
    +      String versionDetails = CarbonSchemaReader
    +          .getVersionDetails(path);
    +      Assert.assertTrue(versionDetails.contains("CarbonSchemaReaderTest in version: "));
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithoutValidate() {
    +    try {
    +      String versionDetails = CarbonSchemaReader
    +          .getVersionDetails(path);
    +      Assert.assertTrue(versionDetails.contains("CarbonSchemaReaderTest in version: "));
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithCarbonDataFile() {
    +    try {
    +      File[] dataFiles = new File(path).listFiles(new FilenameFilter() {
    +        @Override
    +        public boolean accept(File dir, String name) {
    +          return name.endsWith(CARBON_DATA_EXT);
    +        }
    +      });
    +      String versionDetails = CarbonSchemaReader.getVersionDetails(dataFiles[0].getAbsolutePath());
    +      Assert.assertTrue(versionDetails.contains("CarbonSchemaReaderTest in version: "));
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithCarbonIndexFile() {
    +    try {
    +      File[] indexFiles = new File(path).listFiles(new FilenameFilter() {
    +        @Override
    +        public boolean accept(File dir, String name) {
    +          return name.endsWith(INDEX_FILE_EXT);
    +        }
    +      });
    +      CarbonSchemaReader.getVersionDetails(indexFiles[0].getAbsolutePath());
    +      Assert.fail();
    +    } catch (Throwable e) {
    +      Assert.assertTrue(e.getMessage()
    +          .equalsIgnoreCase("Can't get version details from carbonindex file."));
    +    }
    +  }
    +
    +  public void testGetVersionDetailsWithTheSameSchema() {
    +    try {
    +      writeData();
    +      try {
    +        String versionDetails = CarbonSchemaReader
    +            .getVersionDetails(path);
    +        Assert.assertTrue(versionDetails
    +            .contains("CarbonSchemaReaderTest in version: "));
    +      } catch (Exception e) {
    +        Assert.fail();
    +      }
    +    } catch (Throwable e) {
    +      e.printStackTrace();
    +      Assert.fail();
    +    }
    +  }
    +
    +  @Test
    +  public void testGetVersionDetailsWithDifferentSchema() {
    +    try {
    +      int num = 10;
    +      Field[] fields = new Field[2];
    +      fields[0] = new Field("name", DataTypes.STRING);
    +      fields[1] = new Field("age", DataTypes.INT);
    +      CarbonWriter writer = CarbonWriter
    +          .builder()
    +          .outputPath(path)
    +          .withCsvInput(new Schema(fields))
    +          .writtenBy("testReadSchemaWithDifferentSchema")
    +          .build();
    +
    +      for (int i = 0; i < num; i++) {
    +        writer.write(new String[]{"robot" + (i % 10), String.valueOf(i)});
    +      }
    +      writer.close();
    +      try {
    +        CarbonSchemaReader
    +            .getVersionDetails(path);
    +      } catch (Exception e) {
    +        Assert.assertTrue(e.getMessage()
    +            .equalsIgnoreCase("Version details is different between different files."));
    +        Assert.fail();
    --- End diff --
    
     What is the purpose of this test case?  "Version details is different between different files" exception we never get now right? If thee is no separate validation required, we can remove this test.



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10078/



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242429609
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -241,12 +241,52 @@ private static Schema readSchemaFromIndexFile(String indexFilePath) throws IOExc
     
       /**
        * This method return the version details in formatted string by reading from carbondata file
    +   * If validate is true, it will check the version details between different carbondata files.
    +   * And if version details are not the same, it will throw exception
        *
    -   * @param dataFilePath
    -   * @return
    +   * @param path     carbondata file path or folder path
    +   * @param validate whether validate the version details between different carbondata files.
    +   * @return string with information of who has written this file
    +   * in which carbondata project version
        * @throws IOException
        */
    -  public static String getVersionDetails(String dataFilePath) throws IOException {
    +  public static String getVersionDetails(String path, boolean validate) throws IOException {
    --- End diff --
    
    ok, done


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10081/



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    @KanakaKumar @jackylk @QiangCai @ajantha-bhat Please review it.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    @KanakaKumar Please check it again.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Failed  with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10096/



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    @KanakaKumar @jackylk @QiangCai @ajantha-bhat @kunal642 Rebased, Please review it.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    @KanakaKumar Please check it again.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r244102291
  
    --- Diff: store/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonSchemaReader.java ---
    @@ -241,12 +241,25 @@ private static Schema readSchemaFromIndexFile(String indexFilePath) throws IOExc
     
       /**
        * This method return the version details in formatted string by reading from carbondata file
    +   * default won't validate the version details between different carbondata files.
        *
    -   * @param dataFilePath
    -   * @return
    +   * @param path carbondata file path or folder path
    +   * @return string with information of who has written this file
    +   * in which carbondata project version
        * @throws IOException
        */
    -  public static String getVersionDetails(String dataFilePath) throws IOException {
    +  public static String getVersionDetails(String path) throws IOException {
    +    if (path.endsWith(INDEX_FILE_EXT)) {
    +      throw new RuntimeException("Can't get version details from carbonindex file.");
    --- End diff --
    
    IOException is from org.apache.carbondata.sdk.file.CarbonSchemaReader#getVersionDetailsFromDataFile,  which is from org.apache.carbondata.core.datastore.FileReader#readByteBuffer, It's better to keep API, don't change it. @KanakaKumar 


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by KanakaKumar <gi...@git.apache.org>.
Github user KanakaKumar commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242578856
  
    --- Diff: docs/sdk-guide.md ---
    @@ -816,13 +816,15 @@ Find example code at [CarbonReaderExample](https://github.com/apache/carbondata/
        * This method return the version details in formatted string by reading from carbondata file
        * If application name is SDK_1.0.0 and this has written the carbondata file in carbondata 1.6 project version,
        * then this API returns the String "SDK_1.0.0 in version: 1.6.0-SNAPSHOT"
    -   * @param dataFilePath complete path including carbondata file name
    +   * Default value of validate is false, it won't validate the version details between different carbondata files.
    --- End diff --
    
    " Default value of validate is false,"
    - This statement is not valid as we don't have validate and no validate flows differently.


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

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



---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Support folder path in getVersionD...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on the issue:

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


---

[GitHub] carbondata pull request #2919: [CARBONDATA-3097] Support folder path in getV...

Posted by xubo245 <gi...@git.apache.org>.
Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2919#discussion_r242583802
  
    --- Diff: docs/sdk-guide.md ---
    @@ -816,13 +816,15 @@ Find example code at [CarbonReaderExample](https://github.com/apache/carbondata/
        * This method return the version details in formatted string by reading from carbondata file
        * If application name is SDK_1.0.0 and this has written the carbondata file in carbondata 1.6 project version,
        * then this API returns the String "SDK_1.0.0 in version: 1.6.0-SNAPSHOT"
    -   * @param dataFilePath complete path including carbondata file name
    +   * Default value of validate is false, it won't validate the version details between different carbondata files.
    --- End diff --
    
    ok, removed


---

[GitHub] carbondata issue #2919: [CARBONDATA-3097] Optimize getVersionDetails

Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2919
  
    Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10437/



---