Skip to content

Conversation

@cloud-fan
Copy link
Contributor

We will do local projection for LocalRelation, and thus reuse the same Expression object among multiply evaluations. We should reset the mutable states of Expression before evaluate it.

Fix PullOutNondeterministic rule to make it work for Sort.

Also got a chance to cleanup the dataframe test suite.

@cloud-fan cloud-fan changed the title [SPARK-8608][SQL] DataFrame with random columns should return same value when call show [SPARK-8608][SPARK-8609][SQL] reset mutable states of nondeterministic expression before evaluation Jul 26, 2015
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This case should be fixed by #7593. However, PullOutNondeterministic rule can also handle it, so we can add this test before #7593 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to make it more clear, now this PR is aimed to bug fix, i.e. fix nondeterministic expression handling for local projection and sort. Leave #7593 to optimization, i.e. remove still-need-evaluate expressions from Sort.

@SparkQA
Copy link

SparkQA commented Jul 26, 2015

Test build #38467 has finished for PR 7674 at commit 8f57ede.

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

@SparkQA
Copy link

SparkQA commented Jul 26, 2015

Test build #38469 has finished for PR 7674 at commit e3814fb.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

lazy val here - otherwise a failure in the ctor could wipe out the entire test suite and then test cases won't show up as a failed tests.

@rxin
Copy link
Contributor

rxin commented Jul 26, 2015

@cloud-fan I'm confused -- what did this change do? Is it the change in the analyzer that fixes the bug? If yes, can you add a unit test to the analyzer rule, in addition to the test case in the DataFrameSuite?

@cloud-fan cloud-fan changed the title [SPARK-8608][SPARK-8609][SQL] reset mutable states of nondeterministic expression before evaluation [SPARK-8608][SPARK-8609][SPARK-9083][SQL] reset mutable states of nondeterministic expression before evaluation and fix PullOutNondeterministic Jul 27, 2015
@cloud-fan
Copy link
Contributor Author

hi @rxin, I want to make this PR focus on bug fix, i.e. fix nondeterministic expression handling for local projection and sort. Leave #7593 to optimization, i.e. remove still-need-evaluate expressions from Sort. Is that OK?

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #38490 has finished for PR 7674 at commit e0a6bd6.

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

@cloud-fan
Copy link
Contributor Author

retest this please.

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #110 has finished for PR 7674 at commit e0a6bd6.

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

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #38495 has finished for PR 7674 at commit e0a6bd6.

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

@cloud-fan
Copy link
Contributor Author

retest this please.

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #38509 has finished for PR 7674 at commit e0a6bd6.

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

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #111 has finished for PR 7674 at commit e0a6bd6.

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

@SparkQA
Copy link

SparkQA commented Jul 27, 2015

Test build #38528 has finished for PR 7674 at commit 888934f.

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

@rxin
Copy link
Contributor

rxin commented Jul 29, 2015

I'm going to merge this. Would be great if @marmbrus or @yhuai can review this post-hoc still.

@asfgit asfgit closed this in 429b2f0 Jul 29, 2015
@cloud-fan cloud-fan deleted the show branch July 29, 2015 05:32
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.

3 participants