[CARBONDATA-2347][LUCENE]change datamap factory interface to check supported features #2202

akashrn5 · 2018-04-21T10:20:15Z

Added new method to interface which will decide where the table operation present in list of table operations is allowed on the datamap or not.

Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:

Any interfaces changed?
yes
Any backward compatibility impacted?
NA
Document update required?
NA
Testing done
UT tests are run
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
NA

CarbonDataQA · 2018-04-21T12:02:17Z

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4098/

CarbonDataQA · 2018-04-21T13:02:17Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5277/

jackylk · 2018-04-22T09:18:51Z

core/src/main/java/org/apache/carbondata/core/features/FeaturesList.java

+
+package org.apache.carbondata.core.features;
+
+public enum FeaturesList {


change to TableOperation

jackylk · 2018-04-22T09:25:17Z

core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java

+   * @param feature
+   * @return
+   */
+  public static boolean validateFeatureForDatamap(CarbonTable carbonTable, FeaturesList feature) {


move this function and rename to CarbonTable.canAllow, it should return true if the input operation is allowed. If this operation will make the datamap becomes stale, then it is not allowed. Please modify the comment accordingly.

In future, we can change the behavior without changing this API. (After we can incremental refresh the index datamap, carbon table can allow the operation even if it will make the datamap stale)

jackylk · 2018-04-22T09:25:56Z

core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java

+      List<TableDataMap> datamaps = DataMapStoreManager.getInstance().getAllDataMap(carbonTable);
+      if (!datamaps.isEmpty()) {
+        for (TableDataMap dataMap : datamaps) {
+          DataMapFactory factoryClass = DataMapStoreManager.getInstance()


Since we are adding more functionality to the DataMapFactory, it is no long a Factory only. So I suggest to change DataMapFactory class name to IndexDataMap

jackylk · 2018-04-22T09:28:40Z

core/src/main/java/org/apache/carbondata/core/util/CarbonUtil.java

+        for (TableDataMap dataMap : datamaps) {
+          DataMapFactory factoryClass = DataMapStoreManager.getInstance()
+              .getDataMapFactoryClass(dataMap.getDataMapSchema());
+          isSupported = factoryClass.dataMapFeatureScope(feature);


rename dataMapFeatureScope to willBecomeStale. This function should return true is the input operation enum will make the datamap become stale. Please add this comment to that function

jackylk · 2018-04-22T09:35:02Z

...main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableRenameCommand.scala

@@ -77,6 +78,11 @@ private[sql] case class CarbonAlterTableRenameCommand(
    var oldCarbonTable: CarbonTable = null
    oldCarbonTable = metastore.lookupRelation(Some(oldDatabaseName), oldTableName)(sparkSession)
      .asInstanceOf[CarbonRelation].carbonTable
+
+    if (CarbonUtil.validateFeatureForDatamap(oldCarbonTable, FeaturesList.ALTER_RENAME)) {


After you handle previous comments, I think this line will become:
if (!oldCarbonTable.canAllow(TableOperation.ALTER_RENAME))

CarbonDataQA · 2018-04-23T07:58:39Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5321/

CarbonDataQA · 2018-04-23T09:11:24Z

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4146/

CarbonDataQA · 2018-04-23T12:12:43Z

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4152/

ravipesala · 2018-04-23T13:05:46Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4476/

CarbonDataQA · 2018-04-23T14:14:34Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5331/

akashrn5 · 2018-04-23T14:45:57Z

retest this please

akashrn5 · 2018-04-23T14:46:06Z

retest sdv please

CarbonDataQA · 2018-04-23T17:47:10Z

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4165/

ravipesala · 2018-04-23T17:52:38Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4483/

CarbonDataQA · 2018-04-23T23:00:49Z

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4177/

ravipesala · 2018-04-23T23:23:56Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4492/

ravipesala · 2018-04-24T04:16:44Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4498/

akashrn5 · 2018-04-24T06:25:16Z

retest this please

CarbonDataQA · 2018-04-24T07:53:25Z

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4193/

ravipesala · 2018-04-26T15:02:02Z

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4559/

CarbonDataQA · 2018-04-26T18:44:21Z

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4267/

CarbonDataQA · 2018-04-26T19:29:36Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5434/

CarbonDataQA · 2018-04-26T20:47:16Z

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4273/

akashrn5 · 2018-04-27T05:09:50Z

retest this please

CarbonDataQA · 2018-04-27T06:03:28Z

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4287/

CarbonDataQA · 2018-04-27T08:02:25Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5452/

ravipesala · 2018-04-27T12:36:27Z

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4574/

ravipesala · 2018-04-27T15:53:12Z

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4578/

CarbonDataQA · 2018-04-27T18:53:14Z

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4311/

CarbonDataQA · 2018-04-27T18:53:59Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5478/

CarbonDataQA · 2018-04-27T20:34:50Z

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4315/

CarbonDataQA · 2018-04-27T20:34:50Z

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5482/

CarbonDataQA · 2018-04-28T13:01:44Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5505/

CarbonDataQA · 2018-04-28T14:01:44Z

Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4340/

ravipesala · 2018-04-28T16:44:35Z

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4608/

CarbonDataQA · 2018-04-30T06:12:30Z

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5523/

CarbonDataQA · 2018-04-30T06:13:56Z

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4359/

ravipesala · 2018-04-30T07:19:50Z

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4627/

jackylk · 2018-05-01T07:04:54Z

LGTM

…pported features Added new method to interface which will decide where the table operation present in list of table operations is allowed on the datamap or not. This closes apache#2202

jackylk reviewed Apr 22, 2018

View reviewed changes

akashrn5 force-pushed the block_features_lucene branch 2 times, most recently from 59a799e to d45cdd1 Compare April 23, 2018 06:37

akashrn5 changed the title ~~[wip]change datamap factory interface to check supported features~~ [CARBONDATA-2347][LUCENE]change datamap factory interface to check supported features Apr 23, 2018

akashrn5 force-pushed the block_features_lucene branch 4 times, most recently from c7ea04a to e839ad5 Compare April 23, 2018 09:00

akashrn5 force-pushed the block_features_lucene branch from e839ad5 to 79c5a43 Compare April 23, 2018 11:52

akashrn5 force-pushed the block_features_lucene branch from 79c5a43 to 951f00a Compare April 23, 2018 18:11

akashrn5 force-pushed the block_features_lucene branch 2 times, most recently from efc84c8 to f5b1cc8 Compare April 26, 2018 14:43

akashrn5 force-pushed the block_features_lucene branch 3 times, most recently from 67ceb98 to 3924eb3 Compare April 27, 2018 14:48

akashrn5 force-pushed the block_features_lucene branch from 3924eb3 to f72d8c0 Compare April 28, 2018 11:55

change datamap factory interface to check supported features

f316ccc

akashrn5 force-pushed the block_features_lucene branch from f72d8c0 to f316ccc Compare April 30, 2018 05:15

asfgit closed this in 5229443 May 1, 2018


		package org.apache.carbondata.core.features;

		public enum FeaturesList {

[CARBONDATA-2347][LUCENE]change datamap factory interface to check supported features #2202

[CARBONDATA-2347][LUCENE]change datamap factory interface to check supported features #2202

Conversation

akashrn5 commented Apr 21, 2018 • edited

CarbonDataQA commented Apr 21, 2018

CarbonDataQA commented Apr 21, 2018

jackylk Apr 22, 2018 • edited

Choose a reason for hiding this comment

jackylk Apr 22, 2018 • edited

Choose a reason for hiding this comment

jackylk Apr 22, 2018 • edited

Choose a reason for hiding this comment

jackylk Apr 22, 2018 • edited

Choose a reason for hiding this comment

jackylk Apr 22, 2018 • edited

Choose a reason for hiding this comment

CarbonDataQA commented Apr 23, 2018

CarbonDataQA commented Apr 23, 2018

CarbonDataQA commented Apr 23, 2018

ravipesala commented Apr 23, 2018

CarbonDataQA commented Apr 23, 2018

akashrn5 commented Apr 23, 2018

akashrn5 commented Apr 23, 2018

CarbonDataQA commented Apr 23, 2018

ravipesala commented Apr 23, 2018

CarbonDataQA commented Apr 23, 2018

ravipesala commented Apr 23, 2018

ravipesala commented Apr 24, 2018

akashrn5 commented Apr 24, 2018

CarbonDataQA commented Apr 24, 2018

ravipesala commented Apr 26, 2018

CarbonDataQA commented Apr 26, 2018

CarbonDataQA commented Apr 26, 2018

CarbonDataQA commented Apr 26, 2018

akashrn5 commented Apr 27, 2018

CarbonDataQA commented Apr 27, 2018

CarbonDataQA commented Apr 27, 2018

ravipesala commented Apr 27, 2018

ravipesala commented Apr 27, 2018

CarbonDataQA commented Apr 27, 2018

CarbonDataQA commented Apr 27, 2018

CarbonDataQA commented Apr 27, 2018

CarbonDataQA commented Apr 27, 2018

CarbonDataQA commented Apr 28, 2018

CarbonDataQA commented Apr 28, 2018

ravipesala commented Apr 28, 2018

CarbonDataQA commented Apr 30, 2018

CarbonDataQA commented Apr 30, 2018

ravipesala commented Apr 30, 2018

jackylk commented May 1, 2018

akashrn5 commented Apr 21, 2018 •

edited

jackylk Apr 22, 2018 •

edited

jackylk Apr 22, 2018 •

edited

jackylk Apr 22, 2018 •

edited

jackylk Apr 22, 2018 •

edited

jackylk Apr 22, 2018 •

edited