You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by ksimar <gi...@git.apache.org> on 2017/09/04 12:52:13 UTC
[GitHub] carbondata pull request #1320: [CARBONDATA-1446] Fixed Bug for error message...
GitHub user ksimar opened a pull request:
https://github.com/apache/carbondata/pull/1320
[CARBONDATA-1446] Fixed Bug for error message on invalid partition id in alter partition command
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ksimar/incubator-carbondata bugFix/CARBONDATA-1446
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/1320.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 #1320
----
commit ae06ccd8f7f9488d7e4cf46f870f081bd59cfbef
Author: ksimar <si...@gmail.com>
Date: 2017-09-04T12:43:32Z
Fixed Bug on invalid error message in case of wrong partition id in alter split partition.
----
---
[GitHub] carbondata pull request #1320: [CARBONDATA-1446] Fixed Bug for error message...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1320#discussion_r137172035
--- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/PartitionUtils.scala ---
@@ -80,37 +80,42 @@ object PartitionUtils {
dateFormatter: SimpleDateFormat): Unit = {
val columnDataType = partitionInfo.getColumnSchemaList.get(0).getDataType
val index = partitionIdList.indexOf(partitionId)
- if (partitionInfo.getPartitionType == PartitionType.RANGE) {
- val rangeInfo = partitionInfo.getRangeInfo.asScala.toList
- val newRangeInfo = partitionId match {
- case 0 => rangeInfo ++ splitInfo
- case _ => rangeInfo.take(index - 1) ++ splitInfo ++
- rangeInfo.takeRight(rangeInfo.size - index)
+ if (index >= 0) {
+ if (partitionInfo.getPartitionType == PartitionType.RANGE) {
+ val rangeInfo = partitionInfo.getRangeInfo.asScala.toList
+ val newRangeInfo = partitionId match {
+ case 0 => rangeInfo ++ splitInfo
+ case _ => rangeInfo.take(index - 1) ++ splitInfo ++
+ rangeInfo.takeRight(rangeInfo.size - index)
+ }
+ CommonUtil.validateRangeInfo(newRangeInfo, columnDataType,
+ timestampFormatter, dateFormatter)
+ partitionInfo.setRangeInfo(newRangeInfo.asJava)
+ } else if (partitionInfo.getPartitionType == PartitionType.LIST) {
+ val originList = partitionInfo.getListInfo.asScala.map(_.asScala.toList).toList
+ if (partitionId != 0) {
+ val targetListInfo = partitionInfo.getListInfo.get(index - 1)
+ CommonUtil.validateSplitListInfo(targetListInfo.asScala.toList, splitInfo, originList)
+ } else {
+ CommonUtil.validateAddListInfo(splitInfo, originList)
+ }
+ val addListInfo = PartitionUtils.getListInfo(splitInfo.mkString(","))
+ val newListInfo = partitionId match {
+ case 0 => originList ++ addListInfo
+ case _ => originList.take(index - 1) ++ addListInfo ++
+ originList.takeRight(originList.size - index)
+ }
+ partitionInfo.setListInfo(newListInfo.map(_.asJava).asJava)
}
- CommonUtil.validateRangeInfo(newRangeInfo, columnDataType,
- timestampFormatter, dateFormatter)
- partitionInfo.setRangeInfo(newRangeInfo.asJava)
- } else if (partitionInfo.getPartitionType == PartitionType.LIST) {
- val originList = partitionInfo.getListInfo.asScala.map(_.asScala.toList).toList
- if (partitionId != 0) {
- val targetListInfo = partitionInfo.getListInfo.get(index - 1)
- CommonUtil.validateSplitListInfo(targetListInfo.asScala.toList, splitInfo, originList)
+
+ if (partitionId == 0) {
+ partitionInfo.addPartition(splitInfo.size)
} else {
- CommonUtil.validateAddListInfo(splitInfo, originList)
- }
- val addListInfo = PartitionUtils.getListInfo(splitInfo.mkString(","))
- val newListInfo = partitionId match {
- case 0 => originList ++ addListInfo
- case _ => originList.take(index - 1) ++ addListInfo ++
- originList.takeRight(originList.size - index)
+ partitionInfo.splitPartition(index, splitInfo.size)
}
- partitionInfo.setListInfo(newListInfo.map(_.asJava).asJava)
}
-
- if (partitionId == 0) {
- partitionInfo.addPartition(splitInfo.size)
- } else {
- partitionInfo.splitPartition(index, splitInfo.size)
+ else {
+ throw new IllegalArgumentException("Invalid Partition Id")
--- End diff --
Suggest to check `if (index < 0)` and throw exception instead of checking `>=0`
The error message should contain the index value
---
[GitHub] carbondata pull request #1320: [CARBONDATA-1446] Fixed Bug for error message...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1320#discussion_r137171876
--- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/PartitionUtils.scala ---
@@ -80,37 +80,42 @@ object PartitionUtils {
dateFormatter: SimpleDateFormat): Unit = {
val columnDataType = partitionInfo.getColumnSchemaList.get(0).getDataType
val index = partitionIdList.indexOf(partitionId)
- if (partitionInfo.getPartitionType == PartitionType.RANGE) {
- val rangeInfo = partitionInfo.getRangeInfo.asScala.toList
- val newRangeInfo = partitionId match {
- case 0 => rangeInfo ++ splitInfo
- case _ => rangeInfo.take(index - 1) ++ splitInfo ++
- rangeInfo.takeRight(rangeInfo.size - index)
+ if (index >= 0) {
+ if (partitionInfo.getPartitionType == PartitionType.RANGE) {
+ val rangeInfo = partitionInfo.getRangeInfo.asScala.toList
+ val newRangeInfo = partitionId match {
+ case 0 => rangeInfo ++ splitInfo
+ case _ => rangeInfo.take(index - 1) ++ splitInfo ++
+ rangeInfo.takeRight(rangeInfo.size - index)
+ }
+ CommonUtil.validateRangeInfo(newRangeInfo, columnDataType,
+ timestampFormatter, dateFormatter)
+ partitionInfo.setRangeInfo(newRangeInfo.asJava)
+ } else if (partitionInfo.getPartitionType == PartitionType.LIST) {
+ val originList = partitionInfo.getListInfo.asScala.map(_.asScala.toList).toList
+ if (partitionId != 0) {
+ val targetListInfo = partitionInfo.getListInfo.get(index - 1)
+ CommonUtil.validateSplitListInfo(targetListInfo.asScala.toList, splitInfo, originList)
+ } else {
+ CommonUtil.validateAddListInfo(splitInfo, originList)
+ }
+ val addListInfo = PartitionUtils.getListInfo(splitInfo.mkString(","))
+ val newListInfo = partitionId match {
+ case 0 => originList ++ addListInfo
+ case _ => originList.take(index - 1) ++ addListInfo ++
+ originList.takeRight(originList.size - index)
+ }
+ partitionInfo.setListInfo(newListInfo.map(_.asJava).asJava)
}
- CommonUtil.validateRangeInfo(newRangeInfo, columnDataType,
- timestampFormatter, dateFormatter)
- partitionInfo.setRangeInfo(newRangeInfo.asJava)
- } else if (partitionInfo.getPartitionType == PartitionType.LIST) {
- val originList = partitionInfo.getListInfo.asScala.map(_.asScala.toList).toList
- if (partitionId != 0) {
- val targetListInfo = partitionInfo.getListInfo.get(index - 1)
- CommonUtil.validateSplitListInfo(targetListInfo.asScala.toList, splitInfo, originList)
+
+ if (partitionId == 0) {
+ partitionInfo.addPartition(splitInfo.size)
} else {
- CommonUtil.validateAddListInfo(splitInfo, originList)
- }
- val addListInfo = PartitionUtils.getListInfo(splitInfo.mkString(","))
- val newListInfo = partitionId match {
- case 0 => originList ++ addListInfo
- case _ => originList.take(index - 1) ++ addListInfo ++
- originList.takeRight(originList.size - index)
+ partitionInfo.splitPartition(index, splitInfo.size)
}
- partitionInfo.setListInfo(newListInfo.map(_.asJava).asJava)
}
-
- if (partitionId == 0) {
- partitionInfo.addPartition(splitInfo.size)
- } else {
- partitionInfo.splitPartition(index, splitInfo.size)
+ else {
--- End diff --
move to previous line
---
[GitHub] carbondata pull request #1320: [CARBONDATA-1446] Fixed Bug for error message...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1320#discussion_r137279394
--- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/PartitionUtils.scala ---
@@ -80,37 +80,43 @@ object PartitionUtils {
dateFormatter: SimpleDateFormat): Unit = {
val columnDataType = partitionInfo.getColumnSchemaList.get(0).getDataType
val index = partitionIdList.indexOf(partitionId)
- if (partitionInfo.getPartitionType == PartitionType.RANGE) {
- val rangeInfo = partitionInfo.getRangeInfo.asScala.toList
- val newRangeInfo = partitionId match {
- case 0 => rangeInfo ++ splitInfo
- case _ => rangeInfo.take(index - 1) ++ splitInfo ++
- rangeInfo.takeRight(rangeInfo.size - index)
+ if (index < 0) {
+ throw new IllegalArgumentException("Invalid Partition Id " + partitionId +
+ "\n Use show partitions table_name to get the list of valid partitions")
+ }
+ else {
--- End diff --
else clause is not needed
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by ksimar <gi...@git.apache.org>.
Github user ksimar commented on the issue:
https://github.com/apache/carbondata/pull/1320
retest this please
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1320
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/699/
---
[GitHub] carbondata pull request #1320: [CARBONDATA-1446] Fixed Bug for error message...
Posted by ksimar <gi...@git.apache.org>.
Github user ksimar commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1320#discussion_r137186127
--- Diff: integration/spark-common/src/main/scala/org/apache/spark/util/PartitionUtils.scala ---
@@ -80,37 +80,42 @@ object PartitionUtils {
dateFormatter: SimpleDateFormat): Unit = {
val columnDataType = partitionInfo.getColumnSchemaList.get(0).getDataType
val index = partitionIdList.indexOf(partitionId)
- if (partitionInfo.getPartitionType == PartitionType.RANGE) {
- val rangeInfo = partitionInfo.getRangeInfo.asScala.toList
- val newRangeInfo = partitionId match {
- case 0 => rangeInfo ++ splitInfo
- case _ => rangeInfo.take(index - 1) ++ splitInfo ++
- rangeInfo.takeRight(rangeInfo.size - index)
+ if (index >= 0) {
+ if (partitionInfo.getPartitionType == PartitionType.RANGE) {
+ val rangeInfo = partitionInfo.getRangeInfo.asScala.toList
+ val newRangeInfo = partitionId match {
+ case 0 => rangeInfo ++ splitInfo
+ case _ => rangeInfo.take(index - 1) ++ splitInfo ++
+ rangeInfo.takeRight(rangeInfo.size - index)
+ }
+ CommonUtil.validateRangeInfo(newRangeInfo, columnDataType,
+ timestampFormatter, dateFormatter)
+ partitionInfo.setRangeInfo(newRangeInfo.asJava)
+ } else if (partitionInfo.getPartitionType == PartitionType.LIST) {
+ val originList = partitionInfo.getListInfo.asScala.map(_.asScala.toList).toList
+ if (partitionId != 0) {
+ val targetListInfo = partitionInfo.getListInfo.get(index - 1)
+ CommonUtil.validateSplitListInfo(targetListInfo.asScala.toList, splitInfo, originList)
+ } else {
+ CommonUtil.validateAddListInfo(splitInfo, originList)
+ }
+ val addListInfo = PartitionUtils.getListInfo(splitInfo.mkString(","))
+ val newListInfo = partitionId match {
+ case 0 => originList ++ addListInfo
+ case _ => originList.take(index - 1) ++ addListInfo ++
+ originList.takeRight(originList.size - index)
+ }
+ partitionInfo.setListInfo(newListInfo.map(_.asJava).asJava)
}
- CommonUtil.validateRangeInfo(newRangeInfo, columnDataType,
- timestampFormatter, dateFormatter)
- partitionInfo.setRangeInfo(newRangeInfo.asJava)
- } else if (partitionInfo.getPartitionType == PartitionType.LIST) {
- val originList = partitionInfo.getListInfo.asScala.map(_.asScala.toList).toList
- if (partitionId != 0) {
- val targetListInfo = partitionInfo.getListInfo.get(index - 1)
- CommonUtil.validateSplitListInfo(targetListInfo.asScala.toList, splitInfo, originList)
+
+ if (partitionId == 0) {
+ partitionInfo.addPartition(splitInfo.size)
} else {
- CommonUtil.validateAddListInfo(splitInfo, originList)
- }
- val addListInfo = PartitionUtils.getListInfo(splitInfo.mkString(","))
- val newListInfo = partitionId match {
- case 0 => originList ++ addListInfo
- case _ => originList.take(index - 1) ++ addListInfo ++
- originList.takeRight(originList.size - index)
+ partitionInfo.splitPartition(index, splitInfo.size)
}
- partitionInfo.setListInfo(newListInfo.map(_.asJava).asJava)
}
-
- if (partitionId == 0) {
- partitionInfo.addPartition(splitInfo.size)
- } else {
- partitionInfo.splitPartition(index, splitInfo.size)
+ else {
+ throw new IllegalArgumentException("Invalid Partition Id")
--- End diff --
@jackylk please review
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1320
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/516/
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1320
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/573/
---
[GitHub] carbondata pull request #1320: [CARBONDATA-1446] Fixed Bug for error message...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/1320
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1320
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/661/
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1320
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/607/
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1320
LGTM
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1320
@ksimar Can you check the SDV test failure?
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1320
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/554/
---
[GitHub] carbondata issue #1320: [CARBONDATA-1446] Fixed Bug for error message on inv...
Posted by chenliang613 <gi...@git.apache.org>.
Github user chenliang613 commented on the issue:
https://github.com/apache/carbondata/pull/1320
retest this please
---