[GitHub] incubator-carbondata pull request #597: [CARBONDATA-672]complex data type su...

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata pull request #597: [CARBONDATA-672]complex data type su...

qiuchenjian-2
GitHub user rahulforallp opened a pull request:

    https://github.com/apache/incubator-carbondata/pull/597

    [CARBONDATA-672]complex data type supported

    ClassCastException handled with logger .
    Example added for CarbonSessionExample.scala in example/spark2.
    Also resolved ClassCastException for ComplexTypeExample in example/spark

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rahulforallp/incubator-carbondata CARBONDATA-672

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-carbondata/pull/597.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 #597
   
----
commit 6ac0cb00fbe7392056b6cc2a292ee138facb9671
Author: rahulforallp <[hidden email]>
Date:   2017-02-09T07:10:21Z

    complex data type supported with spark 2.1

commit 84735b338e1bea1cab4088aa8504f304179ef0e2
Author: rahulforallp <[hidden email]>
Date:   2017-02-09T07:51:03Z

    complex datat type supported with spark 1.6

commit 2ab9c1ce8dd279398a5a2ed3c52f1763dc0c86ad
Author: rahulforallp <[hidden email]>
Date:   2017-02-09T08:02:14Z

    complex datatype supported

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/883/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/885/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/886/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/887/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/888/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata pull request #597: [CARBONDATA-672]complex data type su...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/597#discussion_r100721184
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonDictionaryDecoder.scala ---
    @@ -175,16 +175,25 @@ case class CarbonDictionaryDecoder(
                 val unsafeProjection = UnsafeProjection.create(output.map(_.dataType).toArray)
                 var flag = true
                 var total = 0L
    +
                 override final def hasNext: Boolean = iter.hasNext
    +
                 override final def next(): InternalRow = {
                   val startTime = System.currentTimeMillis()
                   val row: InternalRow = iter.next()
                   val data = row.toSeq(dataTypes).toArray
                   dictIndex.foreach { index =>
                     if (data(index) != null) {
    -                  data(index) = DataTypeUtil.getDataBasedOnDataType(dicts(index)
    -                    .getDictionaryValueForKey(data(index).asInstanceOf[Int]),
    -                    getDictionaryColumnIds(index)._3)
    +                  try {
    +                    data(index) = DataTypeUtil.getDataBasedOnDataType(dicts(index)
    +                      .getDictionaryValueForKey(data(index).asInstanceOf[Int]),
    +                      getDictionaryColumnIds(index)._3)
    +                  }
    +                  catch {
    +                    case x: ClassCastException => {
    --- End diff --
   
    How just exception logging could solve the issue? It can create some other issue, please try to avoid classcast exception first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata pull request #597: [CARBONDATA-672]complex data type su...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/597#discussion_r101241491
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonDictionaryDecoder.scala ---
    @@ -175,16 +175,25 @@ case class CarbonDictionaryDecoder(
                 val unsafeProjection = UnsafeProjection.create(output.map(_.dataType).toArray)
                 var flag = true
                 var total = 0L
    +
                 override final def hasNext: Boolean = iter.hasNext
    +
                 override final def next(): InternalRow = {
                   val startTime = System.currentTimeMillis()
                   val row: InternalRow = iter.next()
                   val data = row.toSeq(dataTypes).toArray
                   dictIndex.foreach { index =>
                     if (data(index) != null) {
    -                  data(index) = DataTypeUtil.getDataBasedOnDataType(dicts(index)
    -                    .getDictionaryValueForKey(data(index).asInstanceOf[Int]),
    -                    getDictionaryColumnIds(index)._3)
    +                  try {
    +                    data(index) = DataTypeUtil.getDataBasedOnDataType(dicts(index)
    +                      .getDictionaryValueForKey(data(index).asInstanceOf[Int]),
    +                      getDictionaryColumnIds(index)._3)
    +                  }
    +                  catch {
    +                    case x: ClassCastException => {
    --- End diff --
   
    Please just add  `!carbonDimension.isComplex()` in line number 61 and 133. That should solve the problem, remove all other unnecessary excepion handlings



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata pull request #597: [CARBONDATA-672]complex data type su...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/597#discussion_r101241648
 
    --- Diff: integration/spark/src/main/scala/org/apache/spark/sql/CarbonDictionaryDecoder.scala ---
    @@ -193,15 +194,21 @@ case class CarbonDictionaryDecoder(
                   }
                   flag
                 }
    +
                 override final def next(): InternalRow = {
                   val startTime = System.currentTimeMillis()
                   val row: InternalRow = iter.next()
                   val data = row.toSeq(dataTypes).toArray
                   dictIndex.foreach { index =>
                     if (data(index) != null) {
    -                  data(index) = DataTypeUtil.getDataBasedOnDataType(dicts(index)
    -                    .getDictionaryValueForKey(data(index).asInstanceOf[Int]),
    -                    getDictionaryColumnIds(index)._3)
    +                  try {
    +                    data(index) = DataTypeUtil.getDataBasedOnDataType(dicts(index)
    --- End diff --
   
    Please just add  `!carbonDimension.isComplex()` in line number 63 and 132. That should solve the problem, remove all other unnecessary excepion handlings



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Failed  with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/906/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/907/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata pull request #597: [CARBONDATA-672]complex data type su...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/597#discussion_r101950635
 
    --- Diff: integration/spark2/src/main/scala/org/apache/spark/sql/CarbonDictionaryDecoder.scala ---
    @@ -272,26 +274,26 @@ class CarbonDecoderRDD(
           case DataType.DATE => DateType
           case DataType.STRUCT =>
             CarbonMetastoreTypes
    -            .toDataType(s"struct<${ relation.getStructChildren(carbonDimension.getColName) }>")
    +          .toDataType(s"struct<${ relation.getStructChildren(carbonDimension.getColName) }>")
           case DataType.ARRAY =>
             CarbonMetastoreTypes
    -            .toDataType(s"array<${ relation.getArrayChildren(carbonDimension.getColName) }>")
    +          .toDataType(s"array<${ relation.getArrayChildren(carbonDimension.getColName) }>")
         }
       }
     
       val getDictionaryColumnIds = {
         val dictIds: Array[(String, ColumnIdentifier, DataType)] = output.map { a =>
           val attr = aliasMap.getOrElse(a, a)
           val relation = relations.find(p => p.contains(attr))
    -      if(relation.isDefined && canBeDecoded(attr)) {
    +      if (relation.isDefined && canBeDecoded(attr)) {
             val carbonTable = relation.get.carbonRelation.carbonRelation.metaData.carbonTable
             val carbonDimension =
               carbonTable.getDimensionByName(carbonTable.getFactTableName, attr.name)
             if (carbonDimension != null &&
                 carbonDimension.hasEncoding(Encoding.DICTIONARY) &&
                 !carbonDimension.hasEncoding(Encoding.DIRECT_DICTIONARY)) {
    --- End diff --
   
    Please add that check here as well


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/920/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata issue #597: [CARBONDATA-672]complex data type supported

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user ravipesala commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/597
 
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] incubator-carbondata pull request #597: [CARBONDATA-672]complex data type su...

qiuchenjian-2
In reply to this post by qiuchenjian-2
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-carbondata/pull/597


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---