Skip to content

Conversation

@clockfly
Copy link
Contributor

@clockfly clockfly commented Sep 21, 2016

What changes were proposed in this pull request?

Remainder(%) expression's eval() returns incorrect result when the dividend is a big double. The reason is that Remainder converts the double dividend to decimal to do "%", and that lose precision.

This bug only affects the eval() that is used by constant folding, the codegen path is not impacted.

Before change

scala> -5083676433652386516D % 10
res2: Double = -6.0

scala> spark.sql("select -5083676433652386516D % 10 as a").show
+---+
|  a|
+---+
|0.0|
+---+

After change

scala> spark.sql("select -5083676433652386516D % 10 as a").show
+----+
|   a|
+----+
|-6.0|
+----+

How was this patch tested?

Unit test.

@clockfly
Copy link
Contributor Author

@cloud-fan

@cloud-fan
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Sep 21, 2016

Test build #65693 has finished for PR 15171 at commit 955f49a.

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

@clockfly
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Sep 21, 2016

Test build #65701 has finished for PR 15171 at commit 955f49a.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master/2.0!

asfgit pushed a commit that referenced this pull request Sep 21, 2016
…ult on double value

## What changes were proposed in this pull request?

Remainder(%) expression's `eval()` returns incorrect result when the dividend is a big double. The reason is that Remainder converts the double dividend to decimal to do "%", and that lose precision.

This bug only affects the `eval()` that is used by constant folding, the codegen path is not impacted.

### Before change
```
scala> -5083676433652386516D % 10
res2: Double = -6.0

scala> spark.sql("select -5083676433652386516D % 10 as a").show
+---+
|  a|
+---+
|0.0|
+---+
```

### After change
```
scala> spark.sql("select -5083676433652386516D % 10 as a").show
+----+
|   a|
+----+
|-6.0|
+----+
```

## How was this patch tested?

Unit test.

Author: Sean Zhong <[email protected]>

Closes #15171 from clockfly/SPARK-17617.

(cherry picked from commit 3977223)
Signed-off-by: Wenchen Fan <[email protected]>
@asfgit asfgit closed this in 3977223 Sep 21, 2016
@cloud-fan
Copy link
Contributor

also 1.6

asfgit pushed a commit that referenced this pull request Sep 21, 2016
…ult on double value

## What changes were proposed in this pull request?

Remainder(%) expression's `eval()` returns incorrect result when the dividend is a big double. The reason is that Remainder converts the double dividend to decimal to do "%", and that lose precision.

This bug only affects the `eval()` that is used by constant folding, the codegen path is not impacted.

### Before change
```
scala> -5083676433652386516D % 10
res2: Double = -6.0

scala> spark.sql("select -5083676433652386516D % 10 as a").show
+---+
|  a|
+---+
|0.0|
+---+
```

### After change
```
scala> spark.sql("select -5083676433652386516D % 10 as a").show
+----+
|   a|
+----+
|-6.0|
+----+
```

## How was this patch tested?

Unit test.

Author: Sean Zhong <[email protected]>

Closes #15171 from clockfly/SPARK-17617.

(cherry picked from commit 3977223)
Signed-off-by: Wenchen Fan <[email protected]>
zzcclp pushed a commit to zzcclp/spark that referenced this pull request Sep 22, 2016
…ult on double value

## What changes were proposed in this pull request?

Remainder(%) expression's `eval()` returns incorrect result when the dividend is a big double. The reason is that Remainder converts the double dividend to decimal to do "%", and that lose precision.

This bug only affects the `eval()` that is used by constant folding, the codegen path is not impacted.

### Before change
```
scala> -5083676433652386516D % 10
res2: Double = -6.0

scala> spark.sql("select -5083676433652386516D % 10 as a").show
+---+
|  a|
+---+
|0.0|
+---+
```

### After change
```
scala> spark.sql("select -5083676433652386516D % 10 as a").show
+----+
|   a|
+----+
|-6.0|
+----+
```

## How was this patch tested?

Unit test.

Author: Sean Zhong <[email protected]>

Closes apache#15171 from clockfly/SPARK-17617.

(cherry picked from commit 3977223)
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 8f88412)
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