Filed hotfix PR https://github.com/apache/kafka/pull/991.
On Mon, Feb 29, 2016 at 3:34 PM, Guozhang Wang <wangg...@gmail.com> wrote: > Hi Avi, > > Thanks for pointing it out! And yes it is indeed a bug in the code. Could > you file a HOTFIX PR fixing this and also modify the existing unit test to > cover this case? > > Thanks, > Guozhang > > On Mon, Feb 29, 2016 at 2:15 PM, Avi Flax <a...@aviflax.com> wrote: > >> I was just playing around with Streams’ join features, just to get a >> feel for them, and I think I may have noticed a bug in the code, in >> KStreamImpl.java on line 310: >> >> >> https://github.com/apache/kafka/blob/845c6eae1f6c6bcf117f5baa53bb19b4611c0528/streams/src/main/java/org/apache/kafka/streams/kstream/internals/KStreamImpl.java#L310 >> >> (I’m linking to the latest commit that changed this file so that the >> link will be stable, but line 310 is currently identical in this >> commit and trunk.) >> >> the line reads: >> >> .withValues(otherValueSerializer, otherValueDeserializer) >> >> but I think maybe it’s supposed to read: >> >> .withValues(thisValueSerializer, thisValueDeserializer) >> >> I took a look at the tests and it seems they’re not catching this >> because in the current tests, the serdes for both streams are the same >> — it might be a good idea to add a test wherein they’re different. >> >> If Streams was stable I’d offer to prepare a PR but given that it’s a >> WIP I figured it would be better to just share this observation. >> >> HTH! >> >> Avi >> > > > > -- > -- Guozhang > -- -- Guozhang
