Thanks Matthias, those are all good catches. I’ve updated them in the right sections and also added a matching test plan for the non-header supplier + WithHeaders builder case.
- Jess On Wed, Jun 24, 2026 at 5:43 PM Matthias J. Sax <[email protected]> wrote: > Thanks for updating the KIP. It's really well written! > > > > A few nits: > > > MJS5: In the "Proposed Changes" section, the KIP says: > > > As a result, the existing query types (KeyQuery, TimestampedKeyQuery, > RangeQuery, TimestampedRangeQuery on the KV store; WindowKeyQuery, > WindowRangeQuery on the window store; WindowRangeQuery.withKey on the > session store) > > > I think it should be > > `WindowRangeQuery.withWindowStartRange on the window store;` > > > Others part of the KIP specify it correctly already; guess it's just an > oversight. > > > > MJS6a: In the "compatibility" section, you state: > > > ReadOnlyRecord lives in processor.api, so no new dependency is imposed > on the PAPI; IQ (query) depends on processor.api, the correct direction. > > > All these package live in the same module, `kafka-streams` so the > direction does not really matter. We don't get any new dependency in > either case. > > > > MJS6b: Also > > > Headers are returned only when the queried store was built with a > KIP-1271 WithHeaders supplier. Against plain (non-headers) stores the new > query types are unsupported and fail cleanly. > > Do we have a third case, of non-headers-supplier passed into > headers-builder? For this case, the query should succeed, but headers > would of course be empty, because the underlying non-header-supplier > does not allow us to store headers. > > > > -Matthias > > On 6/23/26 6:19 AM, Jess Jin via dev wrote: > > Hey Alieh, > > > > Thanks for the feedback! > > On the AggregationWithHeaders → ReadOnlyRecord mapping: this is actually > > covered in the Proposed Changes section (third bullet under "Handler > > wiring"). > > On the the Motivation wording: I agree; I've reworded it in the wiki. > > > > - Jess > > > > > > On Tue, Jun 23, 2026 at 6:57 AM Alieh Saeedi via dev < > [email protected]> > > wrote: > > > >> Thanks Jess for updating the KIP. > >> > >> Do you think should the KIP explain how AggregationWithHeaders maps onto > >> ReadOnlyRecord (whose timestamp() would be the session-window end). > Right > >> now it's silent. > >> > >> nit: In the `Motivation` section, we have "Against an adapter-built > header > >> store, queries such as KeyQuery… succeed but drop the headers; against a > >> natively-built header store the same queries fail with > UNKNOWN_QUERY_TYPE." > >> The second half is confirmed, but "succeed but drop the headers" is > >> inaccurate. Those query types' result types (ValueAndTimestamp/plain > value) > >> never carried headers, so nothing is being "dropped" — the adapter just > >> exposes the store as a non-header store. Reword to something like > "return > >> only value/timestamp and do not expose headers." > >> > >> -Alieh > >> > >> > >> > >> > >> > >> On Mon, Jun 22, 2026 at 6:28 PM Jess Jin via dev <[email protected]> > >> wrote: > >> > >>> Thanks for the feedback! > >>> > >>> Following the discussion about using Record as the result type, we're > >>> refining the result model. Instead of returning the PAPI Record, IQ > >> header > >>> queries return a new read-only interface ReadOnlyRecord > >>> (key/value/timestamp/headers), which Record implements. > >>> > >>> Reason: returning Record directly exposes its withKey/withValue/… > >> builders, > >>> which are meant for transform-and-forward in processing and do nothing > >> on a > >>> read-only query result (a false affordance). Conceptually, an IQ result > >> and > >>> a Record are the same read concept seen from two layers: ReadOnlyRecord > >> is > >>> the shared read view, and Record extends it with transform-and-forward. > >>> > >>> The change to PAPI is additive only. Record already has those > accessors, > >> so > >>> it just declares implements ReadOnlyRecord (source/binary compatible, > no > >>> behavior change). > >>> > >>> Thanks > >>> - Jess > >>> > >>> On Wed, Jun 17, 2026 at 5:59 PM Alieh Saeedi via dev < > >> [email protected] > >>>> > >>> wrote: > >>> > >>>> Hey Matthias > >>>> > >>>> We store data in SSs in key value form. So the queries return value V > >>> which > >>>> is ValueTimestampHeaders in headers-aware SSs case (and > >> ValueAndTimestamp > >>>> in timestamped SSs). Does returning Record mean that we consider V as > >> an > >>>> intermittent result and then we convert it to Record for the user? > >>>> > >>>> Thanks > >>>> > >>>> -Alieh > >>>> > >>>> On Tue, Jun 16, 2026, 8:28 PM Matthias J. Sax <[email protected]> > >> wrote: > >>>> > >>>>> Thanks. > >>>>> > >>>>> I just had a high level thought. Not sure if this idea is good or > >> bad, > >>>>> so I just wanted to put it out, to see what people think. > >>>>> > >>>>> With the "new" PAPI, we move from KV to `Record` model. For IQ, we > >>>>> historically also use a KV-based design, and extended it > >> incrementally > >>>>> via `ValueAndTimestamp` and most recently via > >> `ValueTimestampHeaders`. > >>>>> > >>>>> While we need `ValueTimestampHeaders` internally (it's the par that > >>> goes > >>>>> into the RocksDB value), I am wondering if it could make sense for > >> IQv2 > >>>>> to also use `Record`? > >>>>> > >>>>> If we use `Record`, we would avoid the `ValueTimestampHeaders` vs > >>>>> `AggregationsWithHeaders` (which also have a ts, but just stored in > >> the > >>>>> RocksDB `SessionKey` object) question, and long term, we might also > >>> move > >>>>> to a `Record` model in the DSL. So we could align and standardize on > >>>>> `Record` throughout the whole API surface area in the long term? > >>>>> > >>>>> > >>>>> Thoughts? > >>>>> > >>>>> > >>>>> -Matthias > >>>>> > >>>>> On 6/16/26 7:19 AM, Jess Jin via dev wrote: > >>>>>> Thanks Alieh! Updated in the wiki. > >>>>>> > >>>>>> - Jess > >>>>>> > >>>>>> On Tue, Jun 16, 2026 at 7:49 AM Alieh Saeedi via dev < > >>>>> [email protected]> > >>>>>> wrote: > >>>>>> > >>>>>>> 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. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>> > >>> > >> > > > >
