[ 
https://issues.apache.org/jira/browse/BEAM-409?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16170592#comment-16170592
 ] 

ASF GitHub Bot commented on BEAM-409:
-------------------------------------

GitHub user youngoli opened a pull request:

    https://github.com/apache/beam/pull/3861

    [BEAM-409] Fixing incorrect use of Math.ceil in ApproximateQuantiles

    Follow this checklist to help us incorporate your contribution quickly and 
easily:
    
     - [x] Make sure there is a [JIRA 
issue](https://issues.apache.org/jira/projects/BEAM/issues/) filed for the 
change (usually before you start working on it).  Trivial changes like typos do 
not require a JIRA issue.  Your pull request should address just this issue, 
without pulling in other changes.
     - [x] Each commit in the pull request should have a meaningful subject 
line and body.
     - [x] Format the pull request title like `[BEAM-XXX] Fixes bug in 
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA 
issue.
     - [x] Write a pull request description that is detailed enough to 
understand what the pull request does, how, and why.
     - [x] Run `mvn clean verify` to make sure basic checks pass. A more 
thorough check will be performed on your pull request automatically.
     - [ ] If this contribution is large, please file an Apache [Individual 
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
    
    ---
    
    Fixed the FindBug issue by avoiding an incorrect integer division. I based 
this on an old unmerged pull request for this bug: 
https://github.com/apache/beam/pull/853
    
    That PR also included a new unit test for ApproximateQuantiles.java, wasn't 
sure if I should also include that unit test.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/youngoli/beam bugfix-beam409

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/beam/pull/3861.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3861
    
----
commit 63096efb16d7397375f64eefd32a1bb4558d01ca
Author: Daniel Oliveira <[email protected]>
Date:   2017-09-15T22:32:49Z

    [BEAM-409] Avoiding integer division in ceil
    
    Casting the denominator in this division in the ceil call to a float to
    avoid an incorrect integer division causing a bug.

----


> Incorrect use of Math.ceil in ApproximateQuantiles
> --------------------------------------------------
>
>                 Key: BEAM-409
>                 URL: https://issues.apache.org/jira/browse/BEAM-409
>             Project: Beam
>          Issue Type: Bug
>          Components: sdk-java-core
>            Reporter: Scott Wegner
>            Assignee: Daniel Oliveira
>            Priority: Minor
>              Labels: findbugs, newbie, starter
>
> [FindBugs 
> ICAST_INT_CAST_TO_DOUBLE_PASSED_TO_CEIL|https://github.com/apache/incubator-beam/blob/58a029a06aea1030279e5da8f9fa3114f456c1db/sdks/java/build-tools/src/main/resources/beam/findbugs-filter.xml#L274]:
>  Integral value cast to double and then passed to Math.ceil
> Applies to: 
> [ApproximateQuanitles|https://github.com/apache/incubator-beam/blob/58a029a06aea1030279e5da8f9fa3114f456c1db/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/ApproximateQuantiles.java#L349].
> This is a good starter bug. When fixing, please remove the corresponding 
> entries from 
> [findbugs-filter.xml|https://github.com/apache/incubator-beam/blob/master/sdks/java/build-tools/src/main/resources/beam/findbugs-filter.xml]
>  and verify the build passes.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to