Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

@rxin
Copy link
Contributor

@rxin rxin commented Jul 17, 2015

Just a small change to add Product type to the base expression/plan abstract classes, based on suggestions on #7434 and offline discussions.

@marmbrus
Copy link
Contributor

marmbrus commented Jul 17, 2015

LGTM

@SparkQA
Copy link

SparkQA commented Jul 17, 2015

Test build #37662 has finished for PR 7479 at commit e407ffd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • abstract class Expression extends TreeNode[Expression] with Product
    • trait Generator extends Expression
    • trait NamedExpression extends Expression
    • abstract class Attribute extends LeafExpression with NamedExpression
    • abstract class LogicalPlan extends QueryPlan[LogicalPlan] with Logging with Product
    • abstract class SparkPlan extends QueryPlan[SparkPlan] with Logging with Product with Serializable

@marmbrus
Copy link
Contributor

marmbrus commented Jul 17, 2015

Merging to master. Good riddance to self: Product =>!

@asfgit asfgit closed this in b2aa490 Jul 17, 2015
cloud-fan
cloud-fan reviewed Jul 18, 2015
View reviewed changes
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala
Copy link
Contributor

@cloud-fan cloud-fan Jul 18, 2015

Choose a reason for hiding this comment

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

why not making QueryPlan extends Product?

Copy link
Contributor

@cloud-fan cloud-fan Jul 18, 2015

Choose a reason for hiding this comment

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

or why not just make TreeNode extends Product?

@cloud-fan cloud-fan mentioned this pull request Jul 18, 2015
Closed
asfgit pushed a commit that referenced this pull request Jul 18, 2015
@cloud-fan @rxin
[SPARK-9142][SQL] remove more self type in catalyst
1b4ff05 
a follow up of #7479.
The `TreeNode` is the root case of the requirement of `self: Product =>` stuff, so why not make `TreeNode` extend `Product`?

Author: Wenchen Fan <[email protected]>

Closes #7495 from cloud-fan/self-type and squashes the following commits:

8676af7 [Wenchen Fan] remove more self type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rxin @marmbrus @SparkQA @cloud-fan