[SPARK-8075][SQL] apply type check interface to more expressions by cloud-fan · Pull Request #6723 · apache/spark

cloud-fan · 2015-06-09T15:11:42Z

a follow up of #6405.
Note: It's not a big change, a lot of changing is due to I swap some code in aggregates.scala to make aggregate functions right below its corresponding aggregate expressions.

SparkQA · 2015-06-09T15:19:36Z

Test build #34506 has finished for PR 6723 at commit 64d78a8.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

cloud-fan · 2015-06-09T15:35:38Z

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

Do we really want this? There are a lot of expressions that don't need type check...

ok i agree with you maybe we should implement this by default to be always "success", so we don't need to define this for every expression.

cloud-fan · 2015-06-09T15:42:07Z

cc @rxin , according to here and here, do we require BinaryExpression to have 2 equal-type children? If so, I think we should add java doc of BinaryExpression to say that, or we should create a subclass of BinaryExpression which has equal-type children.

SparkQA · 2015-06-09T17:19:43Z

Test build #34509 has finished for PR 6723 at commit 305be74.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

SparkQA · 2015-06-09T17:42:35Z

Test build #34508 has finished for PR 6723 at commit 6f09d05.

This patch passes all tests.
This patch does not merge cleanly.
This patch adds the following public classes (experimental):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

rxin · 2015-06-09T23:45:43Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala

why do they need to be here?

cloud-fan · 2015-06-11T09:47:19Z

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

I'm thinking of making resolved final, and add a overridable method like extraResolveLogic, so that people won't forget to check types when they override resolved.

SparkQA · 2015-06-11T09:51:43Z

Test build #34684 has finished for PR 6723 at commit 3c3c87b.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

SparkQA · 2015-06-19T09:07:24Z

Test build #35244 has finished for PR 6723 at commit 0faa15a.

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

cloud-fan · 2015-06-21T09:19:21Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercion.scala

I checked with hive, CreateArray should cast input to string, but with some rules like Coalesce did. For example we should not cast boolean to string for CreateArray.

SparkQA · 2015-06-21T09:56:16Z

Test build #35387 has finished for PR 6723 at commit 343074b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

SparkQA · 2015-06-21T10:45:38Z

Test build #35388 has finished for PR 6723 at commit 2811da0.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class Module(object):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

SparkQA · 2015-06-21T15:54:59Z

Test build #35392 has finished for PR 6723 at commit bb9948a.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class Module(object):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

SparkQA · 2015-06-24T06:54:24Z

Test build #35637 has finished for PR 6723 at commit 2124301.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class CountFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction
- case class CountDistinctFunction(
- case class ApproxCountDistinctPartitionFunction(
- case class ApproxCountDistinctMergeFunction(
- case class Sum(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class CombineSum(child: Expression) extends AggregateExpression
- case class SumDistinct(child: Expression)
- case class CombineSetsAndSum(inputSet: Expression, base: Expression) extends AggregateExpression
- case class CombineSetsAndSumFunction(
- case class First(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]
- case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression]

cloud-fan · 2015-06-24T08:16:00Z

cc @rxin

marmbrus · 2015-06-24T23:26:23Z

Thanks! Merging to master.

cloud-fan force-pushed the type-check branch 2 times, most recently from 6f09d05 to 305be74 Compare June 9, 2015 15:32

cloud-fan reviewed Jun 9, 2015
View reviewed changes

rxin reviewed Jun 9, 2015
View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala Outdated

Copy link

Contributor

rxin Jun 9, 2015

Choose a reason for hiding this comment

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

why do they need to be here?

cloud-fan force-pushed the type-check branch 2 times, most recently from 8423bc7 to 3c3c87b Compare June 11, 2015 09:43

cloud-fan reviewed Jun 11, 2015
View reviewed changes

cloud-fan force-pushed the type-check branch 2 times, most recently from 772866d to 0faa15a Compare June 19, 2015 07:14

cloud-fan force-pushed the type-check branch from 0faa15a to 343074b Compare June 21, 2015 09:14

cloud-fan reviewed Jun 21, 2015
View reviewed changes

cloud-fan force-pushed the type-check branch from 343074b to 8efce5f Compare June 21, 2015 09:28

cloud-fan force-pushed the type-check branch from 8efce5f to 2811da0 Compare June 21, 2015 09:32

cloud-fan added 3 commits June 24, 2015 12:46

apply type check interface to more expressions

287d3bb

add tests

5a658bb

fix tests

2124301

cloud-fan force-pushed the type-check branch from bb9948a to 2124301 Compare June 24, 2015 04:50

asfgit closed this in b71d325 Jun 24, 2015

Conversation

cloud-fan commented Jun 9, 2015

Uh oh!

SparkQA commented Jun 9, 2015

Uh oh!

cloud-fan Jun 9, 2015

Choose a reason for hiding this comment

Uh oh!

rxin Jun 10, 2015

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Jun 9, 2015

Uh oh!

SparkQA commented Jun 9, 2015

Uh oh!

SparkQA commented Jun 9, 2015

Uh oh!

rxin Jun 9, 2015

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 11, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 11, 2015

Uh oh!

SparkQA commented Jun 19, 2015

Uh oh!

cloud-fan Jun 21, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 21, 2015

Uh oh!

SparkQA commented Jun 21, 2015

Uh oh!

SparkQA commented Jun 21, 2015

Uh oh!

SparkQA commented Jun 24, 2015

Uh oh!

cloud-fan commented Jun 24, 2015

Uh oh!

marmbrus commented Jun 24, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments