Hey Jess, I see that you agreed with some of the suggestions and design alternatives, including the naming. Could you please:
- Update the wiki to the committed design? - Fix the motivation wording: existing query types don't "drop" headers... Thanks - Alieh On Mon, Jun 15, 2026 at 10:02 PM Jess Jin via dev <[email protected]> wrote: > Hi Lucas, > > Thanks! I agree. The new type only offers withKey(...) (single key → all > sessions for that key), so "Range" is misleading. I'll rename it to > SessionKeyWithHeadersQuery. That also keeps the "Range" name free for a > future genuine key-range session query. > > - Jess > > On Mon, Jun 15, 2026 at 11:03 AM Lucas Brutschy <[email protected]> > wrote: > > > Hi Jess, > > > > Thanks for the KIP, and for working through the latest round with > > Matthias and Bill. > > > > LB1: For the session query, the existing IQv2 entry point is > > WindowRangeQuery created via withKey(), which returns all sessions for > > a single key. Is "Range" the right word when only withKey() is > > offered? Seems "Range" previously referred to a range of keys, but > > that range is not part of the new type anymore. > > > > Thanks, > > Lucas > > > > On Mon, Jun 15, 2026 at 4:37 PM Jess Jin via dev <[email protected]> > > wrote: > > > > > > Hi Bill, > > > > > > Thanks for the review. I agree this needed to be nailed down. > > > > > > - Jess > > > > > > On Fri, Jun 12, 2026 at 2:41 PM Bill Bejeck <[email protected]> wrote: > > > > > > > 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. > > > > > >>> > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >
