[SPARK-22362][SQL] Add unit test for Window Aggregate Functions #20046

attilapiros · 2017-12-21T16:49:04Z

What changes were proposed in this pull request?

Improving the test coverage of window functions focusing on missing test for window aggregate functions. No new UDAF test is added as it has been tested already.

How was this patch tested?

Only new tests were added, automated tests were executed.

smurakozi · 2017-12-21T16:50:43Z

sql/core/src/test/scala/org/apache/spark/sql/DataFrameWindowFunctionsSuite.scala

+    val e = intercept[AnalysisException](
+      df.select(
+        $"key",
+        count("invalid").over(


over() would be enough, as partition and orderBy is ignored anyway

Thanks, I will remove the unnecessary parts.

smurakozi

You could have tests attempting to feed aggregation functions with columns containing wrong datatypes (e.g. avg on String)

squito · 2017-12-21T19:08:39Z

Jenkins, ok to test

SparkQA · 2017-12-21T21:48:11Z

Test build #85276 has finished for PR 20046 at commit 7b73f57.

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

SparkQA · 2017-12-22T15:31:39Z

Test build #85310 has finished for PR 20046 at commit 00f50c0.

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

SparkQA · 2017-12-22T15:39:43Z

Test build #85311 has finished for PR 20046 at commit 7290ec4.

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

SparkQA · 2018-01-02T17:24:30Z

Test build #85599 has finished for PR 20046 at commit 4dd08dc.

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

SparkQA · 2018-01-02T17:42:37Z

Test build #85601 has finished for PR 20046 at commit 03d6159.

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

SparkQA · 2018-01-02T20:59:20Z

Test build #85610 has finished for PR 20046 at commit caafba7.

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

attilapiros · 2018-01-03T11:54:11Z

cc @gatorsmile @hvanhovell @jiangxb1987

SparkQA · 2018-01-18T22:06:17Z

Test build #86352 has finished for PR 20046 at commit adab4da.

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

jiangxb1987 · 2018-01-18T23:16:42Z

retest this please

SparkQA · 2018-01-19T02:30:20Z

Test build #86362 has finished for PR 20046 at commit adab4da.

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

jiangxb1987 · 2018-01-19T23:48:50Z

sql/core/src/test/scala/org/apache/spark/sql/DataFrameWindowFunctionsSuite.scala

@@ -86,6 +93,429 @@ class DataFrameWindowFunctionsSuite extends QueryTest with SharedSQLContext {
    assert(e.message.contains("requires window to be ordered"))
  }

+  test("aggregation and rows between") {
+    val df = Seq((1, "1"), (2, "1"), (2, "2"), (1, "1"), (2, "2")).toDF("key", "value")


We shall also include null data.

This tests was removed and re-added as result of merge conflict. Now I cleaned up.

jiangxb1987 · 2018-01-19T23:55:45Z

We shall also cover the sql interface, you can find some example in sql/core/src/test/resources/sql-tests/inputs/udaf.sql

smurakozi · 2018-01-22T12:27:09Z

@jiangxb1987 how does your request to cover the sql interface relates to SPARK-23160?
I assume it is to be covered in that issue, is that correct?

attilapiros · 2018-01-22T12:33:09Z

I have already extended sql/core/src/test/resources/sql-tests/inputs/window.sql with the missing window aggregate functions but if you would like I can move it to a different PR too.

SparkQA · 2018-01-22T15:39:27Z

Test build #86474 has finished for PR 20046 at commit 458a0cc.

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

attilapiros · 2018-01-22T16:01:32Z

PySpark failure must be unrelated as only unit tests are added.

SparkQA · 2018-01-22T16:13:55Z

Test build #86472 has finished for PR 20046 at commit 5c941c7.

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

SparkQA · 2018-01-22T16:14:12Z

Test build #86476 has finished for PR 20046 at commit a0e14cc.

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

attilapiros · 2018-02-28T00:43:02Z

cc @gatorsmile @hvanhovell

@jiangxb1987 your review comments are applied, is there something else I should work on regarding this PR?

attilapiros · 2018-03-16T17:18:40Z

gentle ping @gatorsmile @hvanhovell

SparkQA · 2018-04-09T18:36:18Z

Test build #89066 has finished for PR 20046 at commit 93aa930.

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

attilapiros · 2018-04-09T18:40:39Z

ping @hvanhovell @cloud-fan

hvanhovell

A few comments. Looks good overall.

hvanhovell · 2018-02-28T09:50:12Z

sql/core/src/test/scala/org/apache/spark/sql/DataFrameWindowFunctionsSuite.scala

@@ -33,6 +35,11 @@ import org.apache.spark.unsafe.types.CalendarInterval
 class DataFrameWindowFunctionsSuite extends QueryTest with SharedSQLContext {
  import testImplicits._

+  private def sortWrappedArrayInRow(d: DataFrame) = d.map {
+      case Row(key: String, unsorted: mutable.WrappedArray[String]) =>


Let's not pattern match against mutable.WrappedArray and use Seq instead. mutable.WrappedArray is pretty much an implementation detail, and pattern matching against it is brittle.

hvanhovell · 2018-02-28T09:51:09Z

sql/core/src/test/scala/org/apache/spark/sql/DataFrameWindowFunctionsSuite.scala

@@ -33,6 +35,11 @@ import org.apache.spark.unsafe.types.CalendarInterval
 class DataFrameWindowFunctionsSuite extends QueryTest with SharedSQLContext {
  import testImplicits._

+  private def sortWrappedArrayInRow(d: DataFrame) = d.map {


You can also just use the array_sort function. That is probably a lot cheaper.

SparkQA · 2018-04-10T12:05:46Z

Test build #89100 has finished for PR 20046 at commit 9baef9c.

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

attilapiros · 2018-04-13T15:26:20Z

gentle reminder @hvanhovell

attilapiros · 2018-04-18T16:05:25Z

@hvanhovell I would like to ask you to take another quick glance to these change

hvanhovell · 2018-04-19T16:55:19Z

LGTM - merging to master. Thanks!

smurakozi reviewed Dec 21, 2017

View reviewed changes

attilapiros changed the title ~~Adding unit tests for missing aggregate functions~~ [SPARK-22362][SQL] Add unit test for Window Aggregate Functions Dec 21, 2017

attilapiros added 3 commits January 2, 2018 14:56

Adding unit tests for missing aggregate functions

a077a18

Applying review comments.

7a77b67

Renaming a test

03d6159

attilapiros force-pushed the SPARK-22362 branch from 4dd08dc to 03d6159 Compare January 2, 2018 14:46

Sorting WrappedArrays within Rows before compare.

caafba7

Merge branch 'master' into SPARK-22362

adab4da

jiangxb1987 reviewed Jan 19, 2018

View reviewed changes

attilapiros added 4 commits January 22, 2018 10:45

Merge branch 'master' into SPARK-22362

68ae39b

Remove readded tests (result of merge conflict)

5c941c7

Remove readded tests (result of merge conflict)

458a0cc

Adding SQL tests for missing window functions

a0e14cc

Merge branch 'master' into SPARK-22362

93aa930

hvanhovell reviewed Apr 9, 2018

View reviewed changes

applying review comments

9baef9c

asfgit closed this in 9ea8d3d Apr 19, 2018

attilapiros deleted the SPARK-22362 branch April 26, 2018 20:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-22362][SQL] Add unit test for Window Aggregate Functions #20046

[SPARK-22362][SQL] Add unit test for Window Aggregate Functions #20046

attilapiros commented Dec 21, 2017

smurakozi Dec 21, 2017

attilapiros Dec 22, 2017

smurakozi left a comment

squito commented Dec 21, 2017

SparkQA commented Dec 21, 2017

SparkQA commented Dec 22, 2017

SparkQA commented Dec 22, 2017

SparkQA commented Jan 2, 2018

SparkQA commented Jan 2, 2018

SparkQA commented Jan 2, 2018

attilapiros commented Jan 3, 2018

SparkQA commented Jan 18, 2018

jiangxb1987 commented Jan 18, 2018

SparkQA commented Jan 19, 2018

jiangxb1987 Jan 19, 2018

attilapiros Jan 22, 2018

jiangxb1987 commented Jan 19, 2018

smurakozi commented Jan 22, 2018 •

edited

Loading

attilapiros commented Jan 22, 2018

SparkQA commented Jan 22, 2018

attilapiros commented Jan 22, 2018

SparkQA commented Jan 22, 2018

SparkQA commented Jan 22, 2018

attilapiros commented Feb 28, 2018

attilapiros commented Mar 16, 2018

SparkQA commented Apr 9, 2018

attilapiros commented Apr 9, 2018

hvanhovell left a comment

hvanhovell Feb 28, 2018

hvanhovell Feb 28, 2018

SparkQA commented Apr 10, 2018

attilapiros commented Apr 13, 2018

attilapiros commented Apr 18, 2018

hvanhovell commented Apr 19, 2018

[SPARK-22362][SQL] Add unit test for Window Aggregate Functions #20046

[SPARK-22362][SQL] Add unit test for Window Aggregate Functions #20046

Conversation

attilapiros commented Dec 21, 2017

What changes were proposed in this pull request?

How was this patch tested?

smurakozi Dec 21, 2017

Choose a reason for hiding this comment

attilapiros Dec 22, 2017

Choose a reason for hiding this comment

smurakozi left a comment

Choose a reason for hiding this comment

squito commented Dec 21, 2017

SparkQA commented Dec 21, 2017

SparkQA commented Dec 22, 2017

SparkQA commented Dec 22, 2017

SparkQA commented Jan 2, 2018

SparkQA commented Jan 2, 2018

SparkQA commented Jan 2, 2018

attilapiros commented Jan 3, 2018

SparkQA commented Jan 18, 2018

jiangxb1987 commented Jan 18, 2018

SparkQA commented Jan 19, 2018

jiangxb1987 Jan 19, 2018

Choose a reason for hiding this comment

attilapiros Jan 22, 2018

Choose a reason for hiding this comment

jiangxb1987 commented Jan 19, 2018

smurakozi commented Jan 22, 2018 • edited Loading

attilapiros commented Jan 22, 2018

SparkQA commented Jan 22, 2018

attilapiros commented Jan 22, 2018

SparkQA commented Jan 22, 2018

SparkQA commented Jan 22, 2018

attilapiros commented Feb 28, 2018

attilapiros commented Mar 16, 2018

SparkQA commented Apr 9, 2018

attilapiros commented Apr 9, 2018

hvanhovell left a comment

Choose a reason for hiding this comment

hvanhovell Feb 28, 2018

Choose a reason for hiding this comment

hvanhovell Feb 28, 2018

Choose a reason for hiding this comment

SparkQA commented Apr 10, 2018

attilapiros commented Apr 13, 2018

attilapiros commented Apr 18, 2018

hvanhovell commented Apr 19, 2018

smurakozi commented Jan 22, 2018 •

edited

Loading