You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by davies <gi...@git.apache.org> on 2015/10/03 02:54:46 UTC

[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

GitHub user davies opened a pull request:

    https://github.com/apache/spark/pull/8971

    [SPARK-10917] [SQL] improve performance of complex type in columnar cache

    This PR improve the performance of complex types in columnar cache by using UnsafeProjection instead of KryoSerializer. 

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

    $ git pull https://github.com/davies/spark complex

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

    https://github.com/apache/spark/pull/8971.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 #8971
    
----
commit 713c8846297f5b54f922c140df9ee735e127b877
Author: Davies Liu <da...@databricks.com>
Date:   2015-10-03T00:45:39Z

    improve performance of complex type in columnar cache

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145190178
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41415962
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -399,24 +409,158 @@ private[sql] object BINARY extends ByteArrayColumnType(16) {
       override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
         row.getBinary(ordinal)
       }
    +
    +  def serialize(value: Array[Byte]): Array[Byte] = value
    +  def deserialize(bytes: Array[Byte]): Array[Byte] = bytes
     }
     
    -// Used to process generic objects (all types other than those listed above). Objects should be
    -// serialized first before appending to the column `ByteBuffer`, and is also extracted as serialized
    -// byte array.
    -private[sql] case class GENERIC(dataType: DataType) extends ByteArrayColumnType(16) {
    -  override def setField(row: MutableRow, ordinal: Int, value: Array[Byte]): Unit = {
    -    row.update(ordinal, SparkSqlSerializer.deserialize[Any](value))
    +private[sql] case class DECIMAL(precision: Int, scale: Int)
    +  extends ByteArrayColumnType[Decimal](20) {
    +
    +  override val dataType: DataType = DecimalType(precision, scale)
    +
    +  override def getField(row: InternalRow, ordinal: Int): Decimal = {
    +    row.getDecimal(ordinal, precision, scale)
       }
     
    -  override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
    -    SparkSqlSerializer.serialize(row.get(ordinal, dataType))
    +  override def setField(row: MutableRow, ordinal: Int, value: Decimal): Unit = {
    +    row.setDecimal(ordinal, value, precision)
    +  }
    +
    +  override def serialize(value: Decimal): Array[Byte] = {
    +    value.toJavaBigDecimal.unscaledValue().toByteArray
    +  }
    +
    +  override def deserialize(bytes: Array[Byte]): Decimal = {
    +    val javaDecimal = new BigDecimal(new BigInteger(bytes), scale)
    +    Decimal.apply(javaDecimal, precision, scale)
    +  }
    +}
    +
    +private[sql] case class STRUCT(dataType: DataType)
    +  extends ByteArrayColumnType[InternalRow](20) {
    +
    +  private val projection: UnsafeProjection =
    +    UnsafeProjection.create(dataType.asInstanceOf[StructType])
    +  private val numOfFields: Int = dataType.asInstanceOf[StructType].fields.size
    +
    +  override def setField(row: MutableRow, ordinal: Int, value: InternalRow): Unit = {
    +    row.update(ordinal, value)
    +  }
    +
    +  override def getField(row: InternalRow, ordinal: Int): InternalRow = {
    +    row.getStruct(ordinal, numOfFields)
    +  }
    +
    +  override def serialize(value: InternalRow): Array[Byte] = {
    +    val unsafeRow = if (value.isInstanceOf[UnsafeRow]) {
    +      value.asInstanceOf[UnsafeRow]
    +    } else {
    +      projection(value)
    +    }
    +    unsafeRow.getBytes
       }
    +
    +  override def deserialize(bytes: Array[Byte]): InternalRow = {
    +    val unsafeRow = new UnsafeRow
    +    unsafeRow.pointTo(bytes, numOfFields, bytes.length)
    +    unsafeRow
    +  }
    +
    +  override def clone(v: InternalRow): InternalRow = v.copy()
    +}
    +
    +private[sql] case class ARRAY(dataType: DataType)
    +  extends ByteArrayColumnType[ArrayData](16) {
    +
    +  private lazy val projection = UnsafeProjection.create(Array(dataType))
    +  private val mutableRow = new GenericMutableRow(new Array[Any](1))
    +
    +  override def setField(row: MutableRow, ordinal: Int, value: ArrayData): Unit = {
    +    row.update(ordinal, value)
    +  }
    +
    +  override def getField(row: InternalRow, ordinal: Int): ArrayData = {
    +    row.getArray(ordinal)
    +  }
    +
    +  override def serialize(value: ArrayData): Array[Byte] = {
    +    val unsafeArray = if (value.isInstanceOf[UnsafeArrayData]) {
    +      value.asInstanceOf[UnsafeArrayData]
    +    } else {
    +      mutableRow(0) = value
    +      projection(mutableRow).getArray(0)
    --- End diff --
    
    We could, but only saving two lines, it may not worth it.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41430216
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -88,7 +88,7 @@ final class Decimal extends Ordered[Decimal] with Serializable {
           if (precision < 19) {
             return null  // Requested precision is too low to represent this value
           }
    -      this.decimalVal = BigDecimal(unscaled)
    +      this.decimalVal = BigDecimal(unscaled, scale)
    --- End diff --
    
    +1


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41458798
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -88,7 +88,7 @@ final class Decimal extends Ordered[Decimal] with Serializable {
           if (precision < 19) {
             return null  // Requested precision is too low to represent this value
           }
    -      this.decimalVal = BigDecimal(unscaled)
    +      this.decimalVal = BigDecimal(unscaled, scale)
    --- End diff --
    
    https://issues.apache.org/jira/browse/SPARK-10980


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41430980
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -102,6 +103,16 @@ private[sql] sealed abstract class ColumnType[JvmType] {
       override def toString: String = getClass.getSimpleName.stripSuffix("$")
     }
     
    +private[sql] object NULL extends ColumnType[Any] {
    +
    +  override def dataType: DataType = NullType
    +  override def defaultSize: Int = 0
    +  override def append(v: Any, buffer: ByteBuffer): Unit = {}
    +  override def extract(buffer: ByteBuffer): Any = null
    +  override def setField(row: MutableRow, ordinal: Int, value: Any): Unit = row.setNullAt(ordinal)
    +  override def getField(row: InternalRow, ordinal: Int): Any = null
    +}
    +
    --- End diff --
    
    @cloud-fan Oh yes, forgot things like `SELECT NULL`. And checked that in Hive the type of such columns is `void`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145611558
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145191277
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145607323
  
      [Test build #43241 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43241/consoleFull) for   PR 8971 at commit [`59bb2f9`](https://github.com/apache/spark/commit/59bb2f969cef9079606e2918289ce33db3201db4).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145934504
  
      [Test build #43281 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43281/consoleFull) for   PR 8971 at commit [`d0be9e4`](https://github.com/apache/spark/commit/d0be9e4ff97727c637fefb7a24378ad0d14640e9).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145605359
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145679692
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145908987
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41415762
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -399,24 +409,158 @@ private[sql] object BINARY extends ByteArrayColumnType(16) {
       override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
         row.getBinary(ordinal)
       }
    +
    +  def serialize(value: Array[Byte]): Array[Byte] = value
    +  def deserialize(bytes: Array[Byte]): Array[Byte] = bytes
     }
     
    -// Used to process generic objects (all types other than those listed above). Objects should be
    -// serialized first before appending to the column `ByteBuffer`, and is also extracted as serialized
    -// byte array.
    -private[sql] case class GENERIC(dataType: DataType) extends ByteArrayColumnType(16) {
    -  override def setField(row: MutableRow, ordinal: Int, value: Array[Byte]): Unit = {
    -    row.update(ordinal, SparkSqlSerializer.deserialize[Any](value))
    +private[sql] case class DECIMAL(precision: Int, scale: Int)
    +  extends ByteArrayColumnType[Decimal](20) {
    --- End diff --
    
    The constructor of BigInteger need to how the number of bytes, it will become even complicated. And most of Decimal will be smaller than 8 bytes, even with precision as 38.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145690634
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145611560
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43241/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146273752
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145947938
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43276/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145678371
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145621181
  
      [Test build #1843 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1843/consoleFull) for   PR 8971 at commit [`59bb2f9`](https://github.com/apache/spark/commit/59bb2f969cef9079606e2918289ce33db3201db4).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145971056
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41352348
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnStats.scala ---
    @@ -254,7 +255,7 @@ private[sql] class FixedDecimalColumnStats(precision: Int, scale: Int) extends C
     }
     
     private[sql] class GenericColumnStats(dataType: DataType) extends ColumnStats {
    --- End diff --
    
    maybe `NonNativeColumnStats` or `ByteArrayColumnStats`?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145191280
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43203/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41428941
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -102,6 +103,16 @@ private[sql] sealed abstract class ColumnType[JvmType] {
       override def toString: String = getClass.getSimpleName.stripSuffix("$")
     }
     
    +private[sql] object NULL extends ColumnType[Any] {
    +
    +  override def dataType: DataType = NullType
    +  override def defaultSize: Int = 0
    +  override def append(v: Any, buffer: ByteBuffer): Unit = {}
    +  override def extract(buffer: ByteBuffer): Any = null
    +  override def setField(row: MutableRow, ordinal: Int, value: Any): Unit = row.setNullAt(ordinal)
    +  override def getField(row: InternalRow, ordinal: Int): Any = null
    +}
    +
    --- End diff --
    
    Found that:
    
    1. we can create a DataFrame with `NullType` via `sqlContext.createDataFrame(rdd, schema)` with a schema having a `NullType` column
    2. before, all-null columns are materialized using `GENERIC` column type
    
    But I'm not sure whether such columns are useful, since we can't create such columns using either our own SQL dialect or HiveQL.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145675123
  
      [Test build #43249 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43249/consoleFull) for   PR 8971 at commit [`f9a3502`](https://github.com/apache/spark/commit/f9a3502516b0a040f51d02889ea69cb19e05c3cb).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145191270
  
      [Test build #43203 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43203/console) for   PR 8971 at commit [`713c884`](https://github.com/apache/spark/commit/713c8846297f5b54f922c140df9ee735e127b877).
     * This patch **fails MiMa tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41352041
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -399,24 +409,158 @@ private[sql] object BINARY extends ByteArrayColumnType(16) {
       override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
         row.getBinary(ordinal)
       }
    +
    +  def serialize(value: Array[Byte]): Array[Byte] = value
    +  def deserialize(bytes: Array[Byte]): Array[Byte] = bytes
     }
     
    -// Used to process generic objects (all types other than those listed above). Objects should be
    -// serialized first before appending to the column `ByteBuffer`, and is also extracted as serialized
    -// byte array.
    -private[sql] case class GENERIC(dataType: DataType) extends ByteArrayColumnType(16) {
    -  override def setField(row: MutableRow, ordinal: Int, value: Array[Byte]): Unit = {
    -    row.update(ordinal, SparkSqlSerializer.deserialize[Any](value))
    +private[sql] case class DECIMAL(precision: Int, scale: Int)
    +  extends ByteArrayColumnType[Decimal](20) {
    --- End diff --
    
    can we save the length header for decimal? i.e. always write 16 bytes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145679668
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41442678
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnStats.scala ---
    @@ -245,7 +247,8 @@ private[sql] class FixedDecimalColumnStats(precision: Int, scale: Int) extends C
           val value = row.getDecimal(ordinal, precision, scale)
           if (upper == null || value.compareTo(upper) > 0) upper = value
           if (lower == null || value.compareTo(lower) < 0) lower = value
    -      sizeInBytes += FIXED_DECIMAL.defaultSize
    +      // TODO: this is not right for DecimalType with precision > 18
    --- End diff --
    
    Why not use `ObjectColumnStats` for large decimal?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145701554
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41455135
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnStats.scala ---
    @@ -245,7 +247,8 @@ private[sql] class FixedDecimalColumnStats(precision: Int, scale: Int) extends C
           val value = row.getDecimal(ordinal, precision, scale)
           if (upper == null || value.compareTo(upper) > 0) upper = value
           if (lower == null || value.compareTo(lower) < 0) lower = value
    -      sizeInBytes += FIXED_DECIMAL.defaultSize
    +      // TODO: this is not right for DecimalType with precision > 18
    +      sizeInBytes += 8
    --- End diff --
    
    For decimals with precision greater than 18, `CatalystSchemaConverter.minBytesForPrecision` can be useful here. At least it provides a relatively tight upper limit.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146317015
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146356930
  
    Merged into master. Other nit comments will be addressed by follow up PR, thanks!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145933256
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41442531
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnAccessor.scala ---
    @@ -96,11 +101,23 @@ private[sql] class BinaryColumnAccessor(buffer: ByteBuffer)
       extends BasicColumnAccessor[Array[Byte]](buffer, BINARY)
       with NullableColumnAccessor
     
    -private[sql] class FixedDecimalColumnAccessor(buffer: ByteBuffer, precision: Int, scale: Int)
    -  extends NativeColumnAccessor(buffer, FIXED_DECIMAL(precision, scale))
    +private[sql] class CompactDecimalColumnAccessor(buffer: ByteBuffer, dataType: DecimalType)
    +  extends NativeColumnAccessor(buffer, COMPACT_DECIMAL(dataType))
    +
    +private[sql] class DecimalColumnAccessor(buffer: ByteBuffer, dataType: DecimalType)
    --- End diff --
    
    maybe `LargeDecimalColumnAccessor` according to the renaming change?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145678374
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43249/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145718674
  
    Merged build finished. Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145678334
  
      [Test build #43249 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43249/console) for   PR 8971 at commit [`f9a3502`](https://github.com/apache/spark/commit/f9a3502516b0a040f51d02889ea69cb19e05c3cb).
     * This patch **fails MiMa tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class BufferHolder `
      * `public class UnsafeArrayWriter `
      * `public class UnsafeRowWriter `



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146317050
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146273707
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41430109
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnAccessor.scala ---
    @@ -117,9 +135,14 @@ private[sql] object ColumnAccessor {
           case DoubleType => new DoubleColumnAccessor(buf)
           case StringType => new StringColumnAccessor(buf)
           case BinaryType => new BinaryColumnAccessor(buf)
    -      case DecimalType.Fixed(precision, scale) if precision < 19 =>
    -        new FixedDecimalColumnAccessor(buf, precision, scale)
    -      case other => new GenericColumnAccessor(buf, other)
    +      case dt: DecimalType if dt.precision < 19 => new CompactDecimalColumnAccessor(buf, dt)
    --- End diff --
    
    in other places we use `<= Decimal.MAX_LONG_DIGITS`, I think we should follow it here.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41458139
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnStats.scala ---
    @@ -245,7 +247,8 @@ private[sql] class FixedDecimalColumnStats(precision: Int, scale: Int) extends C
           val value = row.getDecimal(ordinal, precision, scale)
           if (upper == null || value.compareTo(upper) > 0) upper = value
           if (lower == null || value.compareTo(lower) < 0) lower = value
    -      sizeInBytes += FIXED_DECIMAL.defaultSize
    +      // TODO: this is not right for DecimalType with precision > 18
    +      sizeInBytes += 8
    --- End diff --
    
    Good point, thanks. I thinking it's better to get the number of written bytes from buffer, no more guessing.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145673389
  
    Merged build started.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41439861
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnBuilder.scala ---
    @@ -109,16 +113,20 @@ private[sql] class StringColumnBuilder extends NativeColumnBuilder(new StringCol
     
     private[sql] class BinaryColumnBuilder extends ComplexColumnBuilder(new BinaryColumnStats, BINARY)
     
    -private[sql] class FixedDecimalColumnBuilder(
    -    precision: Int,
    -    scale: Int)
    -  extends NativeColumnBuilder(
    -    new FixedDecimalColumnStats(precision, scale),
    -    FIXED_DECIMAL(precision, scale))
    +private[sql] class CompactDecimalColumnBuilder(dataType: DecimalType)
    +  extends NativeColumnBuilder(new DecimalColumnStats(dataType), COMPACT_DECIMAL(dataType))
    +
    +private[sql] class DecimalColumnBuilder(dataType: DecimalType)
    +  extends ComplexColumnBuilder(new DecimalColumnStats(dataType), DECIMAL(dataType))
    --- End diff --
    
    No, we may want to have min/max of Decimal


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145690608
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145662420
  
      [Test build #1843 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/1843/console) for   PR 8971 at commit [`59bb2f9`](https://github.com/apache/spark/commit/59bb2f969cef9079606e2918289ce33db3201db4).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145190566
  
      [Test build #43203 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43203/consoleFull) for   PR 8971 at commit [`713c884`](https://github.com/apache/spark/commit/713c8846297f5b54f922c140df9ee735e127b877).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41426857
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -102,6 +103,16 @@ private[sql] sealed abstract class ColumnType[JvmType] {
       override def toString: String = getClass.getSimpleName.stripSuffix("$")
     }
     
    +private[sql] object NULL extends ColumnType[Any] {
    +
    +  override def dataType: DataType = NullType
    +  override def defaultSize: Int = 0
    +  override def append(v: Any, buffer: ByteBuffer): Unit = {}
    +  override def extract(buffer: ByteBuffer): Any = null
    +  override def setField(row: MutableRow, ordinal: Int, value: Any): Unit = row.setNullAt(ordinal)
    +  override def getField(row: InternalRow, ordinal: Int): Any = null
    +}
    +
    --- End diff --
    
    like `select a+b, null from tbl`, if we need to cache the result, we need to store the null column.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145673361
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145611452
  
      [Test build #43241 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43241/console) for   PR 8971 at commit [`59bb2f9`](https://github.com/apache/spark/commit/59bb2f969cef9079606e2918289ce33db3201db4).
     * This patch **fails MiMa tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41425869
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -102,6 +103,16 @@ private[sql] sealed abstract class ColumnType[JvmType] {
       override def toString: String = getClass.getSimpleName.stripSuffix("$")
     }
     
    +private[sql] object NULL extends ColumnType[Any] {
    +
    +  override def dataType: DataType = NullType
    +  override def defaultSize: Int = 0
    +  override def append(v: Any, buffer: ByteBuffer): Unit = {}
    +  override def extract(buffer: ByteBuffer): Any = null
    +  override def setField(row: MutableRow, ordinal: Int, value: Any): Unit = row.setNullAt(ordinal)
    +  override def getField(row: InternalRow, ordinal: Int): Any = null
    +}
    +
    --- End diff --
    
    When do we need to store an in-memory all-null column?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146307599
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41426637
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnAccessor.scala ---
    @@ -117,9 +135,14 @@ private[sql] object ColumnAccessor {
           case DoubleType => new DoubleColumnAccessor(buf)
           case StringType => new StringColumnAccessor(buf)
           case BinaryType => new BinaryColumnAccessor(buf)
    -      case DecimalType.Fixed(precision, scale) if precision < 19 =>
    -        new FixedDecimalColumnAccessor(buf, precision, scale)
    -      case other => new GenericColumnAccessor(buf, other)
    +      case dt: DecimalType if dt.precision < 19 => new CompactDecimalColumnAccessor(buf, dt)
    --- End diff --
    
    I think we already have, https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala#L316.
    
    But it's 18 not 19?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41430728
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnAccessor.scala ---
    @@ -117,9 +135,14 @@ private[sql] object ColumnAccessor {
           case DoubleType => new DoubleColumnAccessor(buf)
           case StringType => new StringColumnAccessor(buf)
           case BinaryType => new BinaryColumnAccessor(buf)
    -      case DecimalType.Fixed(precision, scale) if precision < 19 =>
    -        new FixedDecimalColumnAccessor(buf, precision, scale)
    -      case other => new GenericColumnAccessor(buf, other)
    +      case dt: DecimalType if dt.precision < 19 => new CompactDecimalColumnAccessor(buf, dt)
    --- End diff --
    
    +1


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/8971


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146275566
  
      [Test build #43329 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43329/consoleFull) for   PR 8971 at commit [`9c5718d`](https://github.com/apache/spark/commit/9c5718d0000b796e1c1f5bb78d7e21ed5684c80a).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146351273
  
      [Test build #43341 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43341/console) for   PR 8971 at commit [`4f0a94e`](https://github.com/apache/spark/commit/4f0a94ef3ba332ec9dda634e58cb8236eeaf2bed).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145947937
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145679568
  
    @liancheng Could you help to review this?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145718676
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43257/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146351398
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43341/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145605320
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145701555
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43253/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146293774
  
    LGTM except for a few minor issues.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146307602
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43329/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145947768
  
      [Test build #43276 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43276/console) for   PR 8971 at commit [`54479f1`](https://github.com/apache/spark/commit/54479f15137e399100913cc2392cb00c1196b83d).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146307412
  
      [Test build #43329 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43329/console) for   PR 8971 at commit [`9c5718d`](https://github.com/apache/spark/commit/9c5718d0000b796e1c1f5bb78d7e21ed5684c80a).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41353513
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -365,32 +374,33 @@ private[sql] case class FIXED_DECIMAL(precision: Int, scale: Int)
       }
     }
     
    -private[sql] object FIXED_DECIMAL {
    -  val defaultSize = 8
    -}
    +private[sql] sealed abstract class ByteArrayColumnType[JvmType](val defaultSize: Int)
    +  extends ColumnType[JvmType] {
     
    -private[sql] sealed abstract class ByteArrayColumnType(val defaultSize: Int)
    -  extends ColumnType[Array[Byte]] {
    +  def serialize(value: JvmType): Array[Byte]
    +  def deserialize(bytes: Array[Byte]): JvmType
     
       override def actualSize(row: InternalRow, ordinal: Int): Int = {
    -    getField(row, ordinal).length + 4
    +    // TODO: grow the buffer in append(), so serialize() will not be called twice
    +    serialize(getField(row, ordinal)).length + 4
       }
     
    -  override def append(v: Array[Byte], buffer: ByteBuffer): Unit = {
    -    buffer.putInt(v.length).put(v, 0, v.length)
    +  override def append(v: JvmType, buffer: ByteBuffer): Unit = {
    +    val bytes = serialize(v)
    +    buffer.putInt(bytes.length).put(bytes, 0, bytes.length)
    --- End diff --
    
    looks like only binary type need the length header? struct, array, map all have length info inside.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146319179
  
      [Test build #43341 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43341/consoleFull) for   PR 8971 at commit [`4f0a94e`](https://github.com/apache/spark/commit/4f0a94ef3ba332ec9dda634e58cb8236eeaf2bed).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41425584
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnAccessor.scala ---
    @@ -117,9 +135,14 @@ private[sql] object ColumnAccessor {
           case DoubleType => new DoubleColumnAccessor(buf)
           case StringType => new StringColumnAccessor(buf)
           case BinaryType => new BinaryColumnAccessor(buf)
    -      case DecimalType.Fixed(precision, scale) if precision < 19 =>
    -        new FixedDecimalColumnAccessor(buf, precision, scale)
    -      case other => new GenericColumnAccessor(buf, other)
    +      case dt: DecimalType if dt.precision < 19 => new CompactDecimalColumnAccessor(buf, dt)
    --- End diff --
    
    Would be nice to define this magic number as something like `Decimal.MAX_PRECISION_FOR_INT64`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145908960
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41425580
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/types/Decimal.scala ---
    @@ -88,7 +88,7 @@ final class Decimal extends Ordered[Decimal] with Serializable {
           if (precision < 19) {
             return null  // Requested precision is too low to represent this value
           }
    -      this.decimalVal = BigDecimal(unscaled)
    +      this.decimalVal = BigDecimal(unscaled, scale)
    --- End diff --
    
    This seems to be a bug that worths a separate JIRA ticket.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41352948
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -399,24 +409,158 @@ private[sql] object BINARY extends ByteArrayColumnType(16) {
       override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
         row.getBinary(ordinal)
       }
    +
    +  def serialize(value: Array[Byte]): Array[Byte] = value
    +  def deserialize(bytes: Array[Byte]): Array[Byte] = bytes
     }
     
    -// Used to process generic objects (all types other than those listed above). Objects should be
    -// serialized first before appending to the column `ByteBuffer`, and is also extracted as serialized
    -// byte array.
    -private[sql] case class GENERIC(dataType: DataType) extends ByteArrayColumnType(16) {
    -  override def setField(row: MutableRow, ordinal: Int, value: Array[Byte]): Unit = {
    -    row.update(ordinal, SparkSqlSerializer.deserialize[Any](value))
    +private[sql] case class DECIMAL(precision: Int, scale: Int)
    +  extends ByteArrayColumnType[Decimal](20) {
    +
    +  override val dataType: DataType = DecimalType(precision, scale)
    +
    +  override def getField(row: InternalRow, ordinal: Int): Decimal = {
    +    row.getDecimal(ordinal, precision, scale)
       }
     
    -  override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
    -    SparkSqlSerializer.serialize(row.get(ordinal, dataType))
    +  override def setField(row: MutableRow, ordinal: Int, value: Decimal): Unit = {
    +    row.setDecimal(ordinal, value, precision)
    +  }
    +
    +  override def serialize(value: Decimal): Array[Byte] = {
    +    value.toJavaBigDecimal.unscaledValue().toByteArray
    +  }
    +
    +  override def deserialize(bytes: Array[Byte]): Decimal = {
    +    val javaDecimal = new BigDecimal(new BigInteger(bytes), scale)
    +    Decimal.apply(javaDecimal, precision, scale)
    +  }
    +}
    +
    +private[sql] case class STRUCT(dataType: DataType)
    +  extends ByteArrayColumnType[InternalRow](20) {
    +
    +  private val projection: UnsafeProjection =
    +    UnsafeProjection.create(dataType.asInstanceOf[StructType])
    +  private val numOfFields: Int = dataType.asInstanceOf[StructType].fields.size
    +
    +  override def setField(row: MutableRow, ordinal: Int, value: InternalRow): Unit = {
    +    row.update(ordinal, value)
    +  }
    +
    +  override def getField(row: InternalRow, ordinal: Int): InternalRow = {
    +    row.getStruct(ordinal, numOfFields)
    +  }
    +
    +  override def serialize(value: InternalRow): Array[Byte] = {
    +    val unsafeRow = if (value.isInstanceOf[UnsafeRow]) {
    +      value.asInstanceOf[UnsafeRow]
    +    } else {
    +      projection(value)
    +    }
    +    unsafeRow.getBytes
       }
    +
    +  override def deserialize(bytes: Array[Byte]): InternalRow = {
    +    val unsafeRow = new UnsafeRow
    +    unsafeRow.pointTo(bytes, numOfFields, bytes.length)
    +    unsafeRow
    +  }
    +
    +  override def clone(v: InternalRow): InternalRow = v.copy()
    +}
    +
    +private[sql] case class ARRAY(dataType: DataType)
    +  extends ByteArrayColumnType[ArrayData](16) {
    +
    +  private lazy val projection = UnsafeProjection.create(Array(dataType))
    +  private val mutableRow = new GenericMutableRow(new Array[Any](1))
    +
    +  override def setField(row: MutableRow, ordinal: Int, value: ArrayData): Unit = {
    +    row.update(ordinal, value)
    +  }
    +
    +  override def getField(row: InternalRow, ordinal: Int): ArrayData = {
    +    row.getArray(ordinal)
    +  }
    +
    +  override def serialize(value: ArrayData): Array[Byte] = {
    +    val unsafeArray = if (value.isInstanceOf[UnsafeArrayData]) {
    +      value.asInstanceOf[UnsafeArrayData]
    +    } else {
    +      mutableRow(0) = value
    +      projection(mutableRow).getArray(0)
    --- End diff --
    
    Should we create a `UnsafeArrayConverter` that turn an array to unsafe array directly?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41426190
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -399,24 +415,164 @@ private[sql] object BINARY extends ByteArrayColumnType(16) {
       override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
         row.getBinary(ordinal)
       }
    +
    +  def serialize(value: Array[Byte]): Array[Byte] = value
    +  def deserialize(bytes: Array[Byte]): Array[Byte] = bytes
     }
     
    -// Used to process generic objects (all types other than those listed above). Objects should be
    -// serialized first before appending to the column `ByteBuffer`, and is also extracted as serialized
    -// byte array.
    -private[sql] case class GENERIC(dataType: DataType) extends ByteArrayColumnType(16) {
    -  override def setField(row: MutableRow, ordinal: Int, value: Array[Byte]): Unit = {
    -    row.update(ordinal, SparkSqlSerializer.deserialize[Any](value))
    +private[sql] case class DECIMAL(precision: Int, scale: Int)
    --- End diff --
    
    Maybe rename this one as `LARGE_DECIMAL` (in contrast to `COMPACT_DECIMAL`) since this only handles decimals with precision >= 19.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41352081
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnType.scala ---
    @@ -399,24 +409,158 @@ private[sql] object BINARY extends ByteArrayColumnType(16) {
       override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
         row.getBinary(ordinal)
       }
    +
    +  def serialize(value: Array[Byte]): Array[Byte] = value
    +  def deserialize(bytes: Array[Byte]): Array[Byte] = bytes
     }
     
    -// Used to process generic objects (all types other than those listed above). Objects should be
    -// serialized first before appending to the column `ByteBuffer`, and is also extracted as serialized
    -// byte array.
    -private[sql] case class GENERIC(dataType: DataType) extends ByteArrayColumnType(16) {
    -  override def setField(row: MutableRow, ordinal: Int, value: Array[Byte]): Unit = {
    -    row.update(ordinal, SparkSqlSerializer.deserialize[Any](value))
    +private[sql] case class DECIMAL(precision: Int, scale: Int)
    +  extends ByteArrayColumnType[Decimal](20) {
    +
    +  override val dataType: DataType = DecimalType(precision, scale)
    +
    +  override def getField(row: InternalRow, ordinal: Int): Decimal = {
    +    row.getDecimal(ordinal, precision, scale)
       }
     
    -  override def getField(row: InternalRow, ordinal: Int): Array[Byte] = {
    -    SparkSqlSerializer.serialize(row.get(ordinal, dataType))
    +  override def setField(row: MutableRow, ordinal: Int, value: Decimal): Unit = {
    +    row.setDecimal(ordinal, value, precision)
    +  }
    +
    +  override def serialize(value: Decimal): Array[Byte] = {
    +    value.toJavaBigDecimal.unscaledValue().toByteArray
    +  }
    +
    +  override def deserialize(bytes: Array[Byte]): Decimal = {
    +    val javaDecimal = new BigDecimal(new BigInteger(bytes), scale)
    +    Decimal.apply(javaDecimal, precision, scale)
    +  }
    +}
    +
    +private[sql] case class STRUCT(dataType: DataType)
    +  extends ByteArrayColumnType[InternalRow](20) {
    +
    +  private val projection: UnsafeProjection =
    +    UnsafeProjection.create(dataType.asInstanceOf[StructType])
    +  private val numOfFields: Int = dataType.asInstanceOf[StructType].fields.size
    --- End diff --
    
    why not just make the `dataType` parameter `StructType`?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145933219
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145691725
  
      [Test build #43257 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43257/consoleFull) for   PR 8971 at commit [`3b9f59e`](https://github.com/apache/spark/commit/3b9f59ea0e91c1354a89cbfd8fe7f9c38f4959a4).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145971061
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43281/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145190173
  
     Merged build triggered.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145911454
  
      [Test build #43276 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43276/consoleFull) for   PR 8971 at commit [`54479f1`](https://github.com/apache/spark/commit/54479f15137e399100913cc2392cb00c1196b83d).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145680842
  
      [Test build #43253 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43253/consoleFull) for   PR 8971 at commit [`3ad1e9c`](https://github.com/apache/spark/commit/3ad1e9c134d6f0642b22874dca4012aad8a75ece).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145718624
  
      [Test build #43257 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43257/console) for   PR 8971 at commit [`3b9f59e`](https://github.com/apache/spark/commit/3b9f59ea0e91c1354a89cbfd8fe7f9c38f4959a4).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-146351397
  
    Merged build finished. Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41200027
  
    --- Diff: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/UnsafeArrayData.java ---
    @@ -285,7 +284,12 @@ public UnsafeMapData getMap(int ordinal) {
     
       @Override
       public int hashCode() {
    -    return Murmur3_x86_32.hashUnsafeWords(baseObject, baseOffset, sizeInBytes, 42);
    --- End diff --
    
    The sizeInBytes is not aligned to words.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145970902
  
      [Test build #43281 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43281/console) for   PR 8971 at commit [`d0be9e4`](https://github.com/apache/spark/commit/d0be9e4ff97727c637fefb7a24378ad0d14640e9).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41430343
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/columnar/ColumnTypeSuite.scala ---
    @@ -69,176 +67,65 @@ class ColumnTypeSuite extends SparkFunSuite with Logging {
         checkActualSize(LONG, Long.MaxValue, 8)
         checkActualSize(FLOAT, Float.MaxValue, 4)
         checkActualSize(DOUBLE, Double.MaxValue, 8)
    -    checkActualSize(STRING, UTF8String.fromString("hello"), 4 + "hello".getBytes("utf-8").length)
    +    checkActualSize(STRING, "hello", 4 + "hello".getBytes("utf-8").length)
         checkActualSize(BINARY, Array.fill[Byte](4)(0.toByte), 4 + 4)
    -    checkActualSize(FIXED_DECIMAL(15, 10), Decimal(0, 15, 10), 8)
    -
    -    val generic = Map(1 -> "a")
    -    checkActualSize(MAP_GENERIC, SparkSqlSerializer.serialize(generic), 4 + 8)
    +    checkActualSize(COMPACT_DECIMAL(15, 10), Decimal(0, 15, 10), 8)
    +    checkActualSize(DECIMAL(20, 10), Decimal(0, 20, 10), 5)
    +    checkActualSize(ARRAY_TYPE, Array[Any](1), 16)
    +    checkActualSize(MAP_TYPE, Map(1 -> "a"), 25)
    +    checkActualSize(STRUCT_TYPE, Row("hello"), 28)
       }
     
    -  testNativeColumnType(BOOLEAN)(
    -    (buffer: ByteBuffer, v: Boolean) => {
    -      buffer.put((if (v) 1 else 0).toByte)
    -    },
    -    (buffer: ByteBuffer) => {
    -      buffer.get() == 1
    -    })
    -
    -  testNativeColumnType(BYTE)(_.put(_), _.get)
    -
    -  testNativeColumnType(SHORT)(_.putShort(_), _.getShort)
    -
    -  testNativeColumnType(INT)(_.putInt(_), _.getInt)
    -
    -  testNativeColumnType(LONG)(_.putLong(_), _.getLong)
    -
    -  testNativeColumnType(FLOAT)(_.putFloat(_), _.getFloat)
    -
    -  testNativeColumnType(DOUBLE)(_.putDouble(_), _.getDouble)
    -
    -  testNativeColumnType(FIXED_DECIMAL(15, 10))(
    -    (buffer: ByteBuffer, decimal: Decimal) => {
    -      buffer.putLong(decimal.toUnscaledLong)
    -    },
    -    (buffer: ByteBuffer) => {
    -      Decimal(buffer.getLong(), 15, 10)
    -    })
    -
    -
    -  testNativeColumnType(STRING)(
    -    (buffer: ByteBuffer, string: UTF8String) => {
    -      val bytes = string.getBytes
    -      buffer.putInt(bytes.length)
    -      buffer.put(bytes)
    -    },
    -    (buffer: ByteBuffer) => {
    -      val length = buffer.getInt()
    -      val bytes = new Array[Byte](length)
    -      buffer.get(bytes)
    -      UTF8String.fromBytes(bytes)
    -    })
    -
    -  testColumnType[Array[Byte]](
    -    BINARY,
    -    (buffer: ByteBuffer, bytes: Array[Byte]) => {
    -      buffer.putInt(bytes.length).put(bytes)
    -    },
    -    (buffer: ByteBuffer) => {
    -      val length = buffer.getInt()
    -      val bytes = new Array[Byte](length)
    -      buffer.get(bytes, 0, length)
    -      bytes
    -    })
    -
    -  test("GENERIC") {
    -    val buffer = ByteBuffer.allocate(512)
    -    val obj = Map(1 -> "spark", 2 -> "sql")
    -    val serializedObj = SparkSqlSerializer.serialize(obj)
    -
    -    MAP_GENERIC.append(SparkSqlSerializer.serialize(obj), buffer)
    -    buffer.rewind()
    -
    -    val length = buffer.getInt()
    -    assert(length === serializedObj.length)
    -
    -    assertResult(obj, "Deserialized object didn't equal to the original object") {
    -      val bytes = new Array[Byte](length)
    -      buffer.get(bytes, 0, length)
    -      SparkSqlSerializer.deserialize(bytes)
    -    }
    -
    -    buffer.rewind()
    -    buffer.putInt(serializedObj.length).put(serializedObj)
    -
    -    assertResult(obj, "Deserialized object didn't equal to the original object") {
    -      buffer.rewind()
    -      SparkSqlSerializer.deserialize(MAP_GENERIC.extract(buffer))
    -    }
    +  testNativeColumnType(BOOLEAN)
    +  testNativeColumnType(BYTE)
    +  testNativeColumnType(SHORT)
    +  testNativeColumnType(INT)
    +  testNativeColumnType(LONG)
    +  testNativeColumnType(FLOAT)
    +  testNativeColumnType(DOUBLE)
    +  testNativeColumnType(COMPACT_DECIMAL(15, 10))
    +  testNativeColumnType(STRING)
    +
    +  testColumnType(NULL)
    +  testColumnType(BINARY)
    +  testColumnType(DECIMAL(20, 10))
    +  testColumnType(STRUCT_TYPE)
    +  testColumnType(ARRAY_TYPE)
    +  testColumnType(MAP_TYPE)
    +
    +  def testNativeColumnType[T <: AtomicType](columnType: NativeColumnType[T]): Unit = {
    +    testColumnType[T#InternalType](columnType)
       }
     
    -  test("CUSTOM") {
    -    val conf = new SparkConf()
    -    conf.set("spark.kryo.registrator", "org.apache.spark.sql.columnar.Registrator")
    -    val serializer = new SparkSqlSerializer(conf).newInstance()
    -
    -    val buffer = ByteBuffer.allocate(512)
    -    val obj = CustomClass(Int.MaxValue, Long.MaxValue)
    -    val serializedObj = serializer.serialize(obj).array()
    -
    -    MAP_GENERIC.append(serializer.serialize(obj).array(), buffer)
    -    buffer.rewind()
    -
    -    val length = buffer.getInt
    -    assert(length === serializedObj.length)
    -    assert(13 == length) // id (1) + int (4) + long (8)
    -
    -    val genericSerializedObj = SparkSqlSerializer.serialize(obj)
    -    assert(length != genericSerializedObj.length)
    -    assert(length < genericSerializedObj.length)
    -
    -    assertResult(obj, "Custom deserialized object didn't equal the original object") {
    -      val bytes = new Array[Byte](length)
    -      buffer.get(bytes, 0, length)
    -      serializer.deserialize(ByteBuffer.wrap(bytes))
    -    }
    -
    -    buffer.rewind()
    -    buffer.putInt(serializedObj.length).put(serializedObj)
    -
    -    assertResult(obj, "Custom deserialized object didn't equal the original object") {
    -      buffer.rewind()
    -      serializer.deserialize(ByteBuffer.wrap(MAP_GENERIC.extract(buffer)))
    -    }
    -  }
    -
    -  def testNativeColumnType[T <: AtomicType](
    -      columnType: NativeColumnType[T])
    -      (putter: (ByteBuffer, T#InternalType) => Unit,
    -      getter: (ByteBuffer) => T#InternalType): Unit = {
    -
    -    testColumnType[T#InternalType](columnType, putter, getter)
    -  }
    -
    -  def testColumnType[JvmType](
    -      columnType: ColumnType[JvmType],
    -      putter: (ByteBuffer, JvmType) => Unit,
    -      getter: (ByteBuffer) => JvmType): Unit = {
    +  def testColumnType[JvmType](columnType: ColumnType[JvmType]): Unit = {
     
         val buffer = ByteBuffer.allocate(DEFAULT_BUFFER_SIZE)
         val seq = (0 until 4).map(_ => makeRandomValue(columnType))
    +    val converter = CatalystTypeConverters.createToScalaConverter(columnType.dataType)
     
    -    test(s"$columnType.extract") {
    +    test(s"$columnType append/extract") {
           buffer.rewind()
    -      seq.foreach(putter(buffer, _))
    +      seq.foreach(columnType.append(_, buffer))
     
           buffer.rewind()
           seq.foreach { expected =>
             logInfo("buffer = " + buffer + ", expected = " + expected)
             val extracted = columnType.extract(buffer)
             assert(
    -          expected === extracted,
    +          converter(expected) === converter(extracted),
               "Extracted value didn't equal to the original one. " +
                 hexDump(expected) + " != " + hexDump(extracted) +
                 ", buffer = " + dumpBuffer(buffer.duplicate().rewind().asInstanceOf[ByteBuffer]))
           }
         }
    -
    -    test(s"$columnType.append") {
    -      buffer.rewind()
    -      seq.foreach(columnType.append(_, buffer))
    -
    -      buffer.rewind()
    -      seq.foreach { expected =>
    -        assert(
    -          expected === getter(buffer),
    -          "Extracted value didn't equal to the original one")
    -      }
    -    }
       }
     
       private def hexDump(value: Any): String = {
    -    value.toString.map(ch => Integer.toHexString(ch & 0xffff)).mkString(" ")
    +    if (value == null) {
    +      ""
    +    } else {
    +      value.toString.map(ch => Integer.toHexString(ch & 0xffff)).mkString(" ")
    +    }
    --- End diff --
    
    Are we dumping all types of data using hexdump here? Maybe just for binaries?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8971#issuecomment-145701518
  
      [Test build #43253 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43253/console) for   PR 8971 at commit [`3ad1e9c`](https://github.com/apache/spark/commit/3ad1e9c134d6f0642b22874dca4012aad8a75ece).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `public class BufferHolder `
      * `public class UnsafeArrayWriter `
      * `public class UnsafeRowWriter `



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10917] [SQL] improve performance of com...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8971#discussion_r41431106
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/columnar/ColumnBuilder.scala ---
    @@ -109,16 +113,20 @@ private[sql] class StringColumnBuilder extends NativeColumnBuilder(new StringCol
     
     private[sql] class BinaryColumnBuilder extends ComplexColumnBuilder(new BinaryColumnStats, BINARY)
     
    -private[sql] class FixedDecimalColumnBuilder(
    -    precision: Int,
    -    scale: Int)
    -  extends NativeColumnBuilder(
    -    new FixedDecimalColumnStats(precision, scale),
    -    FIXED_DECIMAL(precision, scale))
    +private[sql] class CompactDecimalColumnBuilder(dataType: DecimalType)
    +  extends NativeColumnBuilder(new DecimalColumnStats(dataType), COMPACT_DECIMAL(dataType))
    +
    +private[sql] class DecimalColumnBuilder(dataType: DecimalType)
    +  extends ComplexColumnBuilder(new DecimalColumnStats(dataType), DECIMAL(dataType))
    --- End diff --
    
    For non-compact decimal, we use `ByteArrayColumnType`, so maybe use `ObjectColumnStats` here?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org