Skip to content

Conversation

@DonnyZone
Copy link
Contributor

What changes were proposed in this pull request?

This is a follow-up PR that moves the test case in PR-18920 (#18920) to DataFrameAggregateSuit.

How was this patch tested?

unit test

@DonnyZone
Copy link
Contributor Author

@gatorsmile

@gatorsmile
Copy link
Member

ok to test

import org.apache.spark.sql.test.SQLTestData.DecimalData
import org.apache.spark.sql.types.{Decimal, DecimalType}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this line. Thanks!

Seq(Row(3, 4, 9)))
}

private def assertNoExceptions(c: Column): Unit = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move it to line 566, just before test("SPARK-21580 ints in aggregation expressions are taken as group-by ordinal.")? Thanks!

@gatorsmile
Copy link
Member

LGTM except two minor comments.

@SparkQA
Copy link

SparkQA commented Aug 15, 2017

Test build #80659 has finished for PR 18946 at commit 9f2ec8f.

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

@SparkQA
Copy link

SparkQA commented Aug 15, 2017

Test build #80657 has finished for PR 18946 at commit f057ff8.

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

@DonnyZone
Copy link
Contributor Author

Is Jenkin unstable?

@SparkQA
Copy link

SparkQA commented Aug 15, 2017

Test build #80674 has finished for PR 18946 at commit ab61839.

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

@gatorsmile
Copy link
Member

LGTM

Thanks! Merging to master

@asfgit asfgit closed this in bc99025 Aug 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants