gaborgsomogyi commented on code in PR #28590:
URL: https://github.com/apache/flink/pull/28590#discussion_r3507910357
##########
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:
Adding `K extends Comparable<K>` directly to `BoundInfo` and
`RangeKeyFilter` would propagate the bound upward through
`SavepointKeyFilter<K>`, then into `KeyedStateInputFormat<K>`, and finally into
the `@PublicEvolving` 4-arg `readKeyedState` method. That last step is a
breaking API change we cannot make.
Instead, the new commit takes the approach Flink already uses for range
operations (`CommonRangeBoundaries`, `TypeComparator`): inject a
`Comparator<K>` rather than require `K extends Comparable<K>`.
The result is the same type-safety the reviewer asked for, without the
cascade:
- `BoundInfo<K>` is now a plain typed container -- `getValue()` returns `K`,
no cast
- `RangeKeyFilter<K>` holds a `Comparator<K>` field; `test()`, `tighter()`,
and `intersectRange()` all use `comparator.compare(a, b)` -- fully type-safe,
the `compare(Comparable<?>, Object)` helper is gone
- `K extends Comparable<K>` appears only on the convenience `range()`
overload (same as before), which delegates to the new general `range(...,
Comparator<K>)` overload via `Comparator.naturalOrder()`
- `SavepointKeyFilter<K>`, `KeyedStateInputFormat<K>`, and the
`@PublicEvolving` `readKeyedState` remain unconstrained -- no breaking change
--
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]
