Thanks for the KIP Lucy.
Overall, it seems to be a non-controversial proposal, to solve the issue
with new overloads.
MJS1: The KIP says:
The discrepancy is not surfaced at runtime
What this paragraph describe is a compile time type check, right? Not a
runtime check?
MJS2: It seems you propose to replace only the 4 methods which take
`ValueJoinerWithKey` argument. While this is the smallest change we
could make, I am wondering if it introduced some name inconsistencies?
So even if technically not necessary, I am wondering if we would get
better UX, if we would also deprecate the 4 method taking `ValueJoiner`?
This way, all method which are doing the same thing, ie
stream-globalTable join have the same name.
MJS3: About naming the new methods. I guess both `joinOnMappedKey` and
`joinByMappedKey` would work. But I am wondering if
`foreignKeyJoin()`would be good names, too? Or maybe
`globalKTableJoin()`? Just putting forward some alternatives to spark
some thoughts. Naming is always hard. Curious to hear what others think.
MJS4: About the last rejected alternative. If my assessment is right, it
would have the advantage that we could add new overloads called
`join()`, and thus we would get better method naming overall, and my
point MJS2 from above would become void? I does have the disadvantage
that we need a new interface, but given the naming question, I am
wondering if it would provide an overall better tradeoff? -- Again,
curious to hear what others think.
-Matthias
On 5/27/26 10:17 PM, Lucy Liu via dev wrote:
Hi Bill,
Thanks for the suggestions! I have made some changes accordingly.
BB1: I changed the 4 added methods to use `by` instead of `on`.
BB2: For each deprecated method, an additional warning comment will be
added, stressing that the `readOnlyKey` is the stream key and might not be
behaving as intended.
Best,
Lucy
On Wed, May 27, 2026 at 4:07 PM Bill Bejeck <[email protected]> wrote:
Hi Lucy,
Thanks for the KIP! This is a very much needed update to Kafka Streams.
Overall the KIP looks good.
BB1. I have one nit comment with regards to the name. What would you think
about `joinByMappedKey / leftJoinByMappedKey` instead?
BB2. Since the error condition will continue to exist until we remove the
deprecated methods, maybe we could add one line to the Javadoc explicitly
stating that readOnlyKey is currently the stream key, not the mapped join
key.
Thanks,
Bill
On Tue, May 26, 2026 at 1:19 PM Lucy Liu via dev <[email protected]>
wrote:
Hi everyone,
Gentle ping on this thread. Looking forward to feedback on the KIP!
Best,
Lucy Liu
On Fri, May 15, 2026 at 4:19 PM Lucy Liu <[email protected]> wrote:
Hello everyone,
I would like to start a discussion on KIP-1340 Pass Join Key to
`ValueJoinerWithKey` in Streams-GlobalKTable Joins
<
https://urldefense.com/v3/__https://cwiki.apache.org/confluence/display/KAFKA/KIP-1340*3A*Pass*Join*Key*to**A60ValueJoinerWithKey*60*in*Streams-GlobalKTable*Joins__;JSsrKysrJSUrKys!!Ayb5sqE7!sMsc7OEJtzyBJn3kfDmAjAi8gklV-8hDbb2Oiwk_NV0RC8giJPWimU7sX4jc4Niu_29yjxAv1souD4H5kUY$
This proposal aims to introduce joinOnMappedKey and
leftJoinOnMappedKey,
new KStream methods that pass the mapped join key (the result of the
user-supplied KeyValueMapper) into ValueJoinerWithKey for
stream-globalTable joins. The existing join/leftJoin overloads pass the
stream record's key into the joiner instead, contradicting KIP-149's
contract that readOnlyKey is the join key and silently producing wrong
values if a joiner intends to read the real join key. The existing
overloads will be deprecated and removed in a future major release.
Best regards,
Lucy Liu
