Hi Jess, Thanks for the KIP.
I'd second the importance of getting the details flushed out in Matthias's 4th comment with regards to session stores and how we'll support `ValueTimestampeHeader` and `AggregationWithHeaders` and if session-header-stores are supported in this KIP. -Bill On Thu, Jun 11, 2026 at 8:59 PM Matthias J. Sax <[email protected]> wrote: > Thanks for the KIP. Very nice to see that IQv2 gets more love. > > > Couple of comments/questions. > > > MJS1: The KIP says. > > > surfaced through the public wrapper type ValueTimestampHeaders<V> > > Yes, but session-header-store actually uses `AggregationWithHeaders`. > Should we mention both cases? Or do we not include session-headers > stores? Later, the KIP only mentions > `MeteredTimestampedKeyValueStoreWithHeaders` and > `MeteredTimestampedWindowStoreWithHeaders`, so maybe that's indeed what > is proposed? If yes, why do exclude session-header-store? Also if, yes, > would be good to call this out explicitly. > > > > MJS2: The KIP says. > > > The existing query types — TimestampedKeyQuery, TimestampedRangeQuery, > WindowKeyQuery, and WindowRangeQuery — all return values without headers, > even when run against a header-aware store. > > This sound as if these query types are already supported, but I believe > they are not. Same for their timestamp-less equivalents like `KeyQuery`. > > I think it would be a good extension to the KIP, to also add support for > these existing query types on the newly added header-stores. We don't > need to define any new classed. The KIP would just say, that we extend > the kv-ts-header store to also support existing KeyQuery and > TimestampedKeyQuery (and similar for window-header-store; and maybe also > session-header-store, in case the KIP does include session-header-stores) > > > > MJS3: Question about naming. You propose to add: > > - TimestampedKeyWithHeadersQuery > - TimestampedRangeWithHeadersQuery > - WindowKeyWithHeadersQuery > - WindowRangeWithHeadersQuery > > But all four query types return some form of `ValueTimestampHeaders` > results. So it seems we should align the names, and maybe call the later > two > > - TimestampedWindowKeyWithHeadersQuery > - TimestampedWindowRangeWithHeadersQuery > > Or, to avoid very long names, shorten the first two to > > - KeyWithHeadersQuery > - RangeWithHeadersQuery > > > > MJS4: About `WindowRangeWithHeadersQuery`. > > First some background: The existing `WindowRangeQuery` has two methods > how to setup the query, and depending which one is used, the query can > be used against either a windowed-store or a session-store, ie, while > both are a `WindowRangeQuery`, it's technically two different queries > for two different stores. > > - `withKey(...)` -> only supported by session stores > - `withWindowStartRange(...)` -> only supported by window store > > The KIP proposed to add both methods, too, but at the same time says, > that window-store won't accept a query created via `withKey` (what in > general aligns to what we have). But the KIP seems to exclude > session-header-store support. So why would we add `withKey` at all? > Would it not be simpler to only add the supported `withWindowStartRange` > method? > > Of course, if we want to mimic the existing query type, the > WindowRangeWithHeadersQuery seems to face the challenge that > window-header-store uses `ValueTimestampeHeader` while > session-header-stores uses `AggregationWithHeaders` type. I would like > to understand better how this should all fit together. Also for the > case, that we add session-header-store support later (ie, we should > avoid to corner ourselves and ensure what we define now, is extensible > in the future). > > > > -Matthias > > > > On 6/11/26 6:23 AM, Jess Jin via dev wrote: > > Hey Alieh, > > > > Thanks for the feedback! Updated in the wiki page. > > > > - Jess > > > > On Wed, Jun 10, 2026 at 3:26 PM Alieh Saeedi <[email protected]> > wrote: > > > >> Hey Jess, > >> > >> Thanks for putting together the KIP. > >> > >> A couple of small nits: > >> > >> AS1: Please replace the `getX()` methods with `x()`. For example, > >> `getKey()` should become `key()`. As far as I know, that’s the usual > >> convention. > >> AS2: In the `Usage Example` section, please replace `long` with `Long`, > >> since `ValueTimestampHeaders.value` is not a primitive. > >> > >> - Alieh > >> > >> On Wed, Jun 10, 2026 at 7:02 PM Jess Jin via dev <[email protected]> > >> wrote: > >> > >>> Hi all, > >>> > >>> I'd like to start a discussion on KIP-1356: Introduce IQv2 for > >>> headers-aware state stores. Link > >>> < > >>> > https://urldefense.com/v3/__https://cwiki.apache.org/confluence/display/KAFKA/KIP-1356*3A*Introduce*IQv2*for*headers-aware*state*stores__;JSsrKysrKw!!Ayb5sqE7!oZFamFyZDrAmJeFHgKc-nOWUWL85bEbfHSdwgrp9jNHVOUCZgoOQ9aiK94L8zPr-tA_zx3WTmNs9vVDlqgQ$ > >>>> > >>> > >>> Thanks. > >>> > >> > > > >
