[SPARK-8226][SQL]Add function shiftrightunsigned #7035

zhichao-li · 2015-06-26T07:58:42Z

No description provided.

zhichao-li · 2015-06-26T07:59:06Z

cc @chenghao-intel @adrian-wang

AmplabJenkins · 2015-06-26T08:02:12Z

Merged build triggered.

AmplabJenkins · 2015-06-26T08:02:21Z

Merged build started.

adrian-wang · 2015-06-26T08:03:14Z

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

you can construct an seq and use contains()

almost the same thing for the performance point the view? any way it might be more concise by using seq.

SparkQA · 2015-06-26T08:03:16Z

Test build #35840 has started for PR 7035 at commit 6bdd7f1.

SparkQA · 2015-06-26T09:21:08Z

Test build #35840 has finished for PR 7035 at commit 6bdd7f1.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class ShiftRightUnsigned(left: Expression, right: Expression) extends Expression

AmplabJenkins · 2015-06-26T09:21:18Z

Merged build finished. Test FAILed.

zhichao-li · 2015-06-29T00:57:53Z

retest this please

AmplabJenkins · 2015-06-29T01:02:12Z

Merged build triggered.

AmplabJenkins · 2015-06-29T01:02:21Z

Merged build started.

SparkQA · 2015-06-29T01:05:17Z

Test build #35949 has started for PR 7035 at commit 6bdd7f1.

SparkQA · 2015-06-29T02:57:58Z

Test build #35949 has finished for PR 7035 at commit 6bdd7f1.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class ShiftRightUnsigned(left: Expression, right: Expression) extends Expression

AmplabJenkins · 2015-06-29T02:58:11Z

Merged build finished. Test PASSed.

chenghao-intel · 2015-06-30T00:58:10Z

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

space before {

Not just child?

chenghao-intel · 2015-06-30T01:10:10Z

Can you wait a little bit for #7034? Hopefully it will be simpler once #7034 merged.

AmplabJenkins · 2015-07-02T04:38:11Z

Merged build triggered.

AmplabJenkins · 2015-07-02T04:38:18Z

Merged build started.

SparkQA · 2015-07-02T04:41:17Z

Test build #36338 has started for PR 7035 at commit b973ac7.

SparkQA · 2015-07-02T04:57:01Z

Test build #36338 has finished for PR 7035 at commit b973ac7.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class ShiftLeft(valueExpr: Expression, bitsExpr: Expression)
- case class ShiftRight(valueExpr: Expression, bitsExpr: Expression)
- case class ShiftRightUnsigned(valueExpr: Expression, bitsExpr: Expression)

AmplabJenkins · 2015-07-02T04:57:08Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-07-02T05:23:11Z

Merged build triggered.

AmplabJenkins · 2015-07-02T05:23:17Z

Merged build started.

SparkQA · 2015-07-02T05:24:22Z

Test build #36346 has started for PR 7035 at commit 945a45d.

SparkQA · 2015-07-02T07:41:17Z

Test build #36346 has finished for PR 7035 at commit 945a45d.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class ShiftLeft(valueExpr: Expression, bitsExpr: Expression)
- case class ShiftRight(valueExpr: Expression, bitsExpr: Expression)
- case class ShiftRightUnsigned(valueExpr: Expression, bitsExpr: Expression)

AmplabJenkins · 2015-07-02T07:42:04Z

Merged build finished. Test PASSed.

chenghao-intel · 2015-07-02T12:06:57Z

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

BinaryExpression?

Or

abstract class BitwiseShiftExpression extends Expression { def value: Expression def bits: Expression ... }

chenghao-intel · 2015-07-02T12:11:15Z

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

Remove it also?

AmplabJenkins · 2015-07-03T05:13:10Z

Merged build triggered.

AmplabJenkins · 2015-07-03T05:13:18Z

Merged build started.

zhichao-li · 2015-07-03T05:13:50Z

Since [#7178] has been merged, I can just simply create a similar one to implement the shiftrightunsigned. cc @davies @tarekauel

SparkQA · 2015-07-03T05:16:38Z

Test build #36473 has started for PR 7035 at commit d85ae0b.

SparkQA · 2015-07-03T05:18:27Z

Test build #36473 has finished for PR 7035 at commit d85ae0b.

This patch fails Python style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class Factorial(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class ShiftRightUnsigned(left: Expression, right: Expression) extends BinaryExpression

AmplabJenkins · 2015-07-03T05:18:28Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-07-03T05:23:12Z

Merged build triggered.

AmplabJenkins · 2015-07-03T05:23:19Z

Merged build started.

SparkQA · 2015-07-03T05:24:45Z

Test build #36476 has started for PR 7035 at commit 3e9f5ae.

tarekbecker · 2015-07-03T05:59:00Z

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

That one is interesting. Have you seen my comment? Can you have a look on that gist: https://gist.github.com/tarekauel/6994983b83a51668c5dc. Am I getting something wrong?

humm, using pattern match with a typed pattern is indeed a best practice in scala rather than type tests and casts. @chenghao-intel any comments?

Just after a second thought the two styles should not have such huge difference. By reducing one parameter: dataType , the testing result is quite close.
https://gist.github.com/zhichao-li/bff04ed95ef72d09d0e9 A: 10 B: 26

but anyway, seems like pattern match with a typed pattern is a little bit faster, if no objections, I'm going to revert to that style.

Yeah, I was surprised too. Thanks for figuring out!

Have you ever try something like https://github.com/apache/spark/pull/7034/files#diff-d788f93e29b4d25cdd7d60328587678bR359 ?
Since the data type is fixed once the expression created, I don't think we need to check the data type during the runtime, even we don't need the pattern match at all in eval (in runtime).

SparkQA · 2015-07-03T06:59:02Z

Test build #36476 has finished for PR 7035 at commit 3e9f5ae.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class Word2VecModel(JavaVectorTransformer, JavaSaveable, JavaLoader):
- case class CreateNamedStruct(children: Seq[Expression]) extends Expression
- case class Factorial(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class ShiftLeft(left: Expression, right: Expression) extends BinaryExpression
- case class ShiftRight(left: Expression, right: Expression) extends BinaryExpression
- case class ShiftRightUnsigned(left: Expression, right: Expression) extends BinaryExpression
- case class Md5(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Sha1(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Crc32(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Not(child: Expression) extends UnaryExpression with Predicate with ExpectsInputTypes
- trait StringRegexExpression extends ExpectsInputTypes
- trait CaseConversionExpression extends ExpectsInputTypes
- trait StringComparison extends ExpectsInputTypes
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes
- protected[sql] abstract class AtomicType extends DataType
- abstract class NumericType extends AtomicType
- abstract class DataType extends AbstractDataType

AmplabJenkins · 2015-07-03T06:59:40Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-07-03T08:18:12Z

Merged build triggered.

AmplabJenkins · 2015-07-03T08:18:18Z

Merged build started.

SparkQA · 2015-07-03T08:21:25Z

Test build #36490 has started for PR 7035 at commit 6bcca5a.

SparkQA · 2015-07-03T09:53:26Z

Test build #36490 has finished for PR 7035 at commit 6bcca5a.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class ShiftRightUnsigned(left: Expression, right: Expression) extends BinaryExpression

AmplabJenkins · 2015-07-03T09:53:58Z

Merged build finished. Test PASSed.

davies · 2015-07-03T22:35:40Z

LGTM, merging this into master, thanks!

adrian-wang reviewed Jun 26, 2015
View reviewed changes

chenghao-intel reviewed Jun 30, 2015
View reviewed changes

zhichao-li force-pushed the shiftRightUnsigned branch from 6bdd7f1 to b973ac7 Compare July 2, 2015 04:36

zhichao-li changed the title ~~[SPARK-8226][SQL][WIP] Add function shiftrightunsigned~~ [SPARK-8226]SPARK-8223][SPARK-8224[SQL][WIP] Add function shiftrightunsigned shiftright and shiftleft Jul 2, 2015

zhichao-li changed the title ~~[SPARK-8226]SPARK-8223][SPARK-8224[SQL][WIP] Add function shiftrightunsigned shiftright and shiftleft~~ [SPARK-8226][SPARK-8223][SPARK-8224[SQL][WIP] Add function shiftrightunsigned shiftright and shiftleft Jul 2, 2015

chenghao-intel reviewed Jul 2, 2015
View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala Outdated

Copy link

Contributor

chenghao-intel Jul 2, 2015

Choose a reason for hiding this comment

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

Remove it also?

add shiftrightunsigned

d85ae0b

zhichao-li force-pushed the shiftRightUnsigned branch from 945a45d to d85ae0b Compare July 3, 2015 05:09

zhichao-li changed the title ~~[SPARK-8226][SPARK-8223][SPARK-8224[SQL][WIP] Add function shiftrightunsigned shiftright and shiftleft~~ [SPARK-8226][SQL]Add function shiftrightunsigned Jul 3, 2015

python style

3e9f5ae

tarekbecker reviewed Jul 3, 2015
View reviewed changes

change coding style

6bcca5a

asfgit closed this in ab535b9 Jul 3, 2015

[SPARK-8226][SQL]Add function shiftrightunsigned #7035

[SPARK-8226][SQL]Add function shiftrightunsigned #7035

Uh oh!

Conversation

zhichao-li commented Jun 26, 2015

Uh oh!

zhichao-li commented Jun 26, 2015

Uh oh!

AmplabJenkins commented Jun 26, 2015

Uh oh!

AmplabJenkins commented Jun 26, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 26, 2015

Uh oh!

SparkQA commented Jun 26, 2015

Uh oh!

AmplabJenkins commented Jun 26, 2015

Uh oh!

zhichao-li commented Jun 29, 2015

Uh oh!

AmplabJenkins commented Jun 29, 2015

Uh oh!

AmplabJenkins commented Jun 29, 2015

Uh oh!

SparkQA commented Jun 29, 2015

Uh oh!

SparkQA commented Jun 29, 2015

Uh oh!

AmplabJenkins commented Jun 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chenghao-intel commented Jun 30, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

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!

AmplabJenkins commented Jul 3, 2015

Uh oh!

AmplabJenkins commented Jul 3, 2015

Uh oh!

zhichao-li commented Jul 3, 2015

Uh oh!

SparkQA commented Jul 3, 2015

Uh oh!

SparkQA commented Jul 3, 2015

Uh oh!

AmplabJenkins commented Jul 3, 2015

Uh oh!

AmplabJenkins commented Jul 3, 2015

Uh oh!