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. > > > > >>>>>>>>>> > > > > >>>>>>>>> > > > > >>>>>>>> > > > > >>>>>>> > > > > >>>>>>> > > > > >>>>>> > > > > >>>> > > > > >>> > > > > >> > > > > > > > > > > > > > > > > > > >
