You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by shardul-cr7 <gi...@git.apache.org> on 2019/01/02 09:23:16 UTC
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
GitHub user shardul-cr7 opened a pull request:
https://github.com/apache/carbondata/pull/3045
[CARBONDATA-3222]Fix dataload failure after creation of preaggregate datamap on main table with long_string_columns
This PR is to Fix dataload failure after creation of preaggregate datamap on main table with long_string_columns.
Dataload is gettling failed because child table properties are not getting modified according to the parent table for long_string_columns.
This occurs only when long_string_columns is not specified in dmproperties for preaggregate datamap but the datamap was getting created and data load was failing. This PR is to avoid the dataload failure in this scenario.
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
- [ ] Any backward compatibility impacted?
- [ ] Document update required?
- [x] Testing done
added a testcase
- [ ] 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/shardul-cr7/carbondata lsc_preagg
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/3045.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 #3045
----
commit ed588b32ff95a451782c98cae991efd6d148b5c3
Author: shardul-cr7 <sh...@...>
Date: 2019-01-02T09:17:34Z
[CARBONDATA-3222]Fix dataload failure after creation of preaggregate datamap on main table with long_string_columns
----
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r244707751
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -126,6 +126,12 @@ case class PreAggregateTableHelper(
newLongStringColumn.mkString(","))
}
+ //Add long_string_columns properties in child table from the parent.
+ tableProperties
--- End diff --
@shardul-cr7 Here you are copying all the long String column...in case of pre aggregate data map may be it will not have all the columns of maintable, its better to add long string properties only for those column which are present in case of pre aggregate datamap......
example: maintable long string columns: column1, column2, column3
datamap1: column1....set long string property for column1
datamap2:count(column1) no need to set if any udf, udaf is present then no need to set this property
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245274147
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
--- End diff --
add if(fields._2.aggregateFunction.isEmpty && fields._2.columnTableRelationList.size==1 && longStringColumnInParents.contains(fields._2.columnTableRelationList.head.parentColumnName)) {
}
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2114/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2405/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on the issue:
https://github.com/apache/carbondata/pull/3045
LGTM
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2387/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2144/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10364/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2320/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10431/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2176/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by kunal642 <gi...@git.apache.org>.
Github user kunal642 commented on the issue:
https://github.com/apache/carbondata/pull/3045
LGTM
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10398/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2189/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245274310
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
--- End diff --
use seq instead of string
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10428/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245511008
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,7 +110,29 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dm properties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
--- End diff --
So user do not have to config dmproperties for long string anymore and this line is no use after this PR?
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10394/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245475409
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
+ val relationList = (fields._2.columnTableRelationList)
+ relationList.foreach(rel => {
+ rel.foreach(col => {
+ if (longStringColumnInParents.contains(col.parentColumnName)) {
+ varcharDatamapFields += col.parentColumnName + ","
+ }
+ })
+ })
+ }
+ })
+ if (varcharDatamapFields.size != 0) {
--- End diff --
Done!
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10369/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245272931
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
--- End diff --
Please use isEmpty
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10445/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245275753
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
+ val relationList = (fields._2.columnTableRelationList)
+ relationList.foreach(rel => {
+ rel.foreach(col => {
+ if (longStringColumnInParents.contains(col.parentColumnName)) {
+ varcharDatamapFields += col.parentColumnName + ","
+ }
+ })
+ })
+ }
+ })
+ if (varcharDatamapFields.size != 0) {
+ longStringColumn = Option(varcharDatamapFields.slice(0, varcharDatamapFields.length - 1))
+ }
if (longStringColumn != None) {
val fieldNames = fields.map(_.column)
- val newLongStringColumn = longStringColumn.get.split(",").map(_.trim).map{ colName =>
+ val newLongStringColumn = longStringColumn.get.split(",").map(_.trim).map { colName =>
val newColName = parentTable.getTableName.toLowerCase() + "_" + colName
if (!fieldNames.contains(newColName)) {
throw new MalformedDataMapCommandException(
CarbonCommonConstants.LONG_STRING_COLUMNS.toUpperCase() + ":" + colName
- + " does not in datamap")
+ + " does not in datamap")
}
newColName
}
tableProperties.put(CarbonCommonConstants.LONG_STRING_COLUMNS,
newLongStringColumn.mkString(","))
}
-
--- End diff --
revert this change
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245475435
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
+ val relationList = (fields._2.columnTableRelationList)
+ relationList.foreach(rel => {
+ rel.foreach(col => {
+ if (longStringColumnInParents.contains(col.parentColumnName)) {
+ varcharDatamapFields += col.parentColumnName + ","
+ }
+ })
+ })
+ }
+ })
+ if (varcharDatamapFields.size != 0) {
+ longStringColumn = Option(varcharDatamapFields.slice(0, varcharDatamapFields.length - 1))
+ }
if (longStringColumn != None) {
val fieldNames = fields.map(_.column)
- val newLongStringColumn = longStringColumn.get.split(",").map(_.trim).map{ colName =>
+ val newLongStringColumn = longStringColumn.get.split(",").map(_.trim).map { colName =>
val newColName = parentTable.getTableName.toLowerCase() + "_" + colName
if (!fieldNames.contains(newColName)) {
throw new MalformedDataMapCommandException(
CarbonCommonConstants.LONG_STRING_COLUMNS.toUpperCase() + ":" + colName
- + " does not in datamap")
+ + " does not in datamap")
--- End diff --
Done!
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10432/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kevinjmh <gi...@git.apache.org>.
Github user kevinjmh commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245511058
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,7 +110,29 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dm properties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
--- End diff --
The codes is not consistent with comments in line 113. Please fix it
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245585973
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
--- End diff --
Done!
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245475427
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
--- End diff --
Done!
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2317/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245275437
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
+ val relationList = (fields._2.columnTableRelationList)
+ relationList.foreach(rel => {
+ rel.foreach(col => {
+ if (longStringColumnInParents.contains(col.parentColumnName)) {
+ varcharDatamapFields += col.parentColumnName + ","
+ }
+ })
+ })
+ }
+ })
+ if (varcharDatamapFields.size != 0) {
--- End diff --
remove this code, covert comma separated string from seq
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by kumarvishal09 <gi...@git.apache.org>.
Github user kumarvishal09 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245273039
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,22 +109,42 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dmproperties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ val longStringColumnInParents = parentTable.getTableInfo.getFactTable.getTableProperties.asScala
+ .getOrElse(CarbonCommonConstants.LONG_STRING_COLUMNS, "").split(",").map(_.trim)
+ var varcharDatamapFields = ""
+ fieldRelationMap foreach (fields => {
+ val aggFunc = fields._2.aggregateFunction
+ if (aggFunc == "") {
+ val relationList = (fields._2.columnTableRelationList)
+ relationList.foreach(rel => {
+ rel.foreach(col => {
+ if (longStringColumnInParents.contains(col.parentColumnName)) {
+ varcharDatamapFields += col.parentColumnName + ","
+ }
+ })
+ })
+ }
+ })
+ if (varcharDatamapFields.size != 0) {
+ longStringColumn = Option(varcharDatamapFields.slice(0, varcharDatamapFields.length - 1))
+ }
if (longStringColumn != None) {
val fieldNames = fields.map(_.column)
- val newLongStringColumn = longStringColumn.get.split(",").map(_.trim).map{ colName =>
+ val newLongStringColumn = longStringColumn.get.split(",").map(_.trim).map { colName =>
val newColName = parentTable.getTableName.toLowerCase() + "_" + colName
if (!fieldNames.contains(newColName)) {
throw new MalformedDataMapCommandException(
CarbonCommonConstants.LONG_STRING_COLUMNS.toUpperCase() + ":" + colName
- + " does not in datamap")
+ + " does not in datamap")
--- End diff --
revery this change
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2115/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2112/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2175/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2140/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245516216
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,7 +110,29 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
--- End diff --
This PR is for the scenario when user doesn't config the long_string_columns in dmprop then we take long_string_cols from the parent table.
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on the issue:
https://github.com/apache/carbondata/pull/3045
retest this please
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2391/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r244992035
--- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/longstring/VarcharDataTypesBasicTestCase.scala ---
@@ -333,6 +333,36 @@ class VarcharDataTypesBasicTestCase extends QueryTest with BeforeAndAfterEach wi
sql(s"DROP DATAMAP IF EXISTS $datamapName ON TABLE $longStringTable")
}
+ test("creating datamap with long string column selected and loading data should be success") {
+
+ sql(s"drop table if exists $longStringTable")
+ val datamapName = "pre_agg_dm"
+ sql(
+ s"""
+ | CREATE TABLE if not exists $longStringTable(
+ | id INT, name STRING, description STRING, address STRING, note STRING
+ | ) STORED BY 'carbondata'
+ | TBLPROPERTIES('LONG_STRING_COLUMNS'='description, note', 'SORT_COLUMNS'='name')
+ |""".stripMargin)
+
+ sql(
+ s"""
+ | CREATE DATAMAP $datamapName ON TABLE $longStringTable
+ | USING 'preaggregate'
+ | AS SELECT id,description,note,count(*) FROM $longStringTable
+ | GROUP BY id,description,note
+ |""".
+ stripMargin)
+
+ sql(
+ s"""
+ | LOAD DATA LOCAL INPATH '$inputFile' INTO TABLE $longStringTable
+ | OPTIONS('header'='false')
+ """.stripMargin)
+
+ sql(s"drop table if exists $longStringTable")
--- End diff --
Added!
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2110/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245534858
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,7 +110,29 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dm properties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
--- End diff --
fine
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2346/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10366/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2352/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2410/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by xuchuanyin <gi...@git.apache.org>.
Github user xuchuanyin commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245509909
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,7 +110,29 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
--- End diff --
emm, actually this line of code was added to solve the same problem mentioned in the PR description. Previously we thought it's user's responsibility to explicitly specify the long_string_columns in the preagg datamap.
Please @kevinjmh also check this.
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2172/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/3045
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by qiuchenjian <gi...@git.apache.org>.
Github user qiuchenjian commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r244706639
--- Diff: integration/spark-common-test/src/test/scala/org/apache/carbondata/spark/testsuite/longstring/VarcharDataTypesBasicTestCase.scala ---
@@ -333,6 +333,36 @@ class VarcharDataTypesBasicTestCase extends QueryTest with BeforeAndAfterEach wi
sql(s"DROP DATAMAP IF EXISTS $datamapName ON TABLE $longStringTable")
}
+ test("creating datamap with long string column selected and loading data should be success") {
+
+ sql(s"drop table if exists $longStringTable")
+ val datamapName = "pre_agg_dm"
+ sql(
+ s"""
+ | CREATE TABLE if not exists $longStringTable(
+ | id INT, name STRING, description STRING, address STRING, note STRING
+ | ) STORED BY 'carbondata'
+ | TBLPROPERTIES('LONG_STRING_COLUMNS'='description, note', 'SORT_COLUMNS'='name')
+ |""".stripMargin)
+
+ sql(
+ s"""
+ | CREATE DATAMAP $datamapName ON TABLE $longStringTable
+ | USING 'preaggregate'
+ | AS SELECT id,description,note,count(*) FROM $longStringTable
+ | GROUP BY id,description,note
+ |""".
+ stripMargin)
+
+ sql(
+ s"""
+ | LOAD DATA LOCAL INPATH '$inputFile' INTO TABLE $longStringTable
+ | OPTIONS('header'='false')
+ """.stripMargin)
+
+ sql(s"drop table if exists $longStringTable")
--- End diff --
Better to add a assert for this test case
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/2390/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r245516158
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -110,7 +110,29 @@ case class PreAggregateTableHelper(
// Datamap table name and columns are automatically added prefix with parent table name
// in carbon. For convenient, users can type column names same as the ones in select statement
// when config dmproperties, and here we update column names with prefix.
- val longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
+ // If longStringColumn is not present in dm properties then we take long_string_columns from
+ // the parent table.
+ var longStringColumn = tableProperties.get(CarbonCommonConstants.LONG_STRING_COLUMNS)
--- End diff --
Fixed it. If the user doesn't configure long_string in dmproperties, we take long_string_cols from the parent table.
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/2194/
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Success with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10450/
---
[GitHub] carbondata pull request #3045: [CARBONDATA-3222]Fix dataload failure after c...
Posted by shardul-cr7 <gi...@git.apache.org>.
Github user shardul-cr7 commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/3045#discussion_r244992105
--- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/execution/command/preaaggregate/PreAggregateTableHelper.scala ---
@@ -126,6 +126,12 @@ case class PreAggregateTableHelper(
newLongStringColumn.mkString(","))
}
+ //Add long_string_columns properties in child table from the parent.
+ tableProperties
--- End diff --
Done. @kumarvishal09 please review.
---
[GitHub] carbondata issue #3045: [CARBONDATA-3222]Fix dataload failure after creation...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/3045
Build Failed with Spark 2.3.2, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/10368/
---