[GitHub] [carbondata] jackylk opened a new pull request #3614: [CARBONDATA-3693] Separate Index command from DataMap command

[GitBox]

jackylk commented on a change in pull request #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385612622
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/DataMapProvider.java
 ##########
 @@ -178,7 +180,7 @@ public boolean rebuild() throws IOException, NoSuchDataMapException {
                   == SegmentStatus.INSERT_OVERWRITE_IN_PROGRESS) && SegmentStatusManager
                   .isLoadInProgress(dataMapTableAbsoluteTableIdentifier,
                       loadMetaDetail.getLoadName())) {
-                throw new RuntimeException("Rebuild to datamap " + dataMapSchema.getDataMapName()
+                throw new RuntimeException("Rebuild to " + dataMapSchema.getDataMapName()
 
 Review comment:
   fixed

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385613902
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java
 ##########
 @@ -168,6 +186,38 @@ public DataMapSchema getDataMapSchema(String dataMapName)
     return provider.retrieveSchema(dataMapName);
   }
 
+  public List<DataMapSchema> getAllMVSchemas() throws IOException {
+    return provider.retrieveAllSchemas().stream()
+        .filter(schema -> !schema.isIndexDataMap())
+        .collect(Collectors.toList());
+  }
+
+  public DataMapSchema getIndexSchema(String indexName)
+      throws NoSuchIndexException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(indexName);
+      if (!schema.isIndexDataMap()) {
+        throw new NoSuchIndexException(indexName);
+      }
+      return schema;
+    } catch (NoSuchDataMapException e) {
 
 Review comment:
   log added

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385613902
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java
 ##########
 @@ -168,6 +186,38 @@ public DataMapSchema getDataMapSchema(String dataMapName)
     return provider.retrieveSchema(dataMapName);
   }
 
+  public List<DataMapSchema> getAllMVSchemas() throws IOException {
+    return provider.retrieveAllSchemas().stream()
+        .filter(schema -> !schema.isIndexDataMap())
+        .collect(Collectors.toList());
+  }
+
+  public DataMapSchema getIndexSchema(String indexName)
+      throws NoSuchIndexException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(indexName);
+      if (!schema.isIndexDataMap()) {
+        throw new NoSuchIndexException(indexName);
+      }
+      return schema;
+    } catch (NoSuchDataMapException e) {
 
 Review comment:
   log added. If reading schema failed, IOexception will be thrown

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385614720
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java
 ##########
 @@ -168,6 +186,38 @@ public DataMapSchema getDataMapSchema(String dataMapName)
     return provider.retrieveSchema(dataMapName);
   }
 
+  public List<DataMapSchema> getAllMVSchemas() throws IOException {
+    return provider.retrieveAllSchemas().stream()
+        .filter(schema -> !schema.isIndexDataMap())
+        .collect(Collectors.toList());
+  }
+
+  public DataMapSchema getIndexSchema(String indexName)
+      throws NoSuchIndexException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(indexName);
+      if (!schema.isIndexDataMap()) {
+        throw new NoSuchIndexException(indexName);
+      }
+      return schema;
+    } catch (NoSuchDataMapException e) {
+      throw new NoSuchIndexException(indexName);
+    }
+  }
+
+  public DataMapSchema getMVSchema(String mvName)
+      throws NoSuchMaterializedViewException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(mvName);
+      if (schema == null || schema.isIndexDataMap()) {
+        throw new NoSuchMaterializedViewException(mvName);
+      }
+      return schema;
+    } catch (NoSuchDataMapException e) {
+      throw new NoSuchMaterializedViewException(mvName);
 
 Review comment:
   log added

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385615266
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java
 ##########
 @@ -168,6 +186,38 @@ public DataMapSchema getDataMapSchema(String dataMapName)
     return provider.retrieveSchema(dataMapName);
   }
 
+  public List<DataMapSchema> getAllMVSchemas() throws IOException {
+    return provider.retrieveAllSchemas().stream()
+        .filter(schema -> !schema.isIndexDataMap())
+        .collect(Collectors.toList());
+  }
+
+  public DataMapSchema getIndexSchema(String indexName)
+      throws NoSuchIndexException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(indexName);
+      if (!schema.isIndexDataMap()) {
+        throw new NoSuchIndexException(indexName);
+      }
+      return schema;
+    } catch (NoSuchDataMapException e) {
+      throw new NoSuchIndexException(indexName);
+    }
+  }
+
+  public DataMapSchema getMVSchema(String mvName)
+      throws NoSuchMaterializedViewException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(mvName);
+      if (schema == null || schema.isIndexDataMap()) {
+        throw new NoSuchMaterializedViewException(mvName);
 
 Review comment:
   In this case, no materialized view is found. I think throwing exception is enough. What kind of log do you suggest?

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385615513
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/status/DataMapStatus.java
 ##########
 @@ -18,7 +18,7 @@
 package org.apache.carbondata.core.datamap.status;
 
 /**
- * DataMap status
+ * Index status
  */
 public enum DataMapStatus {
 
 Review comment:
   But MV also uses this, right?

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385616001
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/status/DataMapStatusManager.java
 ##########
 @@ -142,13 +142,13 @@ public static DataMapSchema getDataMapSchema(String dataMapName)
   }
 
   /**
-   * This method will remove all segments of dataMap table in case of Insert-Overwrite/Update/Delete
+   * This method will remove all segments of MV table in case of Insert-Overwrite/Update/Delete
    * operations on main table
    *
    * @param allDataMapSchemas of main carbon table
    * @throws IOException
    */
-  public static void truncateDataMap(List<DataMapSchema> allDataMapSchemas)
+  public static void truncateMVTable(List<DataMapSchema> allDataMapSchemas)
 
 Review comment:
   DataMapSchema can not be changed in this PR since Index and MV still both use it. Metadata unification will be done in another PR.
   I think @niuge01  is doing 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]


With regards,
Apache Git Services

[GitBox]

jackylk commented on a change in pull request #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385616775
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/index/IndexLevel.java
 ##########
 @@ -15,17 +15,17 @@
  * limitations under the License.
  */
 
-package org.apache.carbondata.core.datamap;
+package org.apache.carbondata.core.index;
 
 import org.apache.carbondata.common.annotations.InterfaceAudience;
 import org.apache.carbondata.common.annotations.InterfaceStability;
 
 /**
- * Index level of the datamap
+ * Index level
 
 Review comment:
   fixed

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385616968
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/index/TableIndex.java
 ##########
 @@ -338,7 +339,7 @@ public Void call() throws IOException {
     try {
       executorService.awaitTermination(2, TimeUnit.HOURS);
     } catch (InterruptedException e) {
-      LOG.error("Error in pruning datamap in multi-thread: " + e.getMessage());
+      LOG.error("Error in pruning in multi-thread: " + e.getMessage());
 
 Review comment:
   fixed

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385617240
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/index/dev/BlockletSerializer.java
 ##########
 @@ -15,19 +15,19 @@
  * limitations under the License.
  */
 
-package org.apache.carbondata.core.datamap.dev;
+package org.apache.carbondata.core.index.dev;
 
 import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
 
 import org.apache.carbondata.common.annotations.InterfaceAudience;
-import org.apache.carbondata.core.datamap.dev.fgdatamap.FineGrainBlocklet;
 import org.apache.carbondata.core.datastore.impl.FileFactory;
+import org.apache.carbondata.core.index.dev.fgindex.FineGrainBlocklet;
 
 /**
  * A serializer/deserializer for {@link FineGrainBlocklet}, it is used after prune the data
- * by {@link org.apache.carbondata.core.datamap.dev.fgdatamap.FineGrainDataMap}
+ * by {@link FineGrainIndex}
 
 Review comment:
   fixed

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385617862
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/index/DistributableIndexFormat.java
 ##########
 @@ -29,16 +29,17 @@
 
 import org.apache.carbondata.common.logging.LogServiceFactory;
 import org.apache.carbondata.core.constants.CarbonCommonConstants;
-import org.apache.carbondata.core.datamap.dev.expr.DataMapDistributableWrapper;
+import org.apache.carbondata.core.datamap.DataMapStoreManager;
+import org.apache.carbondata.core.datamap.Segment;
 import org.apache.carbondata.core.datastore.impl.FileFactory;
+import org.apache.carbondata.core.index.dev.expr.IndexDistributableWrapper;
 import org.apache.carbondata.core.indexstore.ExtendedBlocklet;
 import org.apache.carbondata.core.indexstore.PartitionSpec;
 import org.apache.carbondata.core.metadata.schema.table.CarbonTable;
 import org.apache.carbondata.core.readcommitter.LatestFilesReadCommittedScope;
 import org.apache.carbondata.core.readcommitter.ReadCommittedScope;
 import org.apache.carbondata.core.readcommitter.TableStatusReadCommittedScope;
 import org.apache.carbondata.core.scan.filter.resolver.FilterResolverIntf;
-
 
 Review comment:
   This line is not required

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r385618823
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/status/DiskBasedDataMapStatusProvider.java
 ##########
 @@ -31,11 +31,11 @@
 
 import org.apache.carbondata.common.logging.LogServiceFactory;
 import org.apache.carbondata.core.constants.CarbonCommonConstants;
-import org.apache.carbondata.core.datamap.dev.DataMapSyncStatus;
 import org.apache.carbondata.core.datastore.impl.FileFactory;
 import org.apache.carbondata.core.fileoperations.AtomicFileOperationFactory;
 import org.apache.carbondata.core.fileoperations.AtomicFileOperations;
 import org.apache.carbondata.core.fileoperations.FileWriteOperation;
+import org.apache.carbondata.core.index.dev.DataMapSyncStatus;
 
 Review comment:
   no, package name is changed

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#issuecomment-592471657
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2229/
   

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#issuecomment-592472131
 
 
   Build Failed  with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/529/
   

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#issuecomment-592807190
 
 
   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]


With regards,
Apache Git Services

[GitBox]

CarbonDataQA1 commented on issue #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#issuecomment-592821671
 
 
   Build Failed  with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/534/
   

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#issuecomment-592822066
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2234/
   

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#issuecomment-593088362
 
 
   Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/548/
   

----------------------------------------------------------------
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 #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#issuecomment-593091305
 
 
   Build Failed  with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/2248/
   

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

akashrn5 commented on a change in pull request #3614: [CARBONDATA-3693] Separate Index command from DataMap command
URL: https://github.com/apache/carbondata/pull/3614#discussion_r386205955
 
 

 ##########
 File path: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java
 ##########
 @@ -168,6 +186,38 @@ public DataMapSchema getDataMapSchema(String dataMapName)
     return provider.retrieveSchema(dataMapName);
   }
 
+  public List<DataMapSchema> getAllMVSchemas() throws IOException {
+    return provider.retrieveAllSchemas().stream()
+        .filter(schema -> !schema.isIndexDataMap())
+        .collect(Collectors.toList());
+  }
+
+  public DataMapSchema getIndexSchema(String indexName)
+      throws NoSuchIndexException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(indexName);
+      if (!schema.isIndexDataMap()) {
+        throw new NoSuchIndexException(indexName);
+      }
+      return schema;
+    } catch (NoSuchDataMapException e) {
+      throw new NoSuchIndexException(indexName);
+    }
+  }
+
+  public DataMapSchema getMVSchema(String mvName)
+      throws NoSuchMaterializedViewException, IOException {
+    try {
+      DataMapSchema schema = provider.retrieveSchema(mvName);
+      if (schema == null || schema.isIndexDataMap()) {
+        throw new NoSuchMaterializedViewException(mvName);
 
 Review comment:
   just log error with message and the throwable, if we directly throw may be sometimes if the cause is different it will mislead

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