[GitHub] [carbondata] nihal0107 opened a new pull request #4068: [CARBONDATA-3987] Handled filter and IUD operation for pagination reader in SDK

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754069375


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3520/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754428637


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3521/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

nihal0107 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754429243


   retest this please.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754429208


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5281/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754471198


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5282/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754477527


   Build Failed  with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3522/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

nihal0107 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754511784


   retest this please.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754559176


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5283/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754561987


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3523/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ajantha-bhat commented on a change in pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#discussion_r551950110



##########
File path: sdk/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonIUD.java
##########
@@ -121,6 +125,25 @@ private void closeDelete() throws IOException, InterruptedException {
     for (Map.Entry<String, Map<String, Set<String>>> path : this.filterColumnToValueMappingForDelete
         .entrySet()) {
       deleteExecution(path.getKey());
+      createEmptyMetadataFile(path.getKey());
+    }
+  }
+
+  private void createEmptyMetadataFile(String path) throws IOException {
+    if (!StringUtils.isEmpty(path)) {
+      // Get store path of the table.
+      String schemaFilePath = CarbonTablePath.getSchemaFilePath(path, configuration);
+      // If metadata folder exists, it is a transactional table
+      CarbonFile schemaFile = FileFactory.getCarbonFile(schemaFilePath, configuration);
+      boolean isTransactionalTable = schemaFile.exists();
+      if (!isTransactionalTable) {

Review comment:
       as it is in sdk module, it is always non-transactional right? I think we don't need this check here. Directly create an empty folder in the file path.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ajantha-bhat commented on a change in pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#discussion_r551950859



##########
File path: sdk/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonReaderBuilder.java
##########
@@ -70,6 +75,10 @@
   private boolean usePaginationReader;
   private List fileLists;
   private Class<? extends CarbonReadSupport> readSupportClass;
+  public static final String UPDATE_DELTA_VERSION = "updateDeltaVersion";

Review comment:
       unused ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ajantha-bhat commented on a change in pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#discussion_r551951489



##########
File path: sdk/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonReaderBuilder.java
##########
@@ -427,6 +438,67 @@ private CarbonFileInputFormat prepareFileInputFormat(Job job, boolean enableBloc
     }
   }
 
+  private <T> void totalRowCountInSplits(Job job, List<InputSplit> splits,
+      List<Long> rowCountInSplit)
+      throws IOException, InterruptedException {
+    CarbonFileInputFormat format = this.prepareFileInputFormat(job, false, true);
+    long sum = 0;
+    boolean isIUDTable = false;
+    if (!StringUtils.isEmpty(this.tablePath)) {
+      // Get store path of the table.
+      String schemaFilePath = CarbonTablePath.getSchemaFilePath(this.tablePath, this.hadoopConf);
+      // If metadata folder exists, it is a transactional table
+      CarbonFile schemaFile = FileFactory.getCarbonFile(schemaFilePath, this.hadoopConf);
+      boolean isTransactionalTable = schemaFile.exists();
+      // Check if update or delete happened on the table.
+      if (!isTransactionalTable) {
+        CarbonFile emptyMetadataFile = FileFactory.getCarbonFile(this.tablePath +

Review comment:
       same comment as above, I think this method is called for non-transactional table only




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

nihal0107 commented on a change in pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#discussion_r551974502



##########
File path: sdk/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonIUD.java
##########
@@ -121,6 +125,25 @@ private void closeDelete() throws IOException, InterruptedException {
     for (Map.Entry<String, Map<String, Set<String>>> path : this.filterColumnToValueMappingForDelete
         .entrySet()) {
       deleteExecution(path.getKey());
+      createEmptyMetadataFile(path.getKey());
+    }
+  }
+
+  private void createEmptyMetadataFile(String path) throws IOException {
+    if (!StringUtils.isEmpty(path)) {
+      // Get store path of the table.
+      String schemaFilePath = CarbonTablePath.getSchemaFilePath(path, configuration);
+      // If metadata folder exists, it is a transactional table
+      CarbonFile schemaFile = FileFactory.getCarbonFile(schemaFilePath, configuration);
+      boolean isTransactionalTable = schemaFile.exists();
+      if (!isTransactionalTable) {

Review comment:
       yeah, you are right, earlier I was thinking about the operation performed on the transactional and not transactional table both.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

nihal0107 commented on a change in pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#discussion_r551974878



##########
File path: sdk/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonReaderBuilder.java
##########
@@ -70,6 +75,10 @@
   private boolean usePaginationReader;
   private List fileLists;
   private Class<? extends CarbonReadSupport> readSupportClass;
+  public static final String UPDATE_DELTA_VERSION = "updateDeltaVersion";

Review comment:
       it was getting used to check the transactional table, removed now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

nihal0107 commented on a change in pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#discussion_r551974964



##########
File path: sdk/sdk/src/main/java/org/apache/carbondata/sdk/file/CarbonReaderBuilder.java
##########
@@ -427,6 +438,67 @@ private CarbonFileInputFormat prepareFileInputFormat(Job job, boolean enableBloc
     }
   }
 
+  private <T> void totalRowCountInSplits(Job job, List<InputSplit> splits,
+      List<Long> rowCountInSplit)
+      throws IOException, InterruptedException {
+    CarbonFileInputFormat format = this.prepareFileInputFormat(job, false, true);
+    long sum = 0;
+    boolean isIUDTable = false;
+    if (!StringUtils.isEmpty(this.tablePath)) {
+      // Get store path of the table.
+      String schemaFilePath = CarbonTablePath.getSchemaFilePath(this.tablePath, this.hadoopConf);
+      // If metadata folder exists, it is a transactional table
+      CarbonFile schemaFile = FileFactory.getCarbonFile(schemaFilePath, this.hadoopConf);
+      boolean isTransactionalTable = schemaFile.exists();
+      // Check if update or delete happened on the table.
+      if (!isTransactionalTable) {
+        CarbonFile emptyMetadataFile = FileFactory.getCarbonFile(this.tablePath +

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754731953


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5284/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

CarbonDataQA2 commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754735289


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3524/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

ajantha-bhat commented on pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068#issuecomment-754780154


   LGTM


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]

[GitBox]

asfgit closed pull request #4068:
URL: https://github.com/apache/carbondata/pull/4068


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]