soin08 commented on code in PR #28590:
URL: https://github.com/apache/flink/pull/28590#discussion_r3506904644
##########
docs/content/docs/libs/state_processor_api.md:
##########
@@ -265,20 +265,20 @@ DataStream<KeyedState> keyRange =
savepoint.readKeyedState(
`SavepointKeyFilter` provides the following factory methods:
-* `SavepointKeyFilter.exact(Object key)` /
`SavepointKeyFilter.exact(Set<Object> keys)` — match a single key or a finite
set of keys.
+* `SavepointKeyFilter.exact(K key)` / `SavepointKeyFilter.exact(Set<K> keys)`
— match a single key or a finite set of keys.
* `SavepointKeyFilter.range(Comparable<?> lower, boolean lowerInclusive,
Comparable<?> upper, boolean upperInclusive)` — match a range. Either bound may
be `null` to leave that side unbounded.
Review Comment:
Docs list the old range signature. Both English & Chinese still say
```
SavepointKeyFilter.range(Comparable<?> lower, …, Comparable<?> upper, …)
```
but the API is now
```
range(K lower, …, K upper, …)>.
```
##########
flink-libraries/flink-state-processing-api/src/main/java/org/apache/flink/state/api/filter/RangeKeyFilter.java:
##########
@@ -23,7 +23,7 @@
import java.util.Set;
/** A filter based on a comparable range. */
-final class RangeKeyFilter implements SavepointKeyFilter {
+final class RangeKeyFilter<K extends Comparable<K>> implements
SavepointKeyFilter<K> {
Review Comment:
The constraint `<K extends Comparable<K>>` is decorative here. The class
stores `BoundInfo` (raw
`Comparable<?>`, decoupled from `K`) and compares via the erased
`compare(Comparable<?>, Object)` helper.
If `BoundInfo` is made generic on the key type, `RangeKeyFilter.test`
becomes genuinely type-safe:
```
public final class BoundInfo<K extends Comparable<K>> implements
Serializable {
private final K value;
...
public K getValue() { return value; }
}
```
```
final class RangeKeyFilter<K extends Comparable<K>> implements
SavepointKeyFilter<K> {
@Nullable private final BoundInfo<K> lower;
@Nullable private final BoundInfo<K> upper;
@Override
public boolean test(K key) {
if (lower != null) {
int cmp = lower.getValue().compareTo(key); // no cast
...
}
```
The `compare(Comparable<?>, Object)` helper disappears entirely, and
`intersectRange/tighter` also become cast-free `(K.compareTo(K)`).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
