vvcephei commented on a change in pull request #11581:
URL: https://github.com/apache/kafka/pull/11581#discussion_r766220034



##########
File path: 
streams/src/main/java/org/apache/kafka/streams/state/internals/StoreQueryUtils.java
##########
@@ -62,4 +71,32 @@ public static void updatePosition(
             position.withComponent(meta.topic(), meta.partition(), 
meta.offset());
         }
     }
+
+    public static boolean isPermitted(
+        final Position position,
+        final PositionBound positionBound,
+        final int partition
+    ) {
+        if (positionBound.isUnbounded()) {

Review comment:
       Ok, so the situation is a little different now than when I originally 
added a separate PositionBound class. At that point, we also needed a bound 
representing "latest" (i.e., that the query is executed on an active running 
task), for which there's no sensible Position representation.
   
   My mind was still half in that world last time we spoke about this point. 
Now, I can see that the only special position bound is the "unbounded" one, 
which is the same thing semantically as an empty position.
   
   I just tried out a change to completely get rid of the PositionBound class, 
but I think it makes the API more confusing. As a user, it seems more clear to 
create an "unbounded" PositionBound than an "empty" Position. I think it makes 
sense if you think about in terms of comparing vectors (an empty vector is 
"less than" all other vectors, so when it's used as a lower bound, it permits 
everything). But I don't want people to have to think that hard about it.
   
   Another option I considered is to add a `Position.unbounded()` factory, but 
that doesn't completely make sense either, since a Position is just a point in 
vector space. It doesn't bound anything by itself, though it can be used as a 
bound.
   
   Plus, I think query handling implementation, both for Streams and for custom 
user stores, is easier to keep track of if you have two types. You simply can't 
mix up which Position was supposed to be the bound.
   
   On balance, it still seems better to keep the separate PositionBound class. 
However, I did realize that the logic of PositionBound and the internal logic 
in Streams can be simplified with this observation that an unbounded position 
is equivalent to an empty position. I just added that commit to this PR.
   
   Thanks!




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


Reply via email to