[GitHub] [carbondata] ravipesala opened a new pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.

[GitBox]

ravipesala opened a new pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571
 
 
    ### Why is this PR needed?
    When alluxio path is provided without host and port like alluxio:///user/warehouse then carbon cannot read or write data because of path comparison fails and extracting parent path fails.
   
    ### What changes were proposed in this PR?
     Use Path object to compare paths. And use string utils to extract the parent path.
       
    ### Does this PR introduce any user interface change?
    - No
   
    ### Is any new testcase added?
    - No
   
       
   

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#issuecomment-572532495
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1565/
   

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r364779297
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AbstractDFSCarbonFile.java
 ##########
 @@ -554,6 +555,21 @@ public short getDefaultReplication() {
     return fileSystem.getDefaultReplication(path);
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
 
 Review comment:
   add `{` and `}`

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


With regards,
Apache Git Services

[GitBox]

ravipesala commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r365178265
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datastore/filesystem/AbstractDFSCarbonFile.java
 ##########
 @@ -554,6 +555,21 @@ public short getDefaultReplication() {
     return fileSystem.getDefaultReplication(path);
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
 
 Review comment:
   ok

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#issuecomment-573014185
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1585/
   

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r365520617
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockDataMap.java
 ##########
 @@ -137,12 +137,14 @@ public void init(DataMapModel dataMapModel)
         // if the segment data is written in tablepath then no need to store whole path of file.
         !blockletDataMapInfo.getFilePath().startsWith(
             blockletDataMapInfo.getCarbonTable().getTablePath())) {
-      filePath = path.getParent().toString().getBytes(CarbonCommonConstants.DEFAULT_CHARSET);
+      filePath =
+          path.substring(0, path.lastIndexOf("/")).getBytes(CarbonCommonConstants.DEFAULT_CHARSET);
 
 Review comment:
   I found that you can use `FilenameUtils.getFullPathNoEndSeparator(file)` from Apache Commons
   

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r365520720
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/BlockletDataMapUtil.java
 ##########
 @@ -189,7 +189,7 @@ private static BlockMetaInfo createBlockMetaInfo(
         CarbonFile carbonFile = FileFactory.getCarbonFile(carbonDataFile);
         return new BlockMetaInfo(new String[] { "localhost" }, carbonFile.getSize());
       default:
-        return fileNameToMetaInfoMapping.get(carbonDataFile);
+        return fileNameToMetaInfoMapping.get(FileFactory.getCarbonFile(carbonDataFile).getPath());
 
 Review comment:
   please add comment why this is required? Can we add a case for Alluxio separately?

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r365520784
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/BlockletDataMapUtil.java
 ##########
 @@ -198,9 +198,10 @@ private static BlockMetaInfo createBlockMetaInfo(
     Set<TableBlockIndexUniqueIdentifier> tableBlockIndexUniqueIdentifiers = new HashSet<>();
     Map<String, String> indexFiles = segment.getCommittedIndexFile();
     for (Map.Entry<String, String> indexFileEntry : indexFiles.entrySet()) {
-      Path indexFile = new Path(indexFileEntry.getKey());
+      String indexFile = indexFileEntry.getKey();
       tableBlockIndexUniqueIdentifiers.add(
-          new TableBlockIndexUniqueIdentifier(indexFile.getParent().toString(), indexFile.getName(),
+          new TableBlockIndexUniqueIdentifier(indexFile.substring(0, indexFile.lastIndexOf("/")),
 
 Review comment:
   use FileNameUtils from Apache Commons

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


With regards,
Apache Git Services

[GitBox]

ravipesala commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r366286473
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/indexstore/blockletindex/BlockDataMap.java
 ##########
 @@ -137,12 +137,14 @@ public void init(DataMapModel dataMapModel)
         // if the segment data is written in tablepath then no need to store whole path of file.
         !blockletDataMapInfo.getFilePath().startsWith(
             blockletDataMapInfo.getCarbonTable().getTablePath())) {
-      filePath = path.getParent().toString().getBytes(CarbonCommonConstants.DEFAULT_CHARSET);
+      filePath =
+          path.substring(0, path.lastIndexOf("/")).getBytes(CarbonCommonConstants.DEFAULT_CHARSET);
 
 Review comment:
   ok

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


With regards,
Apache Git Services

[GitBox]

ravipesala commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r366288754
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/BlockletDataMapUtil.java
 ##########
 @@ -189,7 +189,7 @@ private static BlockMetaInfo createBlockMetaInfo(
         CarbonFile carbonFile = FileFactory.getCarbonFile(carbonDataFile);
         return new BlockMetaInfo(new String[] { "localhost" }, carbonFile.getSize());
       default:
-        return fileNameToMetaInfoMapping.get(carbonDataFile);
+        return fileNameToMetaInfoMapping.get(FileFactory.getCarbonFile(carbonDataFile).getPath());
 
 Review comment:
   ok

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


With regards,
Apache Git Services

[GitBox]

ravipesala commented on a change in pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#discussion_r366292603
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/util/BlockletDataMapUtil.java
 ##########
 @@ -198,9 +198,10 @@ private static BlockMetaInfo createBlockMetaInfo(
     Set<TableBlockIndexUniqueIdentifier> tableBlockIndexUniqueIdentifiers = new HashSet<>();
     Map<String, String> indexFiles = segment.getCommittedIndexFile();
     for (Map.Entry<String, String> indexFileEntry : indexFiles.entrySet()) {
-      Path indexFile = new Path(indexFileEntry.getKey());
+      String indexFile = indexFileEntry.getKey();
       tableBlockIndexUniqueIdentifiers.add(
-          new TableBlockIndexUniqueIdentifier(indexFile.getParent().toString(), indexFile.getName(),
+          new TableBlockIndexUniqueIdentifier(indexFile.substring(0, indexFile.lastIndexOf("/")),
 
 Review comment:
   ok

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


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#issuecomment-574165597
 
 
   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1637/
   

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


With regards,
Apache Git Services

[GitBox]

jackylk commented on issue #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571#issuecomment-574193569
 
 
   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]


With regards,
Apache Git Services

[GitBox]

asfgit closed pull request #3571: [CARBONDATA-3659] Fix issues with alluxio without host and port.
URL: https://github.com/apache/carbondata/pull/3571
 
 
   

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


With regards,
Apache Git Services