[SPARK-9085][SQL] Remove LeafNode, UnaryNode, BinaryNode from TreeNode. #7434

rxin · 2015-07-15T23:58:40Z

This builds on #7433 but also removes LeafNode/UnaryNode. These are slightly more complicated to remove. I had to change some abstract classes to traits in order for it to work.

The problem with LeafNode/UnaryNode is that they are often mixed in at the end of an Expression, and then the toString function actually gets resolved to the ones defined in TreeNode, rather than in Expression.

SparkQA · 2015-07-16T00:20:36Z

Test build #37427 has finished for PR 7434 at commit 43eeb12.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class StandaloneRecoveryModeFactory(conf: SparkConf, serializer: Serializer)
- case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute
- abstract class Star extends LeafExpression with NamedExpression
- case class UnresolvedAlias(child: Expression) extends UnaryExpression with NamedExpression
- abstract class LeafExpression extends Expression
- abstract class UnaryExpression extends Expression
- abstract class BinaryExpression extends Expression
- case class SortOrder(child: Expression, direction: SortDirection) extends UnaryExpression
- trait AggregateExpression extends Expression
- trait PartialAggregate extends AggregateExpression
- case class Min(child: Expression) extends UnaryExpression with PartialAggregate
- case class Max(child: Expression) extends UnaryExpression with PartialAggregate
- case class Count(child: Expression) extends UnaryExpression with PartialAggregate
- case class Average(child: Expression) extends UnaryExpression with PartialAggregate
- case class Sum(child: Expression) extends UnaryExpression with PartialAggregate
- case class SumDistinct(child: Expression) extends UnaryExpression with PartialAggregate
- case class First(child: Expression) extends UnaryExpression with PartialAggregate
- case class Last(child: Expression) extends UnaryExpression with PartialAggregate
- case class Pmod(left: Expression, right: Expression) extends BinaryArithmetic
- final class SpecificRow extends $
- trait Generator extends Expression
- case class Explode(child: Expression) extends UnaryExpression with Generator
- trait NamedExpression extends Expression
- abstract class Attribute extends LeafExpression with NamedExpression
- case class PrettyAttribute(name: String) extends Attribute
- abstract class LeafNode extends LogicalPlan
- abstract class UnaryNode extends LogicalPlan
- abstract class BinaryNode extends LogicalPlan

rxin · 2015-07-16T01:22:37Z

cc @marmbrus

SparkQA · 2015-07-16T01:43:03Z

Test build #37435 has finished for PR 7434 at commit 2225331.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute
- abstract class Star extends LeafExpression with NamedExpression
- case class UnresolvedAlias(child: Expression) extends UnaryExpression with NamedExpression
- abstract class LeafExpression extends Expression
- abstract class UnaryExpression extends Expression
- abstract class BinaryExpression extends Expression
- case class SortOrder(child: Expression, direction: SortDirection) extends UnaryExpression
- trait AggregateExpression extends Expression
- trait PartialAggregate extends AggregateExpression
- case class Min(child: Expression) extends UnaryExpression with PartialAggregate
- case class Max(child: Expression) extends UnaryExpression with PartialAggregate
- case class Count(child: Expression) extends UnaryExpression with PartialAggregate
- case class Average(child: Expression) extends UnaryExpression with PartialAggregate
- case class Sum(child: Expression) extends UnaryExpression with PartialAggregate
- case class SumDistinct(child: Expression) extends UnaryExpression with PartialAggregate
- case class First(child: Expression) extends UnaryExpression with PartialAggregate
- case class Last(child: Expression) extends UnaryExpression with PartialAggregate
- trait Generator extends Expression
- case class Explode(child: Expression) extends UnaryExpression with Generator
- trait NamedExpression extends Expression
- abstract class Attribute extends LeafExpression with NamedExpression
- case class PrettyAttribute(name: String) extends Attribute
- abstract class LeafNode extends LogicalPlan
- abstract class UnaryNode extends LogicalPlan
- abstract class BinaryNode extends LogicalPlan

SparkQA · 2015-07-16T02:21:58Z

Test build #37439 has finished for PR 7434 at commit 9c589cf.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute
- abstract class Star extends LeafExpression with NamedExpression
- case class UnresolvedAlias(child: Expression) extends UnaryExpression with NamedExpression
- abstract class LeafExpression extends Expression
- abstract class UnaryExpression extends Expression
- abstract class BinaryExpression extends Expression
- case class SortOrder(child: Expression, direction: SortDirection) extends UnaryExpression
- trait AggregateExpression extends Expression
- trait PartialAggregate extends AggregateExpression
- case class Min(child: Expression) extends UnaryExpression with PartialAggregate
- case class Max(child: Expression) extends UnaryExpression with PartialAggregate
- case class Count(child: Expression) extends UnaryExpression with PartialAggregate
- case class Average(child: Expression) extends UnaryExpression with PartialAggregate
- case class Sum(child: Expression) extends UnaryExpression with PartialAggregate
- case class SumDistinct(child: Expression) extends UnaryExpression with PartialAggregate
- case class First(child: Expression) extends UnaryExpression with PartialAggregate
- case class Last(child: Expression) extends UnaryExpression with PartialAggregate
- trait Generator extends Expression
- case class Explode(child: Expression) extends UnaryExpression with Generator
- trait NamedExpression extends Expression
- abstract class Attribute extends LeafExpression with NamedExpression
- case class PrettyAttribute(name: String) extends Attribute
- abstract class LeafNode extends LogicalPlan
- abstract class UnaryNode extends LogicalPlan
- abstract class BinaryNode extends LogicalPlan

SparkQA · 2015-07-16T03:42:48Z

Test build #37440 has finished for PR 7434 at commit 3135a8b.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute
- abstract class Star extends LeafExpression with NamedExpression
- case class UnresolvedAlias(child: Expression) extends UnaryExpression with NamedExpression
- case class SortOrder(child: Expression, direction: SortDirection) extends UnaryExpression
- trait AggregateExpression extends Expression
- trait PartialAggregate extends AggregateExpression
- case class Min(child: Expression) extends UnaryExpression with PartialAggregate
- case class Max(child: Expression) extends UnaryExpression with PartialAggregate
- case class Count(child: Expression) extends UnaryExpression with PartialAggregate
- case class Average(child: Expression) extends UnaryExpression with PartialAggregate
- case class Sum(child: Expression) extends UnaryExpression with PartialAggregate
- case class SumDistinct(child: Expression) extends UnaryExpression with PartialAggregate
- case class First(child: Expression) extends UnaryExpression with PartialAggregate
- case class Last(child: Expression) extends UnaryExpression with PartialAggregate
- trait Generator extends Expression
- case class Explode(child: Expression) extends UnaryExpression with Generator
- trait NamedExpression extends Expression
- abstract class Attribute extends LeafExpression with NamedExpression
- case class PrettyAttribute(name: String) extends Attribute
- abstract class LeafNode extends LogicalPlan
- abstract class UnaryNode extends LogicalPlan

cloud-fan · 2015-07-16T05:42:44Z

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

how about:

trait AggregateExpression { self: Expression => }

it'd result in a lot more code changes to get it working. e.g.

abstract class AggregateFunction extends LeafExpression with AggregateExpression with Serializable { self: Product => /** Base should return the generic aggregate expression that this function is computing */ val base: AggregateExpression override def nullable: Boolean = base.nullable <--- this won't work anymore override def dataType: DataType = base.dataType

SparkQA · 2015-07-16T05:44:11Z

Test build #37458 has finished for PR 7434 at commit 9e8a4de.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class UnresolvedAttribute(nameParts: Seq[String]) extends Attribute
- abstract class Star extends LeafExpression with NamedExpression
- case class UnresolvedAlias(child: Expression) extends UnaryExpression with NamedExpression
- case class SortOrder(child: Expression, direction: SortDirection) extends UnaryExpression
- trait AggregateExpression extends Expression
- trait PartialAggregate extends AggregateExpression
- case class Min(child: Expression) extends UnaryExpression with PartialAggregate
- case class Max(child: Expression) extends UnaryExpression with PartialAggregate
- case class Count(child: Expression) extends UnaryExpression with PartialAggregate
- case class Average(child: Expression) extends UnaryExpression with PartialAggregate
- case class Sum(child: Expression) extends UnaryExpression with PartialAggregate
- case class SumDistinct(child: Expression) extends UnaryExpression with PartialAggregate
- case class First(child: Expression) extends UnaryExpression with PartialAggregate
- case class Last(child: Expression) extends UnaryExpression with PartialAggregate
- trait Generator extends Expression
- case class Explode(child: Expression) extends UnaryExpression with Generator
- trait NamedExpression extends Expression
- abstract class Attribute extends LeafExpression with NamedExpression
- case class PrettyAttribute(name: String) extends Attribute
- abstract class LeafNode extends LogicalPlan
- abstract class UnaryNode extends LogicalPlan

cloud-fan · 2015-07-16T05:45:23Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala

I remember we set a default parameter value for eval, so we can just write e.eval() here?

rxin · 2015-07-16T20:58:23Z

OK I'm going to merge this. I will try to get rid of the self types in a followup pr.

Just a small change to add Product type to the base expression/plan abstract classes, based on suggestions on #7434 and offline discussions. Author: Reynold Xin <[email protected]> Closes #7479 from rxin/remove-self-types and squashes the following commits: e407ffd [Reynold Xin] [SPARK-9142][SQL] Removing unnecessary self types in Catalyst.

rxin added 2 commits July 15, 2015 18:19

[SPARK-9085][SQL] Remove LeafNode, UnaryNode, BinaryNode from TreeNode.

16b5c90

Aggregate expressions should not be foldable.

2225331

rxin force-pushed the remove-binary-unary-leaf-node branch from 00faa74 to 2225331 Compare July 16, 2015 01:20

rxin added 2 commits July 15, 2015 19:01

Fixed one more test case...

9c589cf

SortOrder should not be foldable.

3135a8b

Generator should not be foldable.

9e8a4de

cloud-fan reviewed Jul 16, 2015
View reviewed changes

asfgit closed this in fec10f0 Jul 16, 2015

rxin mentioned this pull request Jul 17, 2015

[SPARK-9142][SQL] Removing unnecessary self types in Catalyst. #7479

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-9085][SQL] Remove LeafNode, UnaryNode, BinaryNode from TreeNode. #7434

[SPARK-9085][SQL] Remove LeafNode, UnaryNode, BinaryNode from TreeNode. #7434

Uh oh!

rxin commented Jul 15, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

rxin commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

cloud-fan Jul 16, 2015

Uh oh!

rxin Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

cloud-fan Jul 16, 2015

Uh oh!

rxin commented Jul 16, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[SPARK-9085][SQL] Remove LeafNode, UnaryNode, BinaryNode from TreeNode. #7434

[SPARK-9085][SQL] Remove LeafNode, UnaryNode, BinaryNode from TreeNode. #7434

Uh oh!

Conversation

rxin commented Jul 15, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

rxin commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

cloud-fan Jul 16, 2015

Choose a reason for hiding this comment

Uh oh!

rxin Jul 16, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 16, 2015

Uh oh!

cloud-fan Jul 16, 2015

Choose a reason for hiding this comment

Uh oh!

rxin commented Jul 16, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants