[SPARK-21870][SQL] Split aggregation code into small functions #20965

maropu · 2018-04-03T05:24:55Z

What changes were proposed in this pull request?

This pr proposed to split aggregation code into small functions in HashAggregateExec. In #18810, we got performance regression if JVMs didn't compile too long functions. I checked and I found the codegen of HashAggregateExec frequently goes over the limit when a query has too many aggregate functions (e.g., q66 in TPCDS).

The current master places all the generated aggregation code in a single function. In this pr, I modified the code to assign an individual function for each aggregate function (e.g., SUM
and AVG). For example, in a query
SELECT SUM(a), AVG(a) FROM VALUES(1) t(a), the proposed code defines two functions
for SUM(a) and AVG(a) as follows;

generated code with this pr (https://gist.github.com/maropu/812990012bc967a78364be0fa793f559):

/* 173 */   private void agg_doConsume_0(InternalRow inputadapter_row_0, long agg_expr_0_0, boolean agg_exprIsNull_0_0, double agg_expr_1_0, boolean agg_exprIsNull_1_0, long agg_expr_2_0, boolean agg_exprIsNull_2_0) throws java.io.IOException {
/* 174 */     // do aggregate
/* 175 */     // common sub-expressions
/* 176 */
/* 177 */     // evaluate aggregate functions and update aggregation buffers
/* 178 */     agg_doAggregate_sum_0(agg_exprIsNull_0_0, agg_expr_0_0);
/* 179 */     agg_doAggregate_avg_0(agg_expr_1_0, agg_exprIsNull_1_0, agg_exprIsNull_2_0, agg_expr_2_0);
/* 180 */
/* 181 */   }
...
/* 071 */   private void agg_doAggregate_avg_0(double agg_expr_1_0, boolean agg_exprIsNull_1_0, boolean agg_exprIsNull_2_0, long agg_expr_2_0) throws java.io.IOException {
/* 072 */     // do aggregate for avg
/* 073 */     // evaluate aggregate function
/* 074 */     boolean agg_isNull_19 = true;
/* 075 */     double agg_value_19 = -1.0;
...
/* 114 */   private void agg_doAggregate_sum_0(boolean agg_exprIsNull_0_0, long agg_expr_0_0) throws java.io.IOException {
/* 115 */     // do aggregate for sum
/* 116 */     // evaluate aggregate function
/* 117 */     agg_agg_isNull_11_0 = true;
/* 118 */     long agg_value_11 = -1L;

generated code in the current master (https://gist.github.com/maropu/e9d772af2c98d8991a6a5f0af7841760)

/* 059 */   private void agg_doConsume_0(InternalRow localtablescan_row_0, int agg_expr_0_0) throws java.io.IOException {
/* 060 */     // do aggregate
/* 061 */     // common sub-expressions
/* 062 */     boolean agg_isNull_4 = false;
/* 063 */     long agg_value_4 = -1L;
/* 064 */     if (!false) {
/* 065 */       agg_value_4 = (long) agg_expr_0_0;
/* 066 */     }
/* 067 */     // evaluate aggregate function
/* 068 */     agg_agg_isNull_7_0 = true;
/* 069 */     long agg_value_7 = -1L;
/* 070 */     do {
/* 071 */       if (!agg_bufIsNull_0) {
/* 072 */         agg_agg_isNull_7_0 = false;
/* 073 */         agg_value_7 = agg_bufValue_0;
/* 074 */         continue;
/* 075 */       }
/* 076 */
/* 077 */       boolean agg_isNull_9 = false;
/* 078 */       long agg_value_9 = -1L;
/* 079 */       if (!false) {
/* 080 */         agg_value_9 = (long) 0;
/* 081 */       }
/* 082 */       if (!agg_isNull_9) {
/* 083 */         agg_agg_isNull_7_0 = false;
/* 084 */         agg_value_7 = agg_value_9;
/* 085 */         continue;
/* 086 */       }
/* 087 */
/* 088 */     } while (false);
/* 089 */
/* 090 */     long agg_value_6 = -1L;
/* 091 */
/* 092 */     agg_value_6 = agg_value_7 + agg_value_4;
/* 093 */     boolean agg_isNull_11 = true;
/* 094 */     double agg_value_11 = -1.0;
/* 095 */
/* 096 */     if (!agg_bufIsNull_1) {
/* 097 */       agg_agg_isNull_13_0 = true;
/* 098 */       double agg_value_13 = -1.0;
/* 099 */       do {
/* 100 */         boolean agg_isNull_14 = agg_isNull_4;
/* 101 */         double agg_value_14 = -1.0;
/* 102 */         if (!agg_isNull_4) {
/* 103 */           agg_value_14 = (double) agg_value_4;
/* 104 */         }
/* 105 */         if (!agg_isNull_14) {
/* 106 */           agg_agg_isNull_13_0 = false;
/* 107 */           agg_value_13 = agg_value_14;
/* 108 */           continue;
/* 109 */         }
/* 110 */
/* 111 */         boolean agg_isNull_15 = false;
/* 112 */         double agg_value_15 = -1.0;
/* 113 */         if (!false) {
/* 114 */           agg_value_15 = (double) 0;
/* 115 */         }
/* 116 */         if (!agg_isNull_15) {
/* 117 */           agg_agg_isNull_13_0 = false;
/* 118 */           agg_value_13 = agg_value_15;
/* 119 */           continue;
/* 120 */         }
/* 121 */
/* 122 */       } while (false);
/* 123 */
/* 124 */       agg_isNull_11 = false; // resultCode could change nullability.
/* 125 */
/* 126 */       agg_value_11 = agg_bufValue_1 + agg_value_13;
/* 127 */
/* 128 */     }
/* 129 */     boolean agg_isNull_17 = false;
/* 130 */     long agg_value_17 = -1L;
/* 131 */     if (!false && agg_isNull_4) {
/* 132 */       agg_isNull_17 = agg_bufIsNull_2;
/* 133 */       agg_value_17 = agg_bufValue_2;
/* 134 */     } else {
/* 135 */       boolean agg_isNull_20 = true;
/* 136 */       long agg_value_20 = -1L;
/* 137 */
/* 138 */       if (!agg_bufIsNull_2) {
/* 139 */         agg_isNull_20 = false; // resultCode could change nullability.
/* 140 */
/* 141 */         agg_value_20 = agg_bufValue_2 + 1L;
/* 142 */
/* 143 */       }
/* 144 */       agg_isNull_17 = agg_isNull_20;
/* 145 */       agg_value_17 = agg_value_20;
/* 146 */     }
/* 147 */     // update aggregation buffer
/* 148 */     agg_bufIsNull_0 = false;
/* 149 */     agg_bufValue_0 = agg_value_6;
/* 150 */
/* 151 */     agg_bufIsNull_1 = agg_isNull_11;
/* 152 */     agg_bufValue_1 = agg_value_11;
/* 153 */
/* 154 */     agg_bufIsNull_2 = agg_isNull_17;
/* 155 */     agg_bufValue_2 = agg_value_17;
/* 156 */
/* 157 */   }

You can check the previous discussion in #19082

How was this patch tested?

Existing tests

maropu · 2018-04-03T05:27:51Z

Since SPARK-23791 gave me a heads-up, I submitted again for reviews.

mgaido91 · 2018-04-03T14:22:49Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

I think this is not needed since we can use CodeGenerator.calculateParamLength

maropu · 2018-04-03T14:34:13Z

retest this please

SparkQA · 2018-04-03T19:01:23Z

Test build #88840 has finished for PR 20965 at commit 289b6d3.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-04-04T07:05:01Z

Test build #88876 has finished for PR 20965 at commit 696ba17.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-04-04T07:05:02Z

Test build #88875 has finished for PR 20965 at commit 9623765.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2018-04-04T07:57:07Z

retest this please

SparkQA · 2018-04-04T09:48:39Z

Test build #88881 has finished for PR 20965 at commit 696ba17.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2018-04-04T09:10:22Z

...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala

Just put those into object CodeGenerator?

SparkQA · 2018-04-05T01:33:17Z

Test build #88917 has finished for PR 20965 at commit b96aa8d.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-04-05T05:06:04Z

Test build #88918 has finished for PR 20965 at commit 640fd8f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2018-04-05T05:12:08Z

@mgaido91 @viirya Could you check again? Thanks!

mgaido91 · 2018-04-05T12:39:40Z

sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala

is there any reason why we are not using CodeGenerator.splitExpressions instead of this?

that's true and I'll try to fix

aha, I remember; Since this splitting needed to consider subexpression elimination, I added this specific splitting function for HashAggregateExec here.

I see, can't we add instead a splitExpressionsWithSubexprElimination or something similar to CodeGenerator instead, so that it can be reused if needed somewhere else?

ok, I'll recheck.

mgaido91 · 2018-04-05T12:42:34Z

sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala

why is this needed now and it was not before?

If we don't have these copies, some queries return wrong answers, e.g.,

import org.apache.spark.sql.execution.debug._ sql("CREATE OR REPLACE TEMPORARY VIEW testData AS SELECT * FROM VALUES (1, 1), (1, 2), (2, 1) AS testData(a, b)") val df = sql("SELECT SKEWNESS(a) FROM testData") df.debugCodegen df.show scala> df.show (copy) +---------------------------+ |skewness(CAST(a AS DOUBLE))| +---------------------------+ | 0.7071067811865475| +---------------------------+ scala> df.show (non-copy) +---------------------------+ |skewness(CAST(a AS DOUBLE))| +---------------------------+ | -1.368454115659954| +---------------------------+

In the original gen'd code, aggregation buffer updates happen in the end of an Aggregate consume;
https://gist.github.com/maropu/ec5a322c40e9e7e0024c3074a261197b#file-codegen-d-skew-in-the-master-L807

But, if we split aggregation functions into pieces, these updates happen in the end of each split function, e.g.,;
https://gist.github.com/maropu/1e9e7cb2377622549163261fc321a108#file-codegen-d-skew-in-spark-21870-2-local-copy-L822

So, if we don't copy the previous state to local buffers, the aggregation wrongly references the updated state and then returns an incorrect answer.

but this may introduce a perf regression, doesn't it? in this case I am not sure if we should do it or not, WDYT @cloud-fan @gatorsmile @hvanhovell ?

mgaido91 · 2018-04-05T12:43:07Z

sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala

hopefully this method will become easier after @viirya's PR gets merged. Anyway, this seems to me quite a generic method: shall we move it to CodeGenerator?

gatorsmile · 2018-04-06T15:39:41Z

cc @hvanhovell @rednaxelafx @cloud-fan

SparkQA · 2018-04-19T10:47:06Z

Test build #89556 has finished for PR 20965 at commit 6cfc8d4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-04-19T11:01:46Z

Test build #89557 has finished for PR 20965 at commit f2cd5ca.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2018-08-28T07:12:10Z

oh, made a mistake ...

SparkQA · 2018-08-28T11:15:49Z

Test build #95334 has finished for PR 20965 at commit 7564d32.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2019-09-04T09:09:30Z

Thanks for the check, @rednaxelafx ! As @cloud-fan said, I think that code length/bytecode mismatches are our well-known issue...

@cloud-fan Yea, sure. we must check that before merging this.

SparkQA · 2019-09-04T10:42:43Z

Test build #110103 has finished for PR 20965 at commit d1e06a6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya

Looks good now. Just found a style problem.

viirya · 2019-09-04T15:41:39Z

sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala

+          aggCodeBlocks.fold(EmptyBlock)(_ + _).code
+        }
+      } else {
+        aggCodeBlocks.fold(EmptyBlock)(_ + _).code


As conf.codegenSplitAggregateFunc is enabled by default, can we run jenkins test after disabling it? We can enable it before merging.

Yea, reasonable suggestion. I'll try.

viirya · 2019-09-04T15:46:27Z

sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala

+            val bufferOffset = bufferStartOffsets(i)
+            // All the update code for aggregation buffers should be placed in the end
+            // of each aggregation function code.
+             val updateRowBuffer = fastRowEvalsForOneFunc.zipWithIndex.map { case (ev, j) =>


style: ident seems wrong here.

maropu · 2019-09-05T12:44:57Z

To check if the tests passed with splitAggregateFunc.enabled=false, I turned it off temporarily in the latest commit.

maropu · 2019-09-05T12:55:57Z

I checked the TPCDS performance numbers (sf=5) and I couldn't find much difference with/without this pr: https://docs.google.com/spreadsheets/d/10eCV0PHeAaWGaXwpKPDDYCSLzsogl4IIiGiitqvw8PY/edit?usp=sharing
Rather, the numbers in q66 didn't change between them. Probably, the other PRs already have improved it (I didn't dig into it though).

To make sure this pr is still beneficial, I run microbenchmarks below;

$ ./bin/spark-shell --master=local[1] --conf spark.driver.memory=8g --conf spark.sql.shuffle.partitions=1 -v

scala> val numCols = 50
scala> val colExprs = (0 until numCols).map { i => s"id AS _c$i" }
scala> spark.range(5000000).selectExpr(colExprs: _*).createOrReplaceTempView("t")

scala> val aggExprs = (0 until numCols).map { i => s"AVG(_c$i)" }

scala> sql("SET spark.sql.codegen.aggregate.splitAggregateFunc.enabled=true")
scala> timer { sql(s"SELECT ${aggExprs.mkString(", ")} FROM t").write.format("noop").save() }
Elapsed time: 0.997808995s                                                      

scala> sql("SET spark.sql.codegen.aggregate.splitAggregateFunc.enabled=false")
scala> timer { sql(s"SELECT ${aggExprs.mkString(", ")} FROM t").write.format("noop").save() }
Elapsed time: 25.77200574s

SparkQA · 2019-09-05T16:59:37Z

Test build #110183 has finished for PR 20965 at commit 20b310f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

This reverts commit 20b310f.

maropu · 2019-09-05T21:11:29Z

@viirya ok, all the tests passed with splitAggregateFunc.enabled=false.

maropu · 2019-09-05T21:49:01Z

@viirya oh, sorry. I updated the link so that everyone can see it: https://docs.google.com/spreadsheets/d/10eCV0PHeAaWGaXwpKPDDYCSLzsogl4IIiGiitqvw8PY/edit?usp=sharing

viirya · 2019-09-05T22:15:57Z

@maropu Thanks! :)

SparkQA · 2019-09-06T00:52:47Z

Test build #110201 has finished for PR 20965 at commit ab8de83.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2019-09-06T00:55:05Z

ok, I think all the things get fine. @cloud-fan @viirya

cloud-fan · 2019-09-06T03:45:30Z

thanks, merging to master!

maropu · 2019-09-06T03:50:40Z

Thanks for all the reviewers!

…nerated code in HashAggregateExec ### What changes were proposed in this pull request? This pr cleans up string template formats for generated code in HashAggregateExec. This changes comes from rednaxelafx comment: #20965 (comment) ### Why are the changes needed? To improve code-readability. ### Does this PR introduce any user-facing change? No ### How was this patch tested? N/A Closes #25714 from maropu/SPARK-21870-FOLLOWUP. Authored-by: Takeshi Yamamuro <yamamuro@apache.org> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>

## What changes were proposed in this pull request? This pr proposed to split aggregation code into small functions in `HashAggregateExec`. In apache#18810, we got performance regression if JVMs didn't compile too long functions. I checked and I found the codegen of `HashAggregateExec` frequently goes over the limit when a query has too many aggregate functions (e.g., q66 in TPCDS). The current master places all the generated aggregation code in a single function. In this pr, I modified the code to assign an individual function for each aggregate function (e.g., `SUM` and `AVG`). For example, in a query `SELECT SUM(a), AVG(a) FROM VALUES(1) t(a)`, the proposed code defines two functions for `SUM(a)` and `AVG(a)` as follows; - generated code with this pr (https://gist.github.com/maropu/812990012bc967a78364be0fa793f559): ``` /* 173 */ private void agg_doConsume_0(InternalRow inputadapter_row_0, long agg_expr_0_0, boolean agg_exprIsNull_0_0, double agg_expr_1_0, boolean agg_exprIsNull_1_0, long agg_expr_2_0, boolean agg_exprIsNull_2_0) throws java.io.IOException { /* 174 */ // do aggregate /* 175 */ // common sub-expressions /* 176 */ /* 177 */ // evaluate aggregate functions and update aggregation buffers /* 178 */ agg_doAggregate_sum_0(agg_exprIsNull_0_0, agg_expr_0_0); /* 179 */ agg_doAggregate_avg_0(agg_expr_1_0, agg_exprIsNull_1_0, agg_exprIsNull_2_0, agg_expr_2_0); /* 180 */ /* 181 */ } ... /* 071 */ private void agg_doAggregate_avg_0(double agg_expr_1_0, boolean agg_exprIsNull_1_0, boolean agg_exprIsNull_2_0, long agg_expr_2_0) throws java.io.IOException { /* 072 */ // do aggregate for avg /* 073 */ // evaluate aggregate function /* 074 */ boolean agg_isNull_19 = true; /* 075 */ double agg_value_19 = -1.0; ... /* 114 */ private void agg_doAggregate_sum_0(boolean agg_exprIsNull_0_0, long agg_expr_0_0) throws java.io.IOException { /* 115 */ // do aggregate for sum /* 116 */ // evaluate aggregate function /* 117 */ agg_agg_isNull_11_0 = true; /* 118 */ long agg_value_11 = -1L; ``` - generated code in the current master (https://gist.github.com/maropu/e9d772af2c98d8991a6a5f0af7841760) ``` /* 059 */ private void agg_doConsume_0(InternalRow localtablescan_row_0, int agg_expr_0_0) throws java.io.IOException { /* 060 */ // do aggregate /* 061 */ // common sub-expressions /* 062 */ boolean agg_isNull_4 = false; /* 063 */ long agg_value_4 = -1L; /* 064 */ if (!false) { /* 065 */ agg_value_4 = (long) agg_expr_0_0; /* 066 */ } /* 067 */ // evaluate aggregate function /* 068 */ agg_agg_isNull_7_0 = true; /* 069 */ long agg_value_7 = -1L; /* 070 */ do { /* 071 */ if (!agg_bufIsNull_0) { /* 072 */ agg_agg_isNull_7_0 = false; /* 073 */ agg_value_7 = agg_bufValue_0; /* 074 */ continue; /* 075 */ } /* 076 */ /* 077 */ boolean agg_isNull_9 = false; /* 078 */ long agg_value_9 = -1L; /* 079 */ if (!false) { /* 080 */ agg_value_9 = (long) 0; /* 081 */ } /* 082 */ if (!agg_isNull_9) { /* 083 */ agg_agg_isNull_7_0 = false; /* 084 */ agg_value_7 = agg_value_9; /* 085 */ continue; /* 086 */ } /* 087 */ /* 088 */ } while (false); /* 089 */ /* 090 */ long agg_value_6 = -1L; /* 091 */ /* 092 */ agg_value_6 = agg_value_7 + agg_value_4; /* 093 */ boolean agg_isNull_11 = true; /* 094 */ double agg_value_11 = -1.0; /* 095 */ /* 096 */ if (!agg_bufIsNull_1) { /* 097 */ agg_agg_isNull_13_0 = true; /* 098 */ double agg_value_13 = -1.0; /* 099 */ do { /* 100 */ boolean agg_isNull_14 = agg_isNull_4; /* 101 */ double agg_value_14 = -1.0; /* 102 */ if (!agg_isNull_4) { /* 103 */ agg_value_14 = (double) agg_value_4; /* 104 */ } /* 105 */ if (!agg_isNull_14) { /* 106 */ agg_agg_isNull_13_0 = false; /* 107 */ agg_value_13 = agg_value_14; /* 108 */ continue; /* 109 */ } /* 110 */ /* 111 */ boolean agg_isNull_15 = false; /* 112 */ double agg_value_15 = -1.0; /* 113 */ if (!false) { /* 114 */ agg_value_15 = (double) 0; /* 115 */ } /* 116 */ if (!agg_isNull_15) { /* 117 */ agg_agg_isNull_13_0 = false; /* 118 */ agg_value_13 = agg_value_15; /* 119 */ continue; /* 120 */ } /* 121 */ /* 122 */ } while (false); /* 123 */ /* 124 */ agg_isNull_11 = false; // resultCode could change nullability. /* 125 */ /* 126 */ agg_value_11 = agg_bufValue_1 + agg_value_13; /* 127 */ /* 128 */ } /* 129 */ boolean agg_isNull_17 = false; /* 130 */ long agg_value_17 = -1L; /* 131 */ if (!false && agg_isNull_4) { /* 132 */ agg_isNull_17 = agg_bufIsNull_2; /* 133 */ agg_value_17 = agg_bufValue_2; /* 134 */ } else { /* 135 */ boolean agg_isNull_20 = true; /* 136 */ long agg_value_20 = -1L; /* 137 */ /* 138 */ if (!agg_bufIsNull_2) { /* 139 */ agg_isNull_20 = false; // resultCode could change nullability. /* 140 */ /* 141 */ agg_value_20 = agg_bufValue_2 + 1L; /* 142 */ /* 143 */ } /* 144 */ agg_isNull_17 = agg_isNull_20; /* 145 */ agg_value_17 = agg_value_20; /* 146 */ } /* 147 */ // update aggregation buffer /* 148 */ agg_bufIsNull_0 = false; /* 149 */ agg_bufValue_0 = agg_value_6; /* 150 */ /* 151 */ agg_bufIsNull_1 = agg_isNull_11; /* 152 */ agg_bufValue_1 = agg_value_11; /* 153 */ /* 154 */ agg_bufIsNull_2 = agg_isNull_17; /* 155 */ agg_bufValue_2 = agg_value_17; /* 156 */ /* 157 */ } ``` You can check the previous discussion in apache#19082 ## How was this patch tested? Existing tests Closes apache#20965 from maropu/SPARK-21870-2. Authored-by: Takeshi Yamamuro <yamamuro@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…nerated code in HashAggregateExec ### What changes were proposed in this pull request? This pr cleans up string template formats for generated code in HashAggregateExec. This changes comes from rednaxelafx comment: apache#20965 (comment) ### Why are the changes needed? To improve code-readability. ### Does this PR introduce any user-facing change? No ### How was this patch tested? N/A Closes apache#25714 from maropu/SPARK-21870-FOLLOWUP. Authored-by: Takeshi Yamamuro <yamamuro@apache.org> Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>

mgaido91 reviewed Apr 3, 2018

View reviewed changes

maropu force-pushed the SPARK-21870-2 branch 2 times, most recently from ae61eb9 to 696ba17 Compare April 4, 2018 06:05

viirya reviewed Apr 4, 2018

View reviewed changes

maropu force-pushed the SPARK-21870-2 branch 4 times, most recently from ee8a68f to 640fd8f Compare April 5, 2018 01:30

maropu mentioned this pull request Apr 5, 2018

[SPARK-22856][SQL] Add wrappers for codegen output and nullability #20043

Closed

mgaido91 reviewed Apr 5, 2018

View reviewed changes

maropu force-pushed the SPARK-21870-2 branch 3 times, most recently from 63f6e90 to f2cd5ca Compare April 19, 2018 07:13

maropu force-pushed the SPARK-21870-2 branch from f2cd5ca to 7564d32 Compare August 28, 2018 07:10

maropu force-pushed the SPARK-21870-2 branch from 7564d32 to f617623 Compare August 28, 2018 07:53

cloud-fan mentioned this pull request Sep 4, 2019

[SPARK-28916][SQL] Split subexpression elimination functions code for Generate[Mutable|Unsafe]Projection #25642

Closed

viirya approved these changes Sep 4, 2019

View reviewed changes

maropu added 2 commits September 5, 2019 21:41

Fix style issue

5088939

Trun off the split mode temporarily for Jenkins tests

20b310f

Revert "Trun off the split mode temporarily for Jenkins tests"

ab8de83

This reverts commit 20b310f.

cloud-fan closed this in cb0cddf Sep 6, 2019

maropu mentioned this pull request Sep 6, 2019

[SPARK-21870][SQL][FOLLOW-UP] Clean up string template formats for generated code in HashAggregateExec #25714

Closed

[SPARK-21870][SQL] Split aggregation code into small functions #20965

[SPARK-21870][SQL] Split aggregation code into small functions #20965

Uh oh!

Conversation

maropu commented Apr 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

maropu commented Apr 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu commented Apr 3, 2018

Uh oh!

SparkQA commented Apr 3, 2018

Uh oh!

SparkQA commented Apr 4, 2018

Uh oh!

SparkQA commented Apr 4, 2018

Uh oh!

maropu commented Apr 4, 2018

Uh oh!

SparkQA commented Apr 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 5, 2018

Uh oh!

SparkQA commented Apr 5, 2018

Uh oh!

maropu commented Apr 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gatorsmile commented Apr 6, 2018

Uh oh!

SparkQA commented Apr 19, 2018

Uh oh!

SparkQA commented Apr 19, 2018

Uh oh!

maropu commented Aug 28, 2018

Uh oh!

SparkQA commented Aug 28, 2018

Uh oh!

maropu commented Sep 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Sep 4, 2019

Uh oh!

viirya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

maropu commented Apr 3, 2018 •

edited

Loading

maropu commented Sep 4, 2019 •

edited

Loading

maropu commented Sep 5, 2019 •

edited

Loading