-
Notifications
You must be signed in to change notification settings - Fork 13k
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.
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
[FLINK-2178][gelly] Fixed groupReduceOnNeighbors bug #799
Conversation
uhmm, guys! IMO this should be merged before the release! |
Hey @andralungu, I'm not sure I understand this one. It's a coGroup of vertices with the edges. For the vertex iterator to be empty, doesn't it mean that there's an edge with an invalid id? |
Yup, exactly! The use case was: I modified something in the edge data set, called groupReduceOnNeighbors on the result and got NPE, exception that could have been avoided with this check, just like was the case for the Degree NPE. Making sure that the iterator is not null can save some people lots of headaches, IMO :) And it doesn't hurt anyone who has a correct data set. |
Then shouldn't we throw an exception like we do in degrees with a "The edge src/trg id is invalid" or something, instead of hiding the exception? |
1655b60
to
fbac355
Compare
PR updated. |
@andralungu, I believe the same issue appears in |
fbac355
to
ca2c3e4
Compare
PR updated to test the same issue in ApplyCoGroupFunctionOnAllEdges and ApplyCoGroupFunction. |
Perfect, thanks @andralungu! +1 |
2576bd9
to
3192ada
Compare
Merging... |
[FLINK-2178][gelly] Threw exception when iterator did not have next [FLINK-2178][gelly] Added further checks and tests
3192ada
to
0a09e85
Compare
This PR adds an iterator.hasNext() check in the groupReduceOnNeighbors method, fixing the NoSuchElementException bug reported in FLINK-2178. @vasia , @StephanEwen, maybe we can merge this before the upcoming 0.9 release :) Thanks! Author: andralungu <lungu.andra@gmail.com> Closes apache#799 from andralungu/groupReduceOnBug and squashes the following commits: 0a09e85 [andralungu] [FLINK-2178][gelly] Fixed groupReduceOnNeighbors bug
This PR adds an iterator.hasNext() check in the groupReduceOnNeighbors method, fixing the NoSuchElementException bug reported in FLINK-2178. @vasia , @StephanEwen, maybe we can merge this before the upcoming 0.9 release :) Thanks! Author: andralungu <lungu.andra@gmail.com> Closes apache#799 from andralungu/groupReduceOnBug and squashes the following commits: 0a09e85 [andralungu] [FLINK-2178][gelly] Fixed groupReduceOnNeighbors bug
This PR adds an iterator.hasNext() check in the groupReduceOnNeighbors method, fixing the NoSuchElementException bug reported in FLINK-2178.
@vasia , @StephanEwen, maybe we can merge this before the upcoming 0.9 release :)
Thanks!