You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by Sssan520 <gi...@git.apache.org> on 2018/07/11 12:31:41 UTC
[GitHub] carbondata pull request #2491: [CARBONDATA-2700][BloomDataMap] Block droppin...
GitHub user Sssan520 opened a pull request:
https://github.com/apache/carbondata/pull/2491
[CARBONDATA-2700][BloomDataMap] Block dropping index columns for bloomfilter index datamap
1.Block create bloomfilter datamap index on column which its datatype is complex type;
2.Block create bloomfilter datamap index on local_dictionary column;
3.Block change datatype for bloomfilter index datamap;
4.Block dropping index columns for bloomfilter index datamap
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
add a parameter "targets" for method "canAllow" of "CarbonTable" class
- [ ] Any backward compatibility impacted? No
- [ ] Document update required? No
- [ ] Testing done NA
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 bloomfilter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2491.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 #2491
----
commit 3d6597f4c51faefa191a1c068fa1b1c8be4d9f67
Author: lianganping 00251374 <li...@...>
Date: 2018-07-11T12:17:09Z
1.Block create bloomfilter datamap index on column which its datatype is complex;2.Block create bloomfilter datamap index on local_dictionary column;3.Block change datatype for bloomfilter index datamap;4.Block dropping index columns for bloomfilter index datamap
----
---
[GitHub] carbondata issue #2491: [CARBONDATA-2700][BloomDataMap] Block dropping index...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2491
Can one of the admins verify this patch?
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 closed the pull request at:
https://github.com/apache/carbondata/pull/2491
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 closed the pull request at:
https://github.com/apache/carbondata/pull/2491
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202221623
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java ---
@@ -1058,9 +1058,10 @@ public void setTransactionalTable(boolean transactionalTable) {
* if this operation makes datamap stale it is not allowed
* @param carbonTable
* @param operation
+ * @param targets
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
GitHub user Sssan520 reopened a pull request:
https://github.com/apache/carbondata/pull/2491
[CARBONDATA-2698][CARBONDATA-2700][CARBONDATA-2732][BloomDataMap] block some oprerations of bloomfilter datamap
1.Block create bloomfilter datamap index on column which its datatype is complex type;
2.Block change datatype for bloomfilter index datamap;
3.Block dropping index columns for bloomfilter index datamap
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
add a parameter "targets" for method "canAllow" of "CarbonTable" class
- [ ] Any backward compatibility impacted? No
- [ ] Document update required? No
- [ ] Testing done NA
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 bloomfilter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2491.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 #2491
----
commit 815c497380fa52b98aa270d35ce67f2cb9c67191
Author: lianganping 00251374 <li...@...>
Date: 2018-07-11T12:17:09Z
1.Block create bloomfilter datamap index on column which its datatype is complex;2.Block create bloomfilter datamap index on local_dictionary column;3.Block change datatype for bloomfilter index datamap;4.Block dropping index columns for bloomfilter index datamap
----
---
[GitHub] carbondata issue #2491: [CARBONDATA-2700][BloomDataMap] Block dropping index...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2491
Can one of the admins verify this patch?
---
[GitHub] carbondata issue #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA-2730][...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on the issue:
https://github.com/apache/carbondata/pull/2491
retest this please
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202221696
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java ---
@@ -1071,6 +1072,10 @@ public boolean canAllow(CarbonTable carbonTable, TableOperation operation) {
if (factoryClass.willBecomeStale(operation)) {
return false;
}
+ //to check something like columns by the operation
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202000213
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/datamap/CarbonCreateDataMapCommand.scala ---
@@ -109,6 +111,19 @@ case class CarbonCreateDataMapCommand(
throw new MalformedDataMapCommandException(String.format(
"column '%s' already has %s index datamap created",
column.getColName, thisDmProviderName))
+ } else if (isBloomFilter) {
+ // if datamap provider is bloomfilter,the index column datatype cannot be complex type
+ if (column.isComplex) {
+ throw new MalformedDataMapCommandException(String.format(
+ "since datamap provider is bloomfilter , the index column '%s' datatype cannot be" +
--- End diff --
optimize the message format like "BloomFilter datamap does not support complex datatype column: ${column.getColName}"
The same for the below if branch
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201977727
--- Diff: datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapFactory.java ---
@@ -380,6 +380,49 @@ public void deleteDatamapData() {
return false;
}
+ @Override public boolean isOperationBlock(TableOperation operation, Object... targets) {
--- End diff --
2. better to optimize the name of method to 'isOperationBlocked'
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202228474
--- Diff: integration/spark2/src/test/scala/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapSuite.scala ---
@@ -377,7 +382,91 @@ class BloomCoarseGrainDataMapSuite extends QueryTest with BeforeAndAfterAll with
checkQuery("fakeDm", shouldHit = false)
}
+ test("test blocl change datatype for bloomfilter index datamap") {
--- End diff --
yes
---
[GitHub] carbondata issue #2491: [CARBONDATA-2700][BloomDataMap] block some opreratio...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on the issue:
https://github.com/apache/carbondata/pull/2491
retest this please
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 closed the pull request at:
https://github.com/apache/carbondata/pull/2491
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201998117
--- Diff: datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapFactory.java ---
@@ -380,6 +380,49 @@ public void deleteDatamapData() {
return false;
}
+ @Override public boolean isOperationBlock(TableOperation operation, Object... targets) {
+ switch (operation) {
+ case ALTER_DROP: {
+ try {
+ //alter table drop columns
--- End diff --
add space after "//"
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202000836
--- Diff: integration/spark2/src/test/scala/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapSuite.scala ---
@@ -377,7 +382,91 @@ class BloomCoarseGrainDataMapSuite extends QueryTest with BeforeAndAfterAll with
checkQuery("fakeDm", shouldHit = false)
}
+ test("test blocl change datatype for bloomfilter index datamap") {
+ sql(
+ s"""
+ | CREATE TABLE $normalTable(id INT, name STRING, city STRING, age INT,
+ | s1 STRING, s2 STRING, s3 STRING, s4 STRING, s5 STRING, s6 STRING, s7 STRING, s8 STRING)
+ | STORED BY 'carbondata' TBLPROPERTIES('table_blocksize'='128')
+ | """.stripMargin)
+
+ sql(
+ s"""
+ | CREATE DATAMAP $dataMapName ON TABLE $normalTable
+ | USING 'bloomfilter' WITH DEFERRED REBUILD
+ | DMProperties( 'INDEX_COLUMNS'='city,id', 'BLOOM_SIZE'='640000')
+ """.stripMargin)
+ val exception: MalformedCarbonCommandException = intercept[MalformedCarbonCommandException] {
+ sql(s"ALTER TABLE $normalTable CHANGE id id bigint")
+ }
+ assert(exception.getMessage
+ .contains("alter table change datatype is not supported for index datamap"))
+ }
+
+ test("test drop index columns for bloomfilter datamap") {
+ sql(
+ s"""
+ | CREATE TABLE $normalTable(id INT, name STRING, city STRING, age INT,
+ | s1 STRING, s2 STRING, s3 STRING, s4 STRING, s5 STRING, s6 STRING, s7 STRING, s8 STRING)
+ | STORED BY 'carbondata' TBLPROPERTIES('table_blocksize'='128')
+ | """.stripMargin)
+ sql(
+ s"""
+ | CREATE DATAMAP $dataMapName ON TABLE $normalTable
+ | USING 'bloomfilter'
+ | WITH DEFERRED REBUILD
+ | DMProperties('INDEX_COLUMNS'='city,id', 'BLOOM_SIZE'='640000')
+ """.stripMargin)
+ val exception: MalformedCarbonCommandException = intercept[MalformedCarbonCommandException] {
+ sql(s"alter table $normalTable drop columns(name, id)")
+ }
+ assert(exception.getMessage
+ .contains("alter table drop column is not supported for index datamap"))
+ }
+
+ test("test create bloomfilter datamap which index column is localdictcolumn ") {
+ sql(
+ s"""
+ | CREATE TABLE $normalTable(id INT, name STRING, city STRING, age INT,
+ | s1 STRING, s2 STRING, s3 STRING, s4 STRING, s5 STRING, s6 STRING, s7 STRING, s8 STRING)
+ | STORED BY 'carbondata' TBLPROPERTIES('local_dictionary_enable'='true',
+ | 'local_dictionary_include'='city')
+ | """.stripMargin)
+ val exception: MalformedDataMapCommandException = intercept[MalformedDataMapCommandException] {
+ sql(
+ s"""
+ | CREATE DATAMAP $dataMapName ON TABLE $normalTable
+ | USING 'bloomfilter'
+ | WITH DEFERRED REBUILD
+ | DMProperties('INDEX_COLUMNS'='city,id', 'BLOOM_SIZE'='640000')
+ """.stripMargin)
+ }
+ assert(exception.getMessage
+ .contains("cannot be localDictColumn"))
+ }
+
+ test("test create bloomfilter datamap which index column datatype is complex ") {
+ sql(
+ s"""
+ | CREATE TABLE $normalTable(id INT, name STRING, city Array<INT>, age INT,
+ | s1 STRING, s2 STRING, s3 STRING, s4 STRING, s5 STRING, s6 STRING, s7 STRING, s8 STRING)
+ | STORED BY 'carbondata'
+ | """.stripMargin)
+ val exception: MalformedDataMapCommandException = intercept[MalformedDataMapCommandException] {
+ sql(
+ s"""
+ | CREATE DATAMAP $dataMapName ON TABLE $normalTable
+ | USING 'bloomfilter'
+ | WITH DEFERRED REBUILD
+ | DMProperties('INDEX_COLUMNS'='city,id', 'BLOOM_SIZE'='640000')
+ """.stripMargin)
+ }
+ assert(exception.getMessage
--- End diff --
please optimize the indent
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202226296
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableDropColumnCommand.scala ---
@@ -58,7 +58,10 @@ private[sql] case class CarbonAlterTableDropColumnCommand(
.validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
- if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_DROP)) {
+ if (!carbonTable
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202221707
--- Diff: datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapFactory.java ---
@@ -380,6 +380,49 @@ public void deleteDatamapData() {
return false;
}
+ @Override public boolean isOperationBlock(TableOperation operation, Object... targets) {
+ switch (operation) {
+ case ALTER_DROP: {
+ try {
+ //alter table drop columns
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
GitHub user Sssan520 reopened a pull request:
https://github.com/apache/carbondata/pull/2491
[CARBONDATA-2698][CARBONDATA-2700][CARBONDATA-2730][CARBONDATA-2732][BloomDataMap] block some oprerations of bloomfilter datamap
1.Block create bloomfilter datamap index on column which its datatype is complex type;
2.Block change datatype for bloomfilter index datamap;
3.Block dropping index columns for bloomfilter index datamap
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
add a parameter "targets" for method "canAllow" of "CarbonTable" class
- [ ] Any backward compatibility impacted? No
- [ ] Document update required? No
- [ ] Testing done NA
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 bloomfilter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2491.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 #2491
----
commit 815c497380fa52b98aa270d35ce67f2cb9c67191
Author: lianganping 00251374 <li...@...>
Date: 2018-07-11T12:17:09Z
1.Block create bloomfilter datamap index on column which its datatype is complex;2.Block create bloomfilter datamap index on local_dictionary column;3.Block change datatype for bloomfilter index datamap;4.Block dropping index columns for bloomfilter index datamap
----
---
[GitHub] carbondata issue #2491: [CARBONDATA-2700][BloomDataMap] Block dropping index...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on the issue:
https://github.com/apache/carbondata/pull/2491
better to change the title of the PR
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202000492
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableDropColumnCommand.scala ---
@@ -58,7 +58,10 @@ private[sql] case class CarbonAlterTableDropColumnCommand(
.validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
- if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_DROP)) {
+ if (!carbonTable
--- End diff --
please optimize the indent
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201999690
--- Diff: integration/spark-common/src/main/scala/org/apache/spark/sql/catalyst/CarbonDDLSqlParser.scala ---
@@ -302,13 +302,20 @@ abstract class CarbonDDLSqlParser extends AbstractCarbonSparkSQLParser {
if (!CarbonScalaUtil
.validateLocalDictionaryEnable(tableProperties(CarbonCommonConstants
--- End diff --
optimize the indent, move 'validateLocalDictionaryEnable' to previous line
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
GitHub user Sssan520 reopened a pull request:
https://github.com/apache/carbondata/pull/2491
[CARBONDATA-2698][CARBONDATA-2700][CARBONDATA-2730][CARBONDATA-2732][BloomDataMap] block some oprerations of bloomfilter datamap
1.Block create bloomfilter datamap index on column which its datatype is complex type;
2.Block create bloomfilter datamap index on local_dictionary column;
3.Block change datatype for bloomfilter index datamap;
4.Block dropping index columns for bloomfilter index datamap
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
add a parameter "targets" for method "canAllow" of "CarbonTable" class
- [ ] Any backward compatibility impacted? No
- [ ] Document update required? No
- [ ] Testing done NA
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 bloomfilter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2491.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 #2491
----
commit 3d6597f4c51faefa191a1c068fa1b1c8be4d9f67
Author: lianganping 00251374 <li...@...>
Date: 2018-07-11T12:17:09Z
1.Block create bloomfilter datamap index on column which its datatype is complex;2.Block create bloomfilter datamap index on local_dictionary column;3.Block change datatype for bloomfilter index datamap;4.Block dropping index columns for bloomfilter index datamap
----
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202000444
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableDataTypeChangeCommand.scala ---
@@ -55,7 +55,10 @@ private[sql] case class CarbonAlterTableDataTypeChangeCommand(
.validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
- if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_CHANGE_DATATYPE)) {
+ if (!carbonTable
--- End diff --
please optimize the indent
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202221610
--- Diff: core/src/main/java/org/apache/carbondata/core/datamap/dev/DataMapFactory.java ---
@@ -144,4 +144,17 @@ public void validate() throws MalformedDataMapCommandException {
}
}
+ /**
+ * to check something like columns by the operation
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202221798
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/schema/CarbonAlterTableDataTypeChangeCommand.scala ---
@@ -55,7 +55,10 @@ private[sql] case class CarbonAlterTableDataTypeChangeCommand(
.validateTableAndAcquireLock(dbName, tableName, locksToBeAcquired)(sparkSession)
val metastore = CarbonEnv.getInstance(sparkSession).carbonMetastore
carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(sparkSession)
- if (!carbonTable.canAllow(carbonTable, TableOperation.ALTER_CHANGE_DATATYPE)) {
+ if (!carbonTable
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201977605
--- Diff: datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapFactory.java ---
@@ -380,6 +380,49 @@ public void deleteDatamapData() {
return false;
}
+ @Override public boolean isOperationBlock(TableOperation operation, Object... targets) {
--- End diff --
1. move 'override' to the previous line
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2700][CARBONDATA-2730][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 closed the pull request at:
https://github.com/apache/carbondata/pull/2491
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202000664
--- Diff: integration/spark2/src/test/scala/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapSuite.scala ---
@@ -377,7 +382,91 @@ class BloomCoarseGrainDataMapSuite extends QueryTest with BeforeAndAfterAll with
checkQuery("fakeDm", shouldHit = false)
}
+ test("test blocl change datatype for bloomfilter index datamap") {
--- End diff --
block?
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2700][CARBONDATA-2730][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
GitHub user Sssan520 reopened a pull request:
https://github.com/apache/carbondata/pull/2491
[CARBONDATA-2700][CARBONDATA-2730][CARBONDATA-2732][BloomDataMap] block some oprerations of bloomfilter datamap
1.Block create bloomfilter datamap index on column which its datatype is complex type;
2.Block create bloomfilter datamap index on local_dictionary column;
3.Block change datatype for bloomfilter index datamap;
4.Block dropping index columns for bloomfilter index datamap
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
add a parameter "targets" for method "canAllow" of "CarbonTable" class
- [ ] Any backward compatibility impacted? No
- [ ] Document update required? No
- [ ] Testing done NA
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 bloomfilter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2491.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 #2491
----
commit 3d6597f4c51faefa191a1c068fa1b1c8be4d9f67
Author: lianganping 00251374 <li...@...>
Date: 2018-07-11T12:17:09Z
1.Block create bloomfilter datamap index on column which its datatype is complex;2.Block create bloomfilter datamap index on local_dictionary column;3.Block change datatype for bloomfilter index datamap;4.Block dropping index columns for bloomfilter index datamap
----
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202221482
--- Diff: datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapFactory.java ---
@@ -380,6 +380,49 @@ public void deleteDatamapData() {
return false;
}
+ @Override public boolean isOperationBlock(TableOperation operation, Object... targets) {
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201997642
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java ---
@@ -1071,6 +1072,10 @@ public boolean canAllow(CarbonTable carbonTable, TableOperation operation) {
if (factoryClass.willBecomeStale(operation)) {
return false;
}
+ //to check something like columns by the operation
--- End diff --
space after "//"
optimize the comment to "check whether the operation is blocked for datamap"
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201997452
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/CarbonTable.java ---
@@ -1058,9 +1058,10 @@ public void setTransactionalTable(boolean transactionalTable) {
* if this operation makes datamap stale it is not allowed
* @param carbonTable
* @param operation
+ * @param targets
--- End diff --
add description for it
---
[GitHub] carbondata issue #2491: [CARBONDATA-2700][BloomDataMap] block some opreratio...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on the issue:
https://github.com/apache/carbondata/pull/2491
retest this please
---
[GitHub] carbondata issue #2491: [CARBONDATA-2700][BloomDataMap] Block dropping index...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2491
Can one of the admins verify this patch?
---
[GitHub] carbondata issue #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA-2730][...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on the issue:
https://github.com/apache/carbondata/pull/2491
all code review comments has been handled.
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201978444
--- Diff: core/src/main/java/org/apache/carbondata/core/datamap/dev/DataMapFactory.java ---
@@ -144,4 +144,17 @@ public void validate() throws MalformedDataMapCommandException {
}
}
+ /**
+ * to check something like columns by the operation
--- End diff --
better to optimize the description to:
whether to block operation on corresponding table or column. For example, bloomfilter datamap will block changing datatype for bloomindex column. By default it will not block any operation.
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r202221774
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/datamap/CarbonCreateDataMapCommand.scala ---
@@ -109,6 +111,19 @@ case class CarbonCreateDataMapCommand(
throw new MalformedDataMapCommandException(String.format(
"column '%s' already has %s index datamap created",
column.getColName, thisDmProviderName))
+ } else if (isBloomFilter) {
+ // if datamap provider is bloomfilter,the index column datatype cannot be complex type
+ if (column.isComplex) {
+ throw new MalformedDataMapCommandException(String.format(
+ "since datamap provider is bloomfilter , the index column '%s' datatype cannot be" +
--- End diff --
ok
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by Sssan520 <gi...@git.apache.org>.
Github user Sssan520 closed the pull request at:
https://github.com/apache/carbondata/pull/2491
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201999539
--- Diff: datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapFactory.java ---
@@ -380,6 +380,49 @@ public void deleteDatamapData() {
return false;
}
+ @Override public boolean isOperationBlock(TableOperation operation, Object... targets) {
+ switch (operation) {
+ case ALTER_DROP: {
+ try {
+ //alter table drop columns
+ //will be blocked if the columns in bloomfilter datamap
+ List<String> columnsToDrop = (List<String>) targets[0];
+ List<String> indexedColumnNames = dataMapMeta.getIndexedColumnNames();
+ for (String indexedcolumn : indexedColumnNames) {
+ for (String column : columnsToDrop) {
+ if (column.equalsIgnoreCase(indexedcolumn)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ } catch (ClassCastException | NullPointerException ex) {
--- End diff --
better to ignore these exceptions and let it throw out if it really happens.
The same with the below switch branch
---
[GitHub] carbondata pull request #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2491#discussion_r201998306
--- Diff: datamap/bloom/src/main/java/org/apache/carbondata/datamap/bloom/BloomCoarseGrainDataMapFactory.java ---
@@ -380,6 +380,49 @@ public void deleteDatamapData() {
return false;
}
+ @Override public boolean isOperationBlock(TableOperation operation, Object... targets) {
+ switch (operation) {
+ case ALTER_DROP: {
+ try {
+ //alter table drop columns
+ //will be blocked if the columns in bloomfilter datamap
--- End diff --
This comment is not needed.
The same with below switch branch
---
[GitHub] carbondata issue #2491: [CARBONDATA-2698][CARBONDATA-2700][CARBONDATA-2730][...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on the issue:
https://github.com/apache/carbondata/pull/2491
please check and fix the comments
---