[GitHub] carbondata pull request #2619: [CARBONDATA-2819] Fixed cannot drop preagg da...
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...
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...
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...
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...
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...
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...
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...
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...
Github user Sssan520 closed the pull request at: https://github.com/apache/carbondata/pull/2619 ---