Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

Alias with metadata is not a no-op and we should not strip it in RemoveAliasOnlyProject rule.
This PR also did some improvement for this rule:

  1. extend the semantic of alias-only. Now we allow the project list to be partially aliased.
  2. add unit test for this rule.

How was this patch tested?

new RemoveAliasOnlyProjectSuite

@cloud-fan
Copy link
Contributor Author

cc @viirya @yhuai @liancheng

@SparkQA
Copy link

SparkQA commented Jul 8, 2016

Test build #61986 has finished for PR 14106 at commit cd3c128.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 9, 2016

Test build #62009 has finished for PR 14106 at commit 18ecbaa.

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

@viirya
Copy link
Member

viirya commented Jul 10, 2016

LGTM

// Check if projectList in the Project node has the same attribute names and ordering
// as its child node.
/**
* Returns true if the project list is semantically same with child output, after strip alias on
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: "... same with ..." => "... same as ..."

@liancheng
Copy link
Contributor

LGTM except for some minor comments.

@SparkQA
Copy link

SparkQA commented Jul 13, 2016

Test build #62248 has finished for PR 14106 at commit d337e8e.

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

@liancheng
Copy link
Contributor

Merging to master.

@asfgit asfgit closed this in db7317a Jul 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants