[GitHub] [carbondata] ShreelekhyaG opened a new pull request #3922: [WIP] SI compatability issue

[GitBox]

ShreelekhyaG opened a new pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922


    ### Why is this PR needed?
   Read from maintable having SI returns empty resultset when SI has with old tuple id storage format.
   
    ### What changes were proposed in this PR?
   Checked if blockletPath contains old tuple id format, and convert it to compatible format.
       
    ### Does this PR introduce any user interface change?
    - No
    - Yes. (please explain the change and update document)
   
    ### Is any new testcase added?
    - No
    - Yes
   
       
   


----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691048404


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


----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691049096


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


----------------------------------------------------------------
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]

marchpure commented on a change in pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#discussion_r487350326



##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR




----------------------------------------------------------------
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]

marchpure commented on a change in pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#discussion_r487350326



##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR




----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691048404






----------------------------------------------------------------
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]

marchpure commented on a change in pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#discussion_r487350326



##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);

Review comment:
       use File.seperator instead of '/'

##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));

Review comment:
       use File.seperator instead of CarbonCommonConstants.FILE_SEPARATOR




----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691048404






----------------------------------------------------------------
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]

ShreelekhyaG commented on a change in pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#discussion_r487668590



##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -60,12 +61,16 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
   private void addBlockEntry(String blockletPath) {
     String blockId =
         blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {
+      blockId = CarbonTablePath.getShortBlockId(blockId).substring(blockletPath.indexOf('/') + 1);
+    }
     Set<Integer> blockletIds = blockIdToBlockletIdMapping.get(blockId);
     if (null == blockletIds) {
       blockletIds = new HashSet<>();
       blockIdToBlockletIdMapping.put(blockId, blockletIds);
     }
-    blockletIds.add(Integer.parseInt(blockletPath.substring(blockId.length() + 1)));
+    blockletIds.add(Integer.parseInt(blockletPath.substring(blockletPath.lastIndexOf('/') + 1)));

Review comment:
       ok. Changed it to File.seperator instead of '/' in all 3 places.




----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691886899


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


----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691888439


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


----------------------------------------------------------------
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]

akashrn5 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691931424


   @ShreelekhyaG can you please check for the compatibility of update and delete case also and try to handle in this PR? As this is is specific to tupleID, and recent PR changed the tuple ID, i think there may be issues with old stores where we have already performed update and delete operation on that store, and try to read in latest code and try to read by updating and deleting in latest store?


----------------------------------------------------------------
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]

akashrn5 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-691933797


   @ShreelekhyaG also please check the partition table in SI, update and delete cases.


----------------------------------------------------------------
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]

ShreelekhyaG commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-692052269


   @akashrn5 I have tested for update and delete case for a table with and without partition. It is working fine.


----------------------------------------------------------------
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]

akashrn5 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-692059466


   > @akashrn5 I have tested for update and delete case for a table with and without partition. It is working fine.
   
   Yes, since we generate tupleID on the fly and dont store , it works, for SI we store position reference it failed in compatibility. Just check the partition with SI case and confirm


----------------------------------------------------------------
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]

ShreelekhyaG commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-692454326


   @akashrn5 for partition with SI case, partition prefix should not be removed. made small change to my existing code to be compatible. Please check.


----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-692474056


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


----------------------------------------------------------------
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]

CarbonDataQA1 commented on pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#issuecomment-692474670


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


----------------------------------------------------------------
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 #3922:
URL: https://github.com/apache/carbondata/pull/3922#discussion_r488414094



##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -59,13 +60,22 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
 
   private void addBlockEntry(String blockletPath) {
     String blockId =
-        blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+        blockletPath.substring(0, blockletPath.lastIndexOf(File.separator));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {

Review comment:
       Please put the strings in the constant file and then use it.




----------------------------------------------------------------
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]

ShreelekhyaG commented on a change in pull request #3922:
URL: https://github.com/apache/carbondata/pull/3922#discussion_r488507475



##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/conditional/ImplicitExpression.java
##########
@@ -59,13 +60,22 @@ public ImplicitExpression(Map<String, Set<Integer>> blockIdToBlockletIdMapping)
 
   private void addBlockEntry(String blockletPath) {
     String blockId =
-        blockletPath.substring(0, blockletPath.lastIndexOf(CarbonCommonConstants.FILE_SEPARATOR));
+        blockletPath.substring(0, blockletPath.lastIndexOf(File.separator));
+    // Check if blockletPath contains old tuple id format, and convert it to compatible format.
+    if (blockId.contains("batchno")) {

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]