Hi Matthias, thanks for the feedback!
MJS1: Yes, compile-time, not runtime. I reworded that.
MJS2, MJS3, MJS4:
I agree that introducing the new methods will lead to name asymmetry. If we
add a new join/leftJoin overload taking a new functional interface rather
than introducing new methods, all stream-globalTable joins could keep the
same join/leftJoin naming. So I explored the last rejected alternative
further: a new ValueJoinerWithKeys interface `ValueJoinerWithKeys<K1, K2,
V1, V2, VR>` with `apply(readOnlyKey, streamKey, value1, value2)` exposing
both keys, and overloaded the existing join/leftJoin methods to accept it.
The four ValueJoinerWithKey overloads get @Deprecated as in the original
plan, but the migration target is now the new overload rather than a
renamed method. The ValueJoiner overloads stay untouched.
For example, the new `join` method would be:
<GlobalKey, GlobalValue, VOut> KStream<K, VOut> join(final
GlobalKTable<GlobalKey, GlobalValue> globalTable,
final
KeyValueMapper<? super K, ? super V, ? extends GlobalKey> keySelector,
final
ValueJoinerWithKeys<? super GlobalKey, ? super K, ? super V, ? super
GlobalValue, ? extends VOut> joiner);
I think this could be a better solution because the overload works cleanly,
and the joiner allowing both keys offers greater flexibility.
This is some draft code changes this approach would introduce:
https://github.com/apache/kafka/pull/22477
Happy to learn what others think about this approach. If this approach is
indeed better, I will update the KIP.
Best regards,
Lucy Liu
On Wed, Jun 3, 2026 at 4:43 PM Matthias J. Sax <[email protected]> wrote:
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
