You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kiszk <gi...@git.apache.org> on 2018/08/22 18:22:10 UTC

[GitHub] spark pull request #22187: [SPARK-25178][SQL] change the generated code of t...

GitHub user kiszk opened a pull request:

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

    [SPARK-25178][SQL] change the generated code of the keySchema / valueSchema for xxxHashMapGenerator

    ## What changes were proposed in this pull request?
    
    This PR generates the code that to refer a `StructType` generated in the scala code instead of generating `StructType` in Java code. This solves two issues.
    1. Avoid to used the field name such as `key.name`
    1. Support complicated schema (e.g. nested DataType)
    
    Originally, [the JIRA entry](https://issues.apache.org/jira/browse/SPARK-25178) proposed to change the generated field name of the keySchema / valueSchema to a dummy name in `RowBasedHashMapGenerator` and `VectorizedHashMapGenerator.scala`. @Ueshin suggested to refer to a `StructType` generated in the scala code using `ctx.addReferenceObj()`.
    
    ## How was this patch tested?
    
    Existing UTs

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

    $ git pull https://github.com/kiszk/spark SPARK-25178

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

    https://github.com/apache/spark/pull/22187.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 #22187
    
----
commit 0626de74f726622ac3eb251fc9f66aaa3de002d3
Author: Kazuaki Ishizaki <is...@...>
Date:   2018-08-22T18:10:24Z

    initial commit

----


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95141 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95141/testReport)** for PR 22187 at commit [`81ef75a`](https://github.com/apache/spark/commit/81ef75a36a1c9dcd6922d2ec77393bc35389efd0).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2456/
    Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

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


---

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


[GitHub] spark pull request #22187: [SPARK-25178][SQL] Directly ship the StructType o...

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

    https://github.com/apache/spark/pull/22187#discussion_r212164362
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala ---
    @@ -44,31 +44,8 @@ class RowBasedHashMapGenerator(
         groupingKeySchema, bufferSchema) {
     
       override protected def initializeAggregateHashMap(): String = {
    -    val generatedKeySchema: String =
    -      s"new org.apache.spark.sql.types.StructType()" +
    -        groupingKeySchema.map { key =>
    -          val keyName = ctx.addReferenceObj("keyName", key.name)
    -          key.dataType match {
    -            case d: DecimalType =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.createDecimalType(
    -                  |${d.precision}, ${d.scale}))""".stripMargin
    -            case _ =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.${key.dataType})"""
    -          }
    -        }.mkString("\n").concat(";")
    -
    -    val generatedValueSchema: String =
    -      s"new org.apache.spark.sql.types.StructType()" +
    -        bufferSchema.map { key =>
    -          val keyName = ctx.addReferenceObj("keyName", key.name)
    -          key.dataType match {
    -            case d: DecimalType =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.createDecimalType(
    -                  |${d.precision}, ${d.scale}))""".stripMargin
    -            case _ =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.${key.dataType})"""
    -          }
    -        }.mkString("\n").concat(";")
    +    val generatedKeySchema = ctx.addReferenceObj("generatedKeySchemaTerm", groupingKeySchema)
    --- End diff --
    
    nit: the variable name sounds strange because the schema is not generated any more.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:

    https://github.com/apache/spark/pull/22187
  
    Jenkins, retest this please.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2495/
    Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95119 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95119/testReport)** for PR 22187 at commit [`86c9c5b`](https://github.com/apache/spark/commit/86c9c5b31f8a7f8ec722ceeedc8e4812ac7159ef).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95166 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95166/testReport)** for PR 22187 at commit [`aaa3944`](https://github.com/apache/spark/commit/aaa3944f20db36898d231d94281ed08995d1da0b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

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


---

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


[GitHub] spark pull request #22187: [SPARK-25178][SQL] Directly ship the StructType o...

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

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


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2481/
    Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

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


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2479/
    Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95117 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95117/testReport)** for PR 22187 at commit [`0626de7`](https://github.com/apache/spark/commit/0626de74f726622ac3eb251fc9f66aaa3de002d3).


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

Posted by rednaxelafx <gi...@git.apache.org>.
Github user rednaxelafx commented on the issue:

    https://github.com/apache/spark/pull/22187
  
    So the new solution now is to directly ship the `StructType` object as a reference object. Why not? ;-)
    I'm +1 on shipping the object directly instead of generating code to recreate it on the executor. If other reviewers are also on board with this approach, let's update the PR title and the JIRA ticket title to reflect that (since we're no longer generating any "names" at all now).


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

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


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95119 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95119/testReport)** for PR 22187 at commit [`86c9c5b`](https://github.com/apache/spark/commit/86c9c5b31f8a7f8ec722ceeedc8e4812ac7159ef).


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95117 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95117/testReport)** for PR 22187 at commit [`0626de7`](https://github.com/apache/spark/commit/0626de74f726622ac3eb251fc9f66aaa3de002d3).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22187
  
    +1 LGTM


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:

    https://github.com/apache/spark/pull/22187
  
    Thanks, updated both titles


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95144 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95144/testReport)** for PR 22187 at commit [`81ef75a`](https://github.com/apache/spark/commit/81ef75a36a1c9dcd6922d2ec77393bc35389efd0).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark pull request #22187: [SPARK-25178][SQL] change the generated code of t...

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

    https://github.com/apache/spark/pull/22187#discussion_r212062744
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala ---
    @@ -44,31 +44,19 @@ class RowBasedHashMapGenerator(
         groupingKeySchema, bufferSchema) {
     
       override protected def initializeAggregateHashMap(): String = {
    -    val generatedKeySchema: String =
    -      s"new org.apache.spark.sql.types.StructType()" +
    -        groupingKeySchema.map { key =>
    -          val keyName = ctx.addReferenceObj("keyName", key.name)
    -          key.dataType match {
    -            case d: DecimalType =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.createDecimalType(
    -                  |${d.precision}, ${d.scale}))""".stripMargin
    -            case _ =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.${key.dataType})"""
    -          }
    -        }.mkString("\n").concat(";")
    +    val generatedKeyColTypes = groupingKeySchema
    +      .zipWithIndex.map { case (t, i) => (s"_col$i", t.dataType) }
    +    val generatedKeySchemaTypes = generatedKeyColTypes
    +      .foldLeft(new StructType())((schema, colType) => schema.add(colType._1, colType._2))
    --- End diff --
    
    Btw, do we need to recreate the struct type with dummy names if we use `ctx.addReferenceObj()`? We might need to do `.asNullable`, though.


---

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


[GitHub] spark pull request #22187: [SPARK-25178][SQL] change the generated code of t...

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

    https://github.com/apache/spark/pull/22187#discussion_r212075572
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala ---
    @@ -44,31 +44,19 @@ class RowBasedHashMapGenerator(
         groupingKeySchema, bufferSchema) {
     
       override protected def initializeAggregateHashMap(): String = {
    -    val generatedKeySchema: String =
    -      s"new org.apache.spark.sql.types.StructType()" +
    -        groupingKeySchema.map { key =>
    -          val keyName = ctx.addReferenceObj("keyName", key.name)
    -          key.dataType match {
    -            case d: DecimalType =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.createDecimalType(
    -                  |${d.precision}, ${d.scale}))""".stripMargin
    -            case _ =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.${key.dataType})"""
    -          }
    -        }.mkString("\n").concat(";")
    +    val generatedKeyColTypes = groupingKeySchema
    +      .zipWithIndex.map { case (t, i) => (s"_col$i", t.dataType) }
    +    val generatedKeySchemaTypes = generatedKeyColTypes
    +      .foldLeft(new StructType())((schema, colType) => schema.add(colType._1, colType._2))
    --- End diff --
    
    Ah, we do not need recreate the structure type. I think we do not need `.asNullable`.
    
    Furthermore, I think that we do not need `generatedAggBufferColTypes`. We need only # of its fields.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2477/
    Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95146 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95146/testReport)** for PR 22187 at commit [`81ef75a`](https://github.com/apache/spark/commit/81ef75a36a1c9dcd6922d2ec77393bc35389efd0).


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95144 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95144/testReport)** for PR 22187 at commit [`81ef75a`](https://github.com/apache/spark/commit/81ef75a36a1c9dcd6922d2ec77393bc35389efd0).


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2475/
    Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/22187
  
    retest this please


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95166 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95166/testReport)** for PR 22187 at commit [`aaa3944`](https://github.com/apache/spark/commit/aaa3944f20db36898d231d94281ed08995d1da0b).


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

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


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95146 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95146/testReport)** for PR 22187 at commit [`81ef75a`](https://github.com/apache/spark/commit/81ef75a36a1c9dcd6922d2ec77393bc35389efd0).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test FAILed.


---

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


[GitHub] spark pull request #22187: [SPARK-25178][SQL] change the generated code of t...

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

    https://github.com/apache/spark/pull/22187#discussion_r212062022
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/RowBasedHashMapGenerator.scala ---
    @@ -44,31 +44,19 @@ class RowBasedHashMapGenerator(
         groupingKeySchema, bufferSchema) {
     
       override protected def initializeAggregateHashMap(): String = {
    -    val generatedKeySchema: String =
    -      s"new org.apache.spark.sql.types.StructType()" +
    -        groupingKeySchema.map { key =>
    -          val keyName = ctx.addReferenceObj("keyName", key.name)
    -          key.dataType match {
    -            case d: DecimalType =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.createDecimalType(
    -                  |${d.precision}, ${d.scale}))""".stripMargin
    -            case _ =>
    -              s""".add($keyName, org.apache.spark.sql.types.DataTypes.${key.dataType})"""
    -          }
    -        }.mkString("\n").concat(";")
    +    val generatedKeyColTypes = groupingKeySchema
    +      .zipWithIndex.map { case (t, i) => (s"_col$i", t.dataType) }
    +    val generatedKeySchemaTypes = generatedKeyColTypes
    +      .foldLeft(new StructType())((schema, colType) => schema.add(colType._1, colType._2))
    --- End diff --
    
    How about:
    
    ```scala
    val generatedKeyFields = groupingKeySchema
      .zipWithIndex.map { case (t, i) => StructField(s"_col$i", t.dataType) }
    val generatedKeySchemaTypes = new StructType(generatedKeyFields)
    ```



---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/22187
  
    Yeah, I agreed with @rednaxelafx to directly ship the StructType objects looks like a better solution. +1 for that.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

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


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    **[Test build #95141 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95141/testReport)** for PR 22187 at commit [`81ef75a`](https://github.com/apache/spark/commit/81ef75a36a1c9dcd6922d2ec77393bc35389efd0).


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:

    https://github.com/apache/spark/pull/22187
  
    Thanks! merging to master.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] change the generated code of the keyS...

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

    https://github.com/apache/spark/pull/22187
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/2457/
    Test PASSed.


---

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


[GitHub] spark issue #22187: [SPARK-25178][SQL] Directly ship the StructType objects ...

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

    https://github.com/apache/spark/pull/22187
  
    Merged build finished. Test PASSed.


---

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