mjsax commented on code in PR #14570: URL: https://github.com/apache/kafka/pull/14570#discussion_r1367565174
########## streams/src/main/java/org/apache/kafka/streams/query/TimestampedKeyQuery.java: ########## @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.streams.query; + +import org.apache.kafka.common.annotation.InterfaceStability.Evolving; +import org.apache.kafka.streams.state.ValueAndTimestamp; + +import java.util.Objects; + +/** + * Interactive query for retrieving a single record based on its key. + */ +@Evolving +public final class TimestampedKeyQuery<K, V> implements Query<ValueAndTimestamp<V>> { + + private final K key; + private final boolean skipCache; + + private TimestampedKeyQuery(final K key, final boolean skipCache) { + this.key = Objects.requireNonNull(key); + this.skipCache = skipCache; + } + + /** + * Creates a query that will retrieve the record identified by {@code key} if it exists + * (or {@code null} otherwise). + * @param key The key to retrieve + * @param <K> The type of the key + * @param <V> The type of the value that will be retrieved + */ + public static <K, V> TimestampedKeyQuery<K, V> withKey(final K key) { + return new TimestampedKeyQuery<>(key, false); + } + + /** + * Specifies that the cache should be skipped during query evaluation. This means, that the query will always + * get forwarded to the underlying store. + */ + public TimestampedKeyQuery<K, V> skipCache() { + return new TimestampedKeyQuery<>(key, true); + } + + /** + * The key that was specified for this query. + */ + public K getKey() { Review Comment: As discussed on the KIP, should be changed to `key()` -- similar elsewhere. ########## streams/src/main/java/org/apache/kafka/streams/query/TimestampedRangeQuery.java: ########## @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +package org.apache.kafka.streams.query; + + +import org.apache.kafka.common.annotation.InterfaceStability.Evolving; +import org.apache.kafka.streams.state.KeyValueIterator; +import org.apache.kafka.streams.state.ValueAndTimestamp; + + +import java.util.Optional; + +/** + * Interactive query for issuing range queries and scans over KeyValue stores. + * <p> + * A range query retrieves a set of records, specified using an upper and/or lower bound on the keys. + * <p> + * A scan query retrieves all records contained in the store. + * <p> Review Comment: Unnecessary tag. -- Also generic parameters should be describe -- we should also describe ordering guarantees better (we can wait for your other PR to get finished, and mimic what we did there here.) ########## streams/src/main/java/org/apache/kafka/streams/query/TimestampedKeyQuery.java: ########## @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.streams.query; + +import org.apache.kafka.common.annotation.InterfaceStability.Evolving; +import org.apache.kafka.streams.state.ValueAndTimestamp; + +import java.util.Objects; + +/** + * Interactive query for retrieving a single record based on its key. + */ Review Comment: We should add `@param <K> ...` (also for `<V>`) to describe the generic parameters ########## streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java: ########## @@ -99,9 +101,17 @@ public class MeteredKeyValueStore<K, V> RangeQuery.class, (query, positionBound, config, store) -> runRangeQuery(query, positionBound, config) ), + mkEntry( + TimestampedRangeQuery.class, Review Comment: Why does `MeteredKeyValueStore` implement this? I thought this would only go into `MeteredTimestampKeyValueStore`? ########## streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java: ########## @@ -288,6 +298,48 @@ protected <R> QueryResult<R> runRangeQuery(final Query<R> query, return result; } + @SuppressWarnings("unchecked") + protected <R> QueryResult<R> runTimestampRangeQuery(final Query<R> query, + final PositionBound positionBound, + final QueryConfig config) { Review Comment: nit: fix indention ########## streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredTimestampedKeyValueStore.java: ########## @@ -47,7 +59,26 @@ public class MeteredTimestampedKeyValueStore<K, V> super(inner, metricScope, time, keySerde, valueSerde); } - + @SuppressWarnings("rawtypes") + private final Map<Class, StoreQueryUtils.QueryHandler> queryHandlers = + mkMap( + mkEntry( + RangeQuery.class, + (query, positionBound, config, store) -> runRangeQuery(query, positionBound, config) Review Comment: I think we need to overwrite `runRangeQuery` in this class, and change the behavior to unwrap the `ValueAndTimestamp` we get from the inner store, to only return "plain V". (Same for `runKeyQuery`.) ########## streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java: ########## @@ -99,9 +101,17 @@ public class MeteredKeyValueStore<K, V> RangeQuery.class, (query, positionBound, config, store) -> runRangeQuery(query, positionBound, config) ), + mkEntry( + TimestampedRangeQuery.class, + (query, positionBound, config, store) -> runTimestampRangeQuery(query, positionBound, config) + ), mkEntry( KeyQuery.class, (query, positionBound, config, store) -> runKeyQuery(query, positionBound, config) + ), + mkEntry( + TimestampedKeyQuery.class, Review Comment: As above. ########## streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java: ########## @@ -312,6 +364,29 @@ protected <R> QueryResult<R> runKeyQuery(final Query<R> query, return result; } + @SuppressWarnings("unchecked") + protected <R> QueryResult<R> runTimestampKeyQuery(final Query<R> query, Review Comment: Same ########## streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredKeyValueStore.java: ########## @@ -288,6 +298,48 @@ protected <R> QueryResult<R> runRangeQuery(final Query<R> query, return result; } + @SuppressWarnings("unchecked") + protected <R> QueryResult<R> runTimestampRangeQuery(final Query<R> query, Review Comment: as above -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
