Thanks for updating the KIP.

One nit:

The existing methods which accept Named will be marked for deprecation in 4.0.

We can skip `in 4.0`. (1) The next release will be 3.1 (not 4.0) and (2) a KIP could always slip into a future release.


About `TableJoined`: It seems you propose to add static methods for all possible parameter combination. We usually try to avoid this to keep the number of methods low; if we add too many methods, it defeats the purpose to use a "builder like" config object.

To me, it seems sufficient to only have two static methods:

as(final String name);

and

with(final StreamPartitioner<K, Void> partitioner,
     final StreamPartitioner<KO, Void> otherPartitioner);

The second one should allow to pass in `null` to only set one of both partitioners.

Curious to hear what other think.


-Matthias

On 9/20/21 8:27 PM, Victoria Xia wrote:
Hi Matthias,

Thanks for having a look at the KIP! I've updated it with your suggestion
to introduce a new `TableJoined` object with partitioners of type
`StreamPartitioner<K, Void>` and `StreamPartitioner<KO, Void>`, and to
deprecate the existing FK join methods which accept a `Named` object
accordingly. I agree it makes sense to keep the number of join interfaces
smaller.

Thanks,
Victoria

On Sat, Sep 18, 2021 at 11:07 AM Matthias J. Sax <mj...@apache.org> wrote:

Thanks for the KIP Victoria.

As pointed out on the Jira ticket by you, using `<K,V>` and `<KO,VO>` as
partitioner types does not really work, because we don't have access to
the right value on the left side nor have we access to the left value on
the right hand side. -- I like your idea to use `Void` as value types to
make it clear to the users that partitioning must be done on the key only.

For the proposed public API change, I would propose not to pass the
partitioners directly, but to introduce a config object (similar to
`Joined` for stream-table joins, and `StreamJoined` for stream-stream
joins). This new object could also implement `NamedOperation` and thus
replace `Named`. To this end, we would deprecate the existing methods
using `Named` and replace them with the new methods. Net benefit is,
that we don't get more overloads (after we removed the deprecated ones).

Not sure how we want to call the new object. Maybe `TableJoined` in
alignment to `StreamJoined`?


-Matthias

On 9/15/21 3:36 PM, Victoria Xia wrote:
Hi,

I've opened a small KIP for adding Kafka Streams support for foreign key
joins on tables with custom partitioners:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-775%3A+Custom+partitioners+in+foreign+key+joins

Feedback appreciated. Thanks!

- Victoria



Reply via email to