[SPARK-3176] Implement 'ABS and 'LAST' for sql #2099

xinyunh · 2014-08-22T18:20:30Z

Add support for the mathematical function"ABS" and the analytic function "last" to return a subset of the rows satisfying a query within spark sql. Test-cases included.

AmplabJenkins · 2014-08-22T18:23:49Z

Can one of the admins verify this patch?

marmbrus · 2014-08-26T01:09:49Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregates.scala

+  }
+
+  override def eval(input: Row): Any = if (result != null) expr.eval(result.asInstanceOf[Row]) else null
+}


End files with new lines. (Run sbt/sbt scalastyle to check for these types of style violations).

Ok. BTW, need I create a new pull request? Or just updating this branch?

Updating the branch will automatically include new commits in this pull request.

marmbrus · 2014-08-26T01:19:43Z

Thanks for working on this! I made a few comments on the implementation.

xinyunh · 2014-08-29T23:49:15Z

I have made some changes according to the comments.

marmbrus · 2014-08-30T00:12:00Z

ok to test

SparkQA · 2014-08-30T00:13:59Z

QA tests have started for PR 2099 at commit 39f0309.

This patch merges cleanly.

SparkQA · 2014-08-30T00:14:56Z

QA tests have finished for PR 2099 at commit 39f0309.

This patch fails unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class Abs(child: Expression) extends UnaryExpression
- case class Power(base: Expression, exponent: Expression) extends Expression

xinyunh · 2014-09-02T17:06:55Z

May I know in which test cases do we failed?

nchammas · 2014-09-02T17:10:03Z

The link to the test results is in Spark QA's latest message: "QA tests have finished"

It looks like you have a Scala style problem. You can run tests locally by running ./dev/run-tests to check for these things before Spark QA gets to them.

xinyunh · 2014-09-02T17:23:32Z

Thank you! Fixed this code style issue.

SparkQA · 2014-09-02T17:24:17Z

QA tests have started for PR 2099 at commit 8843643.

This patch merges cleanly.

SparkQA · 2014-09-02T19:09:49Z

QA tests have finished for PR 2099 at commit 8843643.

This patch passes unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class Abs(child: Expression) extends UnaryExpression
- case class Power(base: Expression, exponent: Expression) extends Expression

marmbrus · 2014-09-03T03:48:08Z

The implementation of Last and Abs look great to me. I think we could merge them now. Regarding power, I'm concerned that we aren't handling all of the various cases (i.e. what about Decimal?). What do you think about splitting that off into its own PR?

xinyunh · 2014-09-03T17:46:05Z

OK. Thus, I should remove the POWER part from this pull request and create another pull request which includes the POWER, correct? Need I also split the issue on Jira?

marmbrus · 2014-09-03T17:49:39Z

Yes please. We try to keep one JIRA to one PR in generall.

xinyunh · 2014-09-03T18:26:02Z

I have split the POWER to PR #2252, Jira number is 3379

SparkQA · 2014-09-03T18:29:21Z

QA tests have started for PR 2099 at commit 71d15e7.

This patch merges cleanly.

SparkQA · 2014-09-03T20:15:30Z

QA tests have finished for PR 2099 at commit 71d15e7.

This patch passes unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class Abs(child: Expression) extends UnaryExpression

marmbrus · 2014-09-05T02:08:26Z

Mind fixing the title to remove Power?

xinyunh · 2014-09-05T18:30:38Z

Sorry, I forgot

SparkQA · 2014-09-05T23:42:26Z

Can one of the admins verify this patch?

marmbrus · 2014-09-09T02:28:18Z

Mind fixing the conflict so we can merge this? Thanks!

marmbrus · 2014-09-09T02:55:22Z

ok to test

SparkQA · 2014-09-09T03:49:15Z

QA tests have started for PR 2099 at commit 71d15e7.

This patch does not merge cleanly!

SparkQA · 2014-09-09T05:02:36Z

QA tests have finished for PR 2099 at commit 71d15e7.

This patch fails unit tests.
This patch does not merge cleanly!

xinyunh · 2014-09-09T22:05:16Z

Why it fails while I only changed the name of title?

marmbrus · 2014-09-10T01:27:34Z

The test failure was unrelated. I resolved the conflict and tested locally. Merged to master. Thanks!

marmbrus reviewed Aug 26, 2014
View reviewed changes

xinyunh and others added 4 commits August 29, 2014 16:03

add 'Last' component

b3df91b

fix the bug in 'Last' component

7f6980a

add abs() function support

ff8e51e

Modify the code of POWER and ABS. Move them to the file arithmetic

39f0309

xinyunh force-pushed the sqlTest branch from 7570fa6 to 39f0309 Compare August 29, 2014 23:42

fix the code style issue

8843643

remove POWER part

71d15e7

xinyunh changed the title ~~[SPARK-3176] Implement 'POWER', 'ABS and 'LAST' for sql~~ [SPARK-3176] Implement 'ABS and 'LAST' for sql Sep 5, 2014

asfgit closed this in 07ee4a2 Sep 10, 2014

bomeng deleted the sqlTest branch January 22, 2015 19:25

[SPARK-3176] Implement 'ABS and 'LAST' for sql #2099

[SPARK-3176] Implement 'ABS and 'LAST' for sql #2099

Uh oh!

Conversation

xinyunh commented Aug 22, 2014

Uh oh!

AmplabJenkins commented Aug 22, 2014

Uh oh!

marmbrus Aug 26, 2014

Choose a reason for hiding this comment

Uh oh!

xinyunh Aug 29, 2014

Choose a reason for hiding this comment

Uh oh!

nchammas Aug 30, 2014

Choose a reason for hiding this comment

Uh oh!

marmbrus commented Aug 26, 2014

Uh oh!

xinyunh commented Aug 29, 2014

Uh oh!

marmbrus commented Aug 30, 2014

Uh oh!

SparkQA commented Aug 30, 2014

Uh oh!

SparkQA commented Aug 30, 2014

Uh oh!

xinyunh commented Sep 2, 2014

Uh oh!

nchammas commented Sep 2, 2014

Uh oh!

xinyunh commented Sep 2, 2014

Uh oh!

SparkQA commented Sep 2, 2014

Uh oh!

SparkQA commented Sep 2, 2014

Uh oh!

marmbrus commented Sep 3, 2014

Uh oh!

xinyunh commented Sep 3, 2014

Uh oh!

marmbrus commented Sep 3, 2014

Uh oh!

xinyunh commented Sep 3, 2014

Uh oh!

SparkQA commented Sep 3, 2014

Uh oh!

SparkQA commented Sep 3, 2014

Uh oh!

marmbrus commented Sep 5, 2014

Uh oh!

xinyunh commented Sep 5, 2014

Uh oh!

SparkQA commented Sep 5, 2014

Uh oh!

marmbrus commented Sep 9, 2014

Uh oh!

marmbrus commented Sep 9, 2014

Uh oh!

SparkQA commented Sep 9, 2014

Uh oh!

SparkQA commented Sep 9, 2014

Uh oh!

xinyunh commented Sep 9, 2014

Uh oh!

marmbrus commented Sep 10, 2014

Uh oh!

Uh oh!