Skip to content

SPARK-8336 Fix NullPointerException with functions.rand() #6793

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

Closed
tedyu wants to merge 3 commits into from
Closed

SPARK-8336 Fix NullPointerException with functions.rand() #6793

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1d66c5
Fix NullPointerException with functions.rand()
tedyu Jun 12, 2015
750f92c
Add test for Rand() with seed
tedyu Jun 13, 2015
62fd97b
Create RandomSuite
tedyu Jun 15, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/random.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ abstract class RDG(seed: Long) extends LeafExpression with Serializable {
* Record ID within each partition. By being transient, the Random Number Generator is
* reset every time we serialize and deserialize it.
*/
@transient protected lazy val rng = new XORShiftRandom(seed + TaskContext.get().partitionId())
@transient protected lazy val partitionId = TaskContext.get() match {
case null => 0
case _ => TaskContext.get().partitionId()
}
@transient protected lazy val rng = new XORShiftRandom(seed + partitionId)

override def deterministic: Boolean = false

Expand Down
1 change: 1 addition & 0 deletions .../src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper

checkEvaluation(-c1, -1.1, row)
checkEvaluation(c1 + c2, 3.1, row)
checkDoubleEvaluation(Rand(30), (0.7363714192755834 +- 0.001), row)
Copy link
Contributor

@rxin rxin Jun 13, 2015

Choose a reason for hiding this comment

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

can we create a new test case, instead of adding it to the existing one?

I've been meaning to take the existing one apart for a while.

Copy link
Contributor

@rxin rxin Jun 13, 2015

Choose a reason for hiding this comment

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

Also we should have a case where we explicitly set taskcontext

Copy link
Contributor Author

@tedyu tedyu Jun 14, 2015

Choose a reason for hiding this comment

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

Looking at the tests under sql, I don't see how TaskContext is explicitly set.

Creating a new test is fine. The new test would contain a method containing one line.
Just want to make sure this is fine.

Copy link
Contributor Author

@tedyu tedyu Jun 14, 2015

Choose a reason for hiding this comment

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

I am in Beijing now.
Except for difficulty of accessing gmail, github is quite slow as well.

checkDoubleEvaluation(c1 - c2, (-0.9 +- 0.001), row)
checkDoubleEvaluation(c1 * c2, (2.2 +- 0.001), row)
checkDoubleEvaluation(c1 / c2, (0.55 +- 0.001), row)
Expand Down