KAFKA-7316 Fix Streams Scala filter recursive call#5538
KAFKA-7316 Fix Streams Scala filter recursive call#5538guozhangwang merged 1 commit intoapache:trunkfrom
Conversation
|
See also: #5543 |
joan38
changed the title
joan38
force-pushed
the
fix-filter
branch
2 times, most recently
from
0ab1de4 to
0171c33
Compare
|
This PR is good to go. |
There was a problem hiding this comment.
What should the value be? I think it's null, right? Can we put that in the test?
Otherwise, it seems like it might actually be spitting out 1, and we wouldn't know.
There was a problem hiding this comment.
It used to be there but it returns null for a Long and scalatest doesn't seem to work with that. I tried to .asInstanceOf[Any] or null.asInstanceOf[Long] but it's doesn't seems to work. Any idea?
There was a problem hiding this comment.
Even weirder, the null.asInstanceOf[Long] works on my machine but not on CI...
There was a problem hiding this comment.
Well, I guess you can always skip the "fancy" assertion and just do if (record.value != null) { throw new AssertionError(); }
Elegant? No. But it should work ;)
There was a problem hiding this comment.
Or something higher up the chain, like assert(record.value === null)
There was a problem hiding this comment.
Idea shows me this warning Comparing unrelated types: Long and Null
There was a problem hiding this comment.
Trying out this:
record.value shouldBe (null: java.lang.Long)
Works locally, let's see on CI.
|
@joan38, Thanks again for the patch and the test. I left one comment on the tests, otherwise, it looks good to me! |
|
Also, a note that this change should also be cherry-picked to 2.0, which is affected. |
|
One more thing: @joan38, Can you add a brief description to the PR? |
|
Done |
|
@vvcephei worked! |
|
@guozhangwang I think we are all good on this one. Ready for merge. |
Due to lack of conversion to kstream Predicate, existing filter method in KTable.scala would result in StackOverflowError. This PR fixes the bug and adds testing for it. Reviewers: Guozhang Wang <guozhang@confluent.io>, John Roesler <john@confluent.io>
|
Cherry-picked to 2.0 again. |
|
Thanks @guozhangwang ! |
Due to lack of conversion to kstream Predicate, existing filter method in KTable.scala would result in StackOverflowError. This PR fixes the bug and adds testing for it. Reviewers: Guozhang Wang <guozhang@confluent.io>, John Roesler <john@confluent.io>
4 participants
Due to lack of conversion to kstream Predicate, existing filter method in KTable.scala would result in StackOverflowError.
This PR fixes the bug and adds testing for it.