You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@carbondata.apache.org by GitBox <gi...@apache.org> on 2021/05/18 10:44:17 UTC
[GitHub] [carbondata] maheshrajus opened a new pull request #4138: [CARBONDATA-4189] alter table validation issues
maheshrajus opened a new pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138
### Why is this PR needed?
1) Alter table duplicate columns check for dimensions/complex columns missed
2) Alter table properties with long strings for complex columns should not support
### What changes were proposed in this PR?
1) Changed the dimension columns list type in preparing dimensions columns [LinkedHashSet to Scala Seq] for handling the duplicate columns
2) Added check for throwing an exception in case of long strings for complex columns
### Does this PR introduce any user interface change?
- No
### Is any new testcase added?
- Yes
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-845461134
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3670/
--
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-846743269
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-845062440
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5409/
--
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r636377233
##########
File path: integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
##########
@@ -911,8 +911,12 @@ object AlterTableUtil {
" does not exist in table. Please check the DDL."
throw new MalformedCarbonCommandException(errorMsg)
} else if (colSchemas.exists(x => x.getColumnName.equalsIgnoreCase(col) &&
- !x.getDataType.toString
- .equalsIgnoreCase("STRING"))) {
+ x.isComplexColumn)) {
+ val errMsg = "Complex child column " + col + " cannot be set as LONG_STRING_COLUMNS"
Review comment:
```suggestion
val errMsg = s"Complex child column $col cannot be set as LONG_STRING_COLUMNS"
```
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-845462855
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5414/
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-844319821
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3658/
--
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-845732246
LGTM
--
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r636299047
##########
File path: integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
##########
@@ -916,6 +916,10 @@ object AlterTableUtil {
val errMsg = "LONG_STRING_COLUMNS column: " + col +
" is not a string datatype column"
throw new MalformedCarbonCommandException(errMsg)
+ } else if (colSchemas.exists(x => x.getColumnName.equalsIgnoreCase(col) &&
Review comment:
This check can be moved before previous else case and error message could be,
Complex child column $col cannot be set as LONG_STRING_COLUMNS
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-845065338
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3665/
--
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:
users@infra.apache.org
[GitHub] [carbondata] akkio-97 commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
akkio-97 commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r635816643
##########
File path: integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala
##########
@@ -812,6 +812,40 @@ test("test alter command for boolean data type with correct default measure valu
sql("DROP TABLE t1")
}
+ test("testing the duplicate add columns for complex data types") {
+ sql("drop table if exists alter_complex")
+ sql("create table alter_complex (a int, b string, arr1 array<int>) " +
+ "stored as carbondata")
+ intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(arr1 array<int>)")
+ }
Review comment:
please mention which error message is to be expected
--
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:
users@infra.apache.org
[GitHub] [carbondata] maheshrajus commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
maheshrajus commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-843383009
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-843460855
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5394/
--
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:
users@infra.apache.org
[GitHub] [carbondata] akkio-97 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
akkio-97 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-845069978
LGTM
--
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:
users@infra.apache.org
[GitHub] [carbondata] maheshrajus commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
maheshrajus commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r636328822
##########
File path: integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
##########
@@ -916,6 +916,10 @@ object AlterTableUtil {
val errMsg = "LONG_STRING_COLUMNS column: " + col +
" is not a string datatype column"
throw new MalformedCarbonCommandException(errMsg)
+ } else if (colSchemas.exists(x => x.getColumnName.equalsIgnoreCase(col) &&
Review comment:
ok,done
--
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r636304620
##########
File path: integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala
##########
@@ -812,6 +812,56 @@ test("test alter command for boolean data type with correct default measure valu
sql("DROP TABLE t1")
}
+ test("testing the duplicate add columns for complex data types") {
+ sql("drop table if exists alter_complex")
+ sql("create table alter_complex (a int, b string, arr1 array<int>) " +
+ "stored as carbondata")
+ var exception = intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(arr1 array<int>)")
+ }
+ assert(exception.getMessage
Review comment:
This assertion looks duplicate for below code. Can refactor the code, by creating variable errMsg and can directly check intercept[] {}.getmessage.contains(errMsg)
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-843124603
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3647/
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-846803869
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3672/
--
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:
users@infra.apache.org
[GitHub] [carbondata] maheshrajus commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
maheshrajus commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r636392970
##########
File path: integration/spark/src/main/scala/org/apache/spark/util/AlterTableUtil.scala
##########
@@ -911,8 +911,12 @@ object AlterTableUtil {
" does not exist in table. Please check the DDL."
throw new MalformedCarbonCommandException(errorMsg)
} else if (colSchemas.exists(x => x.getColumnName.equalsIgnoreCase(col) &&
- !x.getDataType.toString
- .equalsIgnoreCase("STRING"))) {
+ x.isComplexColumn)) {
+ val errMsg = "Complex child column " + col + " cannot be set as LONG_STRING_COLUMNS"
Review comment:
done
--
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:
users@infra.apache.org
[GitHub] [carbondata] asfgit closed pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138
--
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:
users@infra.apache.org
[GitHub] [carbondata] maheshrajus commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
maheshrajus commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r635993401
##########
File path: integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala
##########
@@ -812,6 +812,40 @@ test("test alter command for boolean data type with correct default measure valu
sql("DROP TABLE t1")
}
+ test("testing the duplicate add columns for complex data types") {
+ sql("drop table if exists alter_complex")
+ sql("create table alter_complex (a int, b string, arr1 array<int>) " +
+ "stored as carbondata")
+ intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(arr1 array<int>)")
+ }
Review comment:
OK, done
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-844304090
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5402/
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-846803389
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5415/
--
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:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#discussion_r636305477
##########
File path: integration/spark/src/test/scala/org/apache/spark/carbondata/restructure/AlterTableValidationTestCase.scala
##########
@@ -812,6 +812,56 @@ test("test alter command for boolean data type with correct default measure valu
sql("DROP TABLE t1")
}
+ test("testing the duplicate add columns for complex data types") {
+ sql("drop table if exists alter_complex")
+ sql("create table alter_complex (a int, b string, arr1 array<int>) " +
+ "stored as carbondata")
+ var exception = intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(arr1 array<int>)")
+ }
+ assert(exception.getMessage
+ .contains("Alter table add operation failed: Duplicate column found with name"))
+ exception = intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(arr2 array<int>, arr2 array<int>)")
+ }
+ assert(exception.getMessage
+ .contains("Alter table add operation failed: Duplicate column found with name"))
+ exception = intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(c int, c int)")
+ }
+ assert(exception.getMessage
+ .contains("Alter table add operation failed: Duplicate column found with name"))
+ exception = intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(c int, c string)")
+ }
+ assert(exception.getMessage
+ .contains("Alter table add operation failed: Duplicate column found with name"))
+ exception = intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(c string, c int)")
+ }
+ assert(exception.getMessage
+ .contains("Alter table add operation failed: Duplicate column found with name"))
+ exception = intercept[ProcessMetaDataException] {
+ sql("alter table alter_complex add columns(c string, c array<string>)")
+ }
+ assert(exception.getMessage
+ .contains("Alter table add operation failed: Duplicate column found with name"))
+ }
+
+ test("testing the long string properties for complex columns") {
+ sql("drop table if exists alter_complex")
+ sql("create table alter_complex (a int, arr1 array<string>) " +
+ "stored as carbondata")
+ val exception = intercept[RuntimeException] {
+ sql("alter table alter_complex SET TBLPROPERTIES" +
+ "('LONG_STRING_COLUMNS'='arr1.val')")
+ }
+ assert(exception.getMessage
Review comment:
Here also, can directly check intercept[] {}.getmessage.contains(errMsg)
--
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-843459470
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12602/job/ApacheCarbon_PR_Builder_2.4.5/3649/
--
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:
users@infra.apache.org
[GitHub] [carbondata] maheshrajus commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
maheshrajus commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-844222134
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:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4138: [CARBONDATA-4189] alter table validation issues
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4138:
URL: https://github.com/apache/carbondata/pull/4138#issuecomment-843118759
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12602/job/ApacheCarbonPRBuilder2.3/5392/
--
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:
users@infra.apache.org