[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-07 Thread Sssan520
GitHub user Sssan520 opened a pull request:

https://github.com/apache/carbondata/pull/2619

[CARBONDATA-2819] Fixed cannot drop preagg datamap on table if the tab…

1.Use "SET carbon.datamap.visible.{dbName}.{mainTable}.{datamapName} = 
{true or false}",if datamap provider is 'preagg',block this configuration.
2.When create preagg datamap, now also create its datamap schema file in 
system folder.
3.Use command "show datamap on table {tableName}" will get its all datamaps 
from datamap schema files

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

 - [ ] Any interfaces changed?No
 
 - [ ] Any backward compatibility impacted? No
 
 - [ ] Document update required? No

 - [ ] Testing done 
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. 



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Sssan520/carbondata droppreagg

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/carbondata/pull/2619.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2619


commit 7884ba86f13b89fb23895400bbac12eaa4e62546
Author: Sssan520 
Date:   2018-08-08T06:27:41Z

[CARBONDATA-2819]Fixed cannot drop preagg datamap on table if the table has 
other index datamaps




---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-08 Thread xuchuanyin
Github user xuchuanyin commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2619#discussion_r208819917
  
--- Diff: 
integration/spark2/src/main/java/org/apache/carbondata/datamap/PreAggregateDataMapProvider.java
 ---
@@ -77,13 +79,15 @@ private void validateDmProperty(DataMapSchema 
dataMapSchema)
   }
 
   @Override
-  public void cleanMeta() {
+  public void cleanMeta() throws IOException {
+DataMapSchema dataMapSchema = getDataMapSchema();
 dropTableCommand = new CarbonDropTableCommand(
 true,
 new Some<>(dbName),
 tableName,
 true);
 dropTableCommand.processMetadata(sparkSession);
+
DataMapStoreManager.getInstance().dropDataMapSchema(dataMapSchema.getDataMapName());
--- End diff --

Can you check how MV deal with this? In another word, what's the logic of 
dropping a MV?


---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-08 Thread xuchuanyin
Github user xuchuanyin commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2619#discussion_r208818959
  
--- Diff: 
examples/spark2/src/main/scala/org/apache/carbondata/examples/PreAggregateDataMapExample.scala
 ---
@@ -175,7 +175,7 @@ object PreAggregateDataMapExample {
 
 // create pre-aggregate table by datamap
 spark.sql("""
-   CREATE datamap preagg_avg on table personTable using 'preaggregate' 
as
+   CREATE datamap preagg_avg_dm on table personTable using 
'preaggregate' as
--- End diff --

why this modification is needed?


---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-08 Thread xuchuanyin
Github user xuchuanyin commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2619#discussion_r208818835
  
--- Diff: 
core/src/main/java/org/apache/carbondata/core/util/SessionParams.java ---
@@ -213,13 +217,20 @@ private boolean validateKeyValue(String key, String 
value) throws InvalidConfigu
   isValid = true;
 } else if 
(key.startsWith(CarbonCommonConstants.CARBON_DATAMAP_VISIBLE)) {
   String[] keyArray = key.split("\\.");
-  isValid = DataMapStoreManager.getInstance().isDataMapExist(
-  keyArray[keyArray.length - 3],
-  keyArray[keyArray.length - 2],
-  keyArray[keyArray.length - 1]);
-  if (!isValid) {
+  try {
+DataMapSchema dataMapSchema =
+
DataMapStoreManager.getInstance().getDataMapSchema(keyArray[keyArray.length - 
1]);
+if (DataMapClassProvider.PREAGGREGATE.getShortName()
--- End diff --

This command is only works for index datamap, so please optimize it again


---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-08 Thread xuchuanyin
Github user xuchuanyin commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2619#discussion_r208821154
  
--- Diff: 
integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/datamap/CarbonDropDataMapCommand.scala
 ---
@@ -114,12 +114,6 @@ case class CarbonDropDataMapCommand(
 LOGGER.audit(s"Deleting datamap [$dataMapName] under table 
[$tableName]")
 
 // drop index datamap on the main table
-if (mainTable != null &&
--- End diff --

Can you explain why this modification is needed?


---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-10 Thread Sssan520
Github user Sssan520 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2619#discussion_r209205953
  
--- Diff: 
examples/spark2/src/main/scala/org/apache/carbondata/examples/PreAggregateDataMapExample.scala
 ---
@@ -175,7 +175,7 @@ object PreAggregateDataMapExample {
 
 // create pre-aggregate table by datamap
 spark.sql("""
-   CREATE datamap preagg_avg on table personTable using 'preaggregate' 
as
+   CREATE datamap preagg_avg_dm on table personTable using 
'preaggregate' as
--- End diff --

As datamap name is a global, there is already the same  datamap name before.


---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-10 Thread Sssan520
Github user Sssan520 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2619#discussion_r209206775
  
--- Diff: 
integration/spark2/src/main/java/org/apache/carbondata/datamap/PreAggregateDataMapProvider.java
 ---
@@ -77,13 +79,15 @@ private void validateDmProperty(DataMapSchema 
dataMapSchema)
   }
 
   @Override
-  public void cleanMeta() {
+  public void cleanMeta() throws IOException {
+DataMapSchema dataMapSchema = getDataMapSchema();
 dropTableCommand = new CarbonDropTableCommand(
 true,
 new Some<>(dbName),
 tableName,
 true);
 dropTableCommand.processMetadata(sparkSession);
+
DataMapStoreManager.getInstance().dropDataMapSchema(dataMapSchema.getDataMapName());
--- End diff --

when create a mv datamap, will create a datamap scheme file and register a 
datamap catalog; then when drop the mv datamap,will delete the datamap schema 
file and unregister the datamap catalog.


---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-10 Thread Sssan520
Github user Sssan520 commented on a diff in the pull request:

https://github.com/apache/carbondata/pull/2619#discussion_r209208751
  
--- Diff: 
integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/datamap/CarbonDropDataMapCommand.scala
 ---
@@ -114,12 +114,6 @@ case class CarbonDropDataMapCommand(
 LOGGER.audit(s"Deleting datamap [$dataMapName] under table 
[$tableName]")
 
 // drop index datamap on the main table
-if (mainTable != null &&
--- End diff --

when drop preagg datamap  , should delete the datamap schema file and main 
tableinfo schema info.


---


[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...

2018-08-16 Thread Sssan520
Github user Sssan520 closed the pull request at:

https://github.com/apache/carbondata/pull/2619


---