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.

















Reply via email to