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

[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

GitHub user heary-cao opened a pull request:

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

    [SPARK-25974][SQL]Optimizes Generates bytecode for ordering based on the given order

    ## What changes were proposed in this pull request?
    
    Currently, when generates the code for ordering based on the given order, too many variables and assignment statements will be generated, which is not necessary. This PR will eliminate redundant variables. Optimizes Generates bytecode for ordering based on the given order.
    The generated code looks like:
    
    spark.range(1).selectExpr(
         "id as key",
         "(id & 1023) as value1",
    "cast(id & 1023 as double) as value2",
    "cast(id & 1023 as int) as value3"
    ).select("value1", "value2", "value3").orderBy("value1", "value2").collect()
    
    
    before PR(codegen size: 178)
    
    Generated Ordering by input[0, bigint, false] ASC NULLS FIRST,input[1, double, false] ASC NULLS FIRST:
    /* 001 */ public SpecificOrdering generate(Object[] references) {
    /* 002 */   return new SpecificOrdering(references);
    /* 003 */ }
    /* 004 */
    /* 005 */ class SpecificOrdering extends org.apache.spark.sql.catalyst.expressions.codegen.BaseOrdering {
    /* 006 */
    /* 007 */   private Object[] references;
    /* 008 */
    /* 009 */
    /* 010 */   public SpecificOrdering(Object[] references) {
    /* 011 */     this.references = references;
    /* 012 */
    /* 013 */   }
    /* 014 */
    /* 015 */   public int compare(InternalRow a, InternalRow b) {
    /* 016 */
    /* 017 */     InternalRow i = null;
    /* 018 */
    /* 019 */     i = a;
    /* 020 */     boolean isNullA_0;
    /* 021 */     long primitiveA_0;
    /* 022 */     {
    /* 023 */       long value_0 = i.getLong(0);
    /* 024 */       isNullA_0 = false;
    /* 025 */       primitiveA_0 = value_0;
    /* 026 */     }
    /* 027 */     i = b;
    /* 028 */     boolean isNullB_0;
    /* 029 */     long primitiveB_0;
    /* 030 */     {
    /* 031 */       long value_0 = i.getLong(0);
    /* 032 */       isNullB_0 = false;
    /* 033 */       primitiveB_0 = value_0;
    /* 034 */     }
    /* 035 */     if (isNullA_0 && isNullB_0) {
    /* 036 */       // Nothing
    /* 037 */     } else if (isNullA_0) {
    /* 038 */       return -1;
    /* 039 */     } else if (isNullB_0) {
    /* 040 */       return 1;
    /* 041 */     } else {
    /* 042 */       int comp = (primitiveA_0 > primitiveB_0 ? 1 : primitiveA_0 < primitiveB_0 ? -1 : 0);
    /* 043 */       if (comp != 0) {
    /* 044 */         return comp;
    /* 045 */       }
    /* 046 */     }
    /* 047 */
    /* 048 */     i = a;
    /* 049 */     boolean isNullA_1;
    /* 050 */     double primitiveA_1;
    /* 051 */     {
    /* 052 */       double value_1 = i.getDouble(1);
    /* 053 */       isNullA_1 = false;
    /* 054 */       primitiveA_1 = value_1;
    /* 055 */     }
    /* 056 */     i = b;
    /* 057 */     boolean isNullB_1;
    /* 058 */     double primitiveB_1;
    /* 059 */     {
    /* 060 */       double value_1 = i.getDouble(1);
    /* 061 */       isNullB_1 = false;
    /* 062 */       primitiveB_1 = value_1;
    /* 063 */     }
    /* 064 */     if (isNullA_1 && isNullB_1) {
    /* 065 */       // Nothing
    /* 066 */     } else if (isNullA_1) {
    /* 067 */       return -1;
    /* 068 */     } else if (isNullB_1) {
    /* 069 */       return 1;
    /* 070 */     } else {
    /* 071 */       int comp = org.apache.spark.util.Utils.nanSafeCompareDoubles(primitiveA_1, primitiveB_1);
    /* 072 */       if (comp != 0) {
    /* 073 */         return comp;
    /* 074 */       }
    /* 075 */     }
    /* 076 */
    /* 077 */
    /* 078 */     return 0;
    /* 079 */   }
    /* 080 */
    /* 081 */
    /* 082 */ }
    
    After PR(codegen size: 89)
    Generated Ordering by input[0, bigint, false] ASC NULLS FIRST,input[1, double, false] ASC NULLS FIRST:
    /* 001 */ public SpecificOrdering generate(Object[] references) {
    /* 002 */   return new SpecificOrdering(references);
    /* 003 */ }
    /* 004 */
    /* 005 */ class SpecificOrdering extends org.apache.spark.sql.catalyst.expressions.codegen.BaseOrdering {
    /* 006 */
    /* 007 */   private Object[] references;
    /* 008 */
    /* 009 */
    /* 010 */   public SpecificOrdering(Object[] references) {
    /* 011 */     this.references = references;
    /* 012 */
    /* 013 */   }
    /* 014 */
    /* 015 */   public int compare(InternalRow a, InternalRow b) {
    /* 016 */
    /* 017 */
    /* 018 */     long value_0 = a.getLong(0);
    /* 019 */     long value_2 = b.getLong(0);
    /* 020 */     if (false && false) {
    /* 021 */       // Nothing
    /* 022 */     } else if (false) {
    /* 023 */       return -1;
    /* 024 */     } else if (false) {
    /* 025 */       return 1;
    /* 026 */     } else {
    /* 027 */       int comp = (value_0 > value_2 ? 1 : value_0 < value_2 ? -1 : 0);
    /* 028 */       if (comp != 0) {
    /* 029 */         return comp;
    /* 030 */       }
    /* 031 */     }
    /* 032 */
    /* 033 */     double value_1 = a.getDouble(1);
    /* 034 */     double value_3 = b.getDouble(1);
    /* 035 */     if (false && false) {
    /* 036 */       // Nothing
    /* 037 */     } else if (false) {
    /* 038 */       return -1;
    /* 039 */     } else if (false) {
    /* 040 */       return 1;
    /* 041 */     } else {
    /* 042 */       int comp = org.apache.spark.util.Utils.nanSafeCompareDoubles(value_1, value_3);
    /* 043 */       if (comp != 0) {
    /* 044 */         return comp;
    /* 045 */       }
    /* 046 */     }
    /* 047 */
    /* 048 */
    /* 049 */     return 0;
    /* 050 */   }
    /* 051 */
    /* 052 */
    /* 053 */ }
    
    ## How was this patch tested?
    
    the existed test cases.


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

    $ git pull https://github.com/heary-cao/spark GenArrayData

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

    https://github.com/apache/spark/pull/22976.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 #22976
    
----
commit a42c8a70c68abed0a553b1e1232add4bbae079bb
Author: caoxuewen <ca...@...>
Date:   2018-11-08T10:24:17Z

    Optimizes Generates bytecode for ordering based on the given order

----


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    LGTM except one comment, cc @rednaxelafx


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    cc @cloud-fan, @maropu, @kiszk


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    **[Test build #98632 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98632/testReport)** for PR 22976 at commit [`5acf2a4`](https://github.com/apache/spark/commit/5acf2a44ef12b1af4457f07ff1bee6476c9b27d1).


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r232443230
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -133,7 +126,6 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
           returnType = "int",
           makeSplitFunction = { body =>
             s"""
    --- End diff --
    
    Would you update this to use `|` and `.stripMargin`?


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    +1 LGTM, thanks for making this improvement!


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    ok to test


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    gentle ping @rednaxelafx


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r233771269
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,62 +68,55 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +      ctx: CodegenContext,
    +      row: String,
    +      ordering: Seq[SortOrder]): Seq[ExprCode] = {
    +    ctx.INPUT_ROW = row
    +    // to use INPUT_ROW we must make sure currentVars is null
    +    ctx.currentVars = null
    --- End diff --
    
    I thought about making the same suggestion, but on a second thought it's fine to just leave it here as well. The overhead isn't gonna be high, but it's nice to see how the `ctx.INPUT_ROW` and `ctx.currentVars` interact in adjacent lines.
    
    So I'm okay either way (as-is or with @viirya 's suggestion)


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    LGTM


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    LGTM


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r231886019
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,57 +68,51 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +    ctx: CodegenContext,
    +    row: String,
    +    ordering: Seq[SortOrder]): Seq[ExprCode] = {
    +    ctx.INPUT_ROW = row
    +    ctx.currentVars = null
    +    ordering.map(_.child.genCode(ctx))
    +  }
    +
       /**
        * Generates the code for ordering based on the given order.
        */
       def genComparisons(ctx: CodegenContext, ordering: Seq[SortOrder]): String = {
         val oldInputRow = ctx.INPUT_ROW
         val oldCurrentVars = ctx.currentVars
    -    val inputRow = "i"
    -    ctx.INPUT_ROW = inputRow
         // to use INPUT_ROW we must make sure currentVars is null
         ctx.currentVars = null
    -
    -    val comparisons = ordering.map { order =>
    -      val eval = order.child.genCode(ctx)
    -      val asc = order.isAscending
    -      val isNullA = ctx.freshName("isNullA")
    -      val primitiveA = ctx.freshName("primitiveA")
    -      val isNullB = ctx.freshName("isNullB")
    -      val primitiveB = ctx.freshName("primitiveB")
    +    val rowAKeys = createOrderKeys(ctx, "a", ordering)
    +    val rowBKeys = createOrderKeys(ctx, "b", ordering)
    +    val comparisons = rowAKeys.zip(rowBKeys).zipWithIndex.map { case ((l, r), i) =>
    +      val dt = ordering(i).child.dataType
    +      val asc = ordering(i).isAscending
    +      val nullOrdering = ordering(i).nullOrdering
           s"""
    -          ${ctx.INPUT_ROW} = a;
    -          boolean $isNullA;
    -          ${CodeGenerator.javaType(order.child.dataType)} $primitiveA;
    -          {
    -            ${eval.code}
    -            $isNullA = ${eval.isNull};
    -            $primitiveA = ${eval.value};
    -          }
    -          ${ctx.INPUT_ROW} = b;
    -          boolean $isNullB;
    -          ${CodeGenerator.javaType(order.child.dataType)} $primitiveB;
    -          {
    -            ${eval.code}
    -            $isNullB = ${eval.isNull};
    -            $primitiveB = ${eval.value};
    -          }
    -          if ($isNullA && $isNullB) {
    +          ${l.code}
    +          ${r.code}
    +          if (${l.isNull} && ${r.isNull}) {
                 // Nothing
    -          } else if ($isNullA) {
    +          } else if (${l.isNull}) {
                 return ${
    -              order.nullOrdering match {
    -                case NullsFirst => "-1"
    -                case NullsLast => "1"
    -              }};
    -          } else if ($isNullB) {
    +        nullOrdering match {
    --- End diff --
    
    nit: indentation problem


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    **[Test build #98602 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98602/testReport)** for PR 22976 at commit [`a42c8a7`](https://github.com/apache/spark/commit/a42c8a70c68abed0a553b1e1232add4bbae079bb).
     * 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 issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    **[Test build #98632 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98632/testReport)** for PR 22976 at commit [`5acf2a4`](https://github.com/apache/spark/commit/5acf2a44ef12b1af4457f07ff1bee6476c9b27d1).
     * 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 pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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/22976#discussion_r232552336
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,62 +68,55 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +    ctx: CodegenContext,
    --- End diff --
    
    4 space identation


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

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


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    cc @cloud-fan @mgaido91 


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r233771918
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,62 +68,55 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +      ctx: CodegenContext,
    +      row: String,
    +      ordering: Seq[SortOrder]): Seq[ExprCode] = {
    +    ctx.INPUT_ROW = row
    +    // to use INPUT_ROW we must make sure currentVars is null
    +    ctx.currentVars = null
    --- End diff --
    
    Very agreed. :)


---

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


[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r233764857
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -133,30 +126,26 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
           returnType = "int",
           makeSplitFunction = { body =>
             s"""
    -          InternalRow ${ctx.INPUT_ROW} = null;  // Holds current row being evaluated.
    -          $body
    -          return 0;
    -        """
    +          |$body
    +          |return 0;
    +        """.stripMargin
           },
           foldFunctions = { funCalls =>
             funCalls.zipWithIndex.map { case (funCall, i) =>
               val comp = ctx.freshName("comp")
               s"""
    -            int $comp = $funCall;
    -            if ($comp != 0) {
    -              return $comp;
    -            }
    -          """
    +            |int $comp = $funCall;
    +            |if ($comp != 0) {
    +            |  return $comp;
    +            |}
    +          """.stripMargin
             }.mkString
           })
         ctx.currentVars = oldCurrentVars
         ctx.INPUT_ROW = oldInputRow
         // make sure INPUT_ROW is declared even if splitExpressions
         // returns an inlined block
    --- End diff --
    
    This comment is not invalid and can be removed.


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r233765313
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,62 +68,55 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +      ctx: CodegenContext,
    +      row: String,
    +      ordering: Seq[SortOrder]): Seq[ExprCode] = {
    +    ctx.INPUT_ROW = row
    +    // to use INPUT_ROW we must make sure currentVars is null
    +    ctx.currentVars = null
    --- End diff --
    
    nit: we only need one `ctx.currentVars = null`. Maybe move it out of this function and put it at previous position. 


---

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


[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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/22976#discussion_r233787377
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -133,30 +126,26 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
           returnType = "int",
           makeSplitFunction = { body =>
             s"""
    -          InternalRow ${ctx.INPUT_ROW} = null;  // Holds current row being evaluated.
    -          $body
    -          return 0;
    -        """
    +          |$body
    +          |return 0;
    +        """.stripMargin
           },
           foldFunctions = { funCalls =>
             funCalls.zipWithIndex.map { case (funCall, i) =>
               val comp = ctx.freshName("comp")
               s"""
    -            int $comp = $funCall;
    -            if ($comp != 0) {
    -              return $comp;
    -            }
    -          """
    +            |int $comp = $funCall;
    +            |if ($comp != 0) {
    +            |  return $comp;
    +            |}
    +          """.stripMargin
             }.mkString
           })
         ctx.currentVars = oldCurrentVars
         ctx.INPUT_ROW = oldInputRow
         // make sure INPUT_ROW is declared even if splitExpressions
         // returns an inlined block
    --- End diff --
    
    sorry didn't see this comment when merging. feel free to send a follow-up. thanks!


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98631/
    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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r232443205
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -154,7 +146,6 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         // make sure INPUT_ROW is declared even if splitExpressions
         // returns an inlined block
         s"""
    --- End diff --
    
    Can we use just `code`?


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r231885902
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,57 +68,51 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +    ctx: CodegenContext,
    +    row: String,
    +    ordering: Seq[SortOrder]): Seq[ExprCode] = {
    +    ctx.INPUT_ROW = row
    +    ctx.currentVars = null
    +    ordering.map(_.child.genCode(ctx))
    +  }
    +
       /**
        * Generates the code for ordering based on the given order.
        */
       def genComparisons(ctx: CodegenContext, ordering: Seq[SortOrder]): String = {
         val oldInputRow = ctx.INPUT_ROW
         val oldCurrentVars = ctx.currentVars
    -    val inputRow = "i"
    -    ctx.INPUT_ROW = inputRow
         // to use INPUT_ROW we must make sure currentVars is null
         ctx.currentVars = null
    --- End diff --
    
    Now, can we remove this line?


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    **[Test build #98631 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98631/testReport)** for PR 22976 at commit [`b07acdb`](https://github.com/apache/spark/commit/b07acdbb95b43f3cbfdf6c5c5e42dcab828937bc).
     * 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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    **[Test build #98612 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98612/testReport)** for PR 22976 at commit [`a42c8a7`](https://github.com/apache/spark/commit/a42c8a70c68abed0a553b1e1232add4bbae079bb).
     * 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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    **[Test build #98678 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98678/testReport)** for PR 22976 at commit [`e58dc14`](https://github.com/apache/spark/commit/e58dc14b14d482f458900eeb71f7663a14bafddd).
     * 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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    thanks, All 
    @cloud-fan, @rednaxelafx, @viirya, @kiszk


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    Two minor comments. LGTM


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98602/
    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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r232443266
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,57 +68,50 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +    ctx: CodegenContext,
    +    row: String,
    +    ordering: Seq[SortOrder]): Seq[ExprCode] = {
    +    ctx.INPUT_ROW = row
    +    // to use INPUT_ROW we must make sure currentVars is null
    +    ctx.currentVars = null
    +    ordering.map(_.child.genCode(ctx))
    +  }
    +
       /**
        * Generates the code for ordering based on the given order.
        */
       def genComparisons(ctx: CodegenContext, ordering: Seq[SortOrder]): String = {
         val oldInputRow = ctx.INPUT_ROW
         val oldCurrentVars = ctx.currentVars
    -    val inputRow = "i"
    -    ctx.INPUT_ROW = inputRow
    -    // to use INPUT_ROW we must make sure currentVars is null
    -    ctx.currentVars = null
    -
    -    val comparisons = ordering.map { order =>
    -      val eval = order.child.genCode(ctx)
    -      val asc = order.isAscending
    -      val isNullA = ctx.freshName("isNullA")
    -      val primitiveA = ctx.freshName("primitiveA")
    -      val isNullB = ctx.freshName("isNullB")
    -      val primitiveB = ctx.freshName("primitiveB")
    +    val rowAKeys = createOrderKeys(ctx, "a", ordering)
    +    val rowBKeys = createOrderKeys(ctx, "b", ordering)
    +    val comparisons = rowAKeys.zip(rowBKeys).zipWithIndex.map { case ((l, r), i) =>
    +      val dt = ordering(i).child.dataType
    +      val asc = ordering(i).isAscending
    +      val nullOrdering = ordering(i).nullOrdering
           s"""
    -          ${ctx.INPUT_ROW} = a;
    -          boolean $isNullA;
    -          ${CodeGenerator.javaType(order.child.dataType)} $primitiveA;
    -          {
    -            ${eval.code}
    -            $isNullA = ${eval.isNull};
    -            $primitiveA = ${eval.value};
    -          }
    -          ${ctx.INPUT_ROW} = b;
    -          boolean $isNullB;
    -          ${CodeGenerator.javaType(order.child.dataType)} $primitiveB;
    -          {
    -            ${eval.code}
    -            $isNullB = ${eval.isNull};
    -            $primitiveB = ${eval.value};
    -          }
    -          if ($isNullA && $isNullB) {
    +          ${l.code}
    --- End diff --
    
    Would you update this to use | and .stripMargin?


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

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


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r231886071
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -68,57 +68,51 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
         genComparisons(ctx, ordering)
       }
     
    +  /**
    +   * Creates the variables for ordering based on the given order.
    +   */
    +  private def createOrderKeys(
    +    ctx: CodegenContext,
    +    row: String,
    +    ordering: Seq[SortOrder]): Seq[ExprCode] = {
    +    ctx.INPUT_ROW = row
    +    ctx.currentVars = null
    +    ordering.map(_.child.genCode(ctx))
    +  }
    +
       /**
        * Generates the code for ordering based on the given order.
        */
       def genComparisons(ctx: CodegenContext, ordering: Seq[SortOrder]): String = {
         val oldInputRow = ctx.INPUT_ROW
         val oldCurrentVars = ctx.currentVars
    -    val inputRow = "i"
    -    ctx.INPUT_ROW = inputRow
         // to use INPUT_ROW we must make sure currentVars is null
         ctx.currentVars = null
    -
    -    val comparisons = ordering.map { order =>
    -      val eval = order.child.genCode(ctx)
    -      val asc = order.isAscending
    -      val isNullA = ctx.freshName("isNullA")
    -      val primitiveA = ctx.freshName("primitiveA")
    -      val isNullB = ctx.freshName("isNullB")
    -      val primitiveB = ctx.freshName("primitiveB")
    +    val rowAKeys = createOrderKeys(ctx, "a", ordering)
    +    val rowBKeys = createOrderKeys(ctx, "b", ordering)
    +    val comparisons = rowAKeys.zip(rowBKeys).zipWithIndex.map { case ((l, r), i) =>
    +      val dt = ordering(i).child.dataType
    +      val asc = ordering(i).isAscending
    +      val nullOrdering = ordering(i).nullOrdering
           s"""
    -          ${ctx.INPUT_ROW} = a;
    -          boolean $isNullA;
    -          ${CodeGenerator.javaType(order.child.dataType)} $primitiveA;
    -          {
    -            ${eval.code}
    -            $isNullA = ${eval.isNull};
    -            $primitiveA = ${eval.value};
    -          }
    -          ${ctx.INPUT_ROW} = b;
    -          boolean $isNullB;
    -          ${CodeGenerator.javaType(order.child.dataType)} $primitiveB;
    -          {
    -            ${eval.code}
    -            $isNullB = ${eval.isNull};
    -            $primitiveB = ${eval.value};
    -          }
    -          if ($isNullA && $isNullB) {
    +          ${l.code}
    +          ${r.code}
    +          if (${l.isNull} && ${r.isNull}) {
                 // Nothing
    -          } else if ($isNullA) {
    +          } else if (${l.isNull}) {
                 return ${
    -              order.nullOrdering match {
    -                case NullsFirst => "-1"
    -                case NullsLast => "1"
    -              }};
    -          } else if ($isNullB) {
    +        nullOrdering match {
    +          case NullsFirst => "-1"
    +          case NullsLast => "1"
    +        }};
    +          } else if (${r.isNull}) {
                 return ${
    -              order.nullOrdering match {
    -                case NullsFirst => "1"
    -                case NullsLast => "-1"
    -              }};
    +        nullOrdering match {
    --- End diff --
    
    ditto


---

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


[GitHub] spark pull request #22976: [SPARK-25974][SQL]Optimizes Generates bytecode fo...

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

    https://github.com/apache/spark/pull/22976#discussion_r232474090
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateOrdering.scala ---
    @@ -133,7 +126,6 @@ object GenerateOrdering extends CodeGenerator[Seq[SortOrder], Ordering[InternalR
           returnType = "int",
           makeSplitFunction = { body =>
             s"""
    --- End diff --
    
    thanks, I have update it.


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    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 #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    Can one of the admins verify this patch?


---

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


[GitHub] spark issue #22976: [SPARK-25974][SQL]Optimizes Generates bytecode for order...

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

    https://github.com/apache/spark/pull/22976
  
    **[Test build #98724 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98724/testReport)** for PR 22976 at commit [`cbfd0a8`](https://github.com/apache/spark/commit/cbfd0a831bbd8ac167f54e46cdb1fb5ce9f19868).
     * 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