Thank you, Matthias and Alieh,
I've initiated a vote.
Sincerely,
Hanyu
On Fri, Oct 13, 2023 at 9:14 AM Matthias J. Sax wrote:
> Thanks for pointing this out Alieh! I totally missed this.
>
> So I guess everything is settled and Hanyu can start a VOTE?
>
> For the KIP PR, we should ensure to up
Thanks for pointing this out Alieh! I totally missed this.
So I guess everything is settled and Hanyu can start a VOTE?
For the KIP PR, we should ensure to update the JavaDocs to avoid
confusion in the future.
-Matthias
On 10/12/23 12:21 PM, Alieh Saeedi wrote:
Hi,
just pointing to javadoc
Hi,
just pointing to javadocs for range() and reverseRange():
range(): *@return The iterator for this range, from smallest to largest
bytes.*
reverseRange(): * @return The reverse iterator for this range, from largest
to smallest key bytes.
Cheers,
Alieh
On Thu, Oct 12, 2023 at 7:32 AM Matthias
Quick addendum.
Some DSL operator use `range` and seems to rely on ascending order,
too... Of course, if we say `range()` has no ordering guarantee, we
would add `forwardRange()` and let the DSL use `forwardRange`, too.
The discussion of course also applies to `all()` and `reveserAll()`, and
Thanks for raising this question Hanyu. Great find!
My interpretation is as follows (it's actually a warning signal that the
API contract is not better defined, and we should fix this by extending
JavaDocs and docs on the web page about it).
We have existing `range()` and `reverseRange()` met
Thank you Colt,
At first, we misinterpreted the JavaDoc. Upon further discussion, we
realized that after the key is converted to bytes, queries are based on the
key's byte order, not its intrinsic order.
Sincerely,
Hanyu
On Mon, Oct 9, 2023 at 6:55 PM Colt McNealy wrote:
> Hanyu,
>
> I like th
Hanyu,
I like the attention to detail!
It is correct that the JavaDoc does not "guarantee" order. However, the
public API contract specified in the javadoc does mention lexicographical
ordering of the bytes, which is a useful API contract. In our Streams app
we make use of that contract during in
After our discussion, we discovered something intriguing. The definitions
for the range and reverseRange methods in the ReadOnlyKeyValueStore are as
follows:
/**
* Get an iterator over a given range of keys. This iterator must be
closed after use.
* The returned iterator must be safe from
Thank you, Matthias, for the detailed implementation and explanation. As of
now, our capability is limited to executing interactive queries on
individual partitions. To illustrate:
Consider the IQv2StoreIntegrationTest:
We have two partitions:
Partition0 contains key-value pairs: <0,0> and <2,2>.
Hi, Hao,
In this case, it will return an empty set or list in the end.
Sincerely,
Hanyu
On Wed, Oct 4, 2023 at 10:29 PM Matthias J. Sax wrote:
> Great discussion!
>
> It seems the only open question might be about ordering guarantees?
> IIRC, we had a discussion about this in the past.
>
>
> T
Great discussion!
It seems the only open question might be about ordering guarantees?
IIRC, we had a discussion about this in the past.
Technically (at least from my POV), existing `RangeQuery` does not have
a guarantee that data is return in any specific order (not even on a per
partitions
Hi Hanyu,
Thanks for the KIP! Seems there are already a lot of good discussions. I
only have two comments:
1. Please make it clear in
```
/**
* Interactive range query using a lower and upper bound to filter the
keys returned.
* @param lower The key that specifies the lower bound of
For testing purposes, we previously used a Set to record the results in
IQv2StoreIntegrationTest. Let's take an example where we now have two
partitions and four key-value pairs: <0,0> in p0, <1,1> in p1, <2,2> in p0,
and <3,3> in p1.
If we execute withRange(1,3), it will return a Set of <1, 2, 3>
Hi, Bruno
Thank you for your suggestions, I will update them soon.
Sincerely,
Hanyu
On Wed, Oct 4, 2023 at 9:25 AM Hanyu (Peter) Zheng
wrote:
> Hi, Lucas,
>
> Thank you for your suggestions.
> I will update the KIP and code together.
>
> Sincerely,
> Hanyu
>
> On Tue, Oct 3, 2023 at 8:16 PM H
Hi, Lucas,
Thank you for your suggestions.
I will update the KIP and code together.
Sincerely,
Hanyu
On Tue, Oct 3, 2023 at 8:16 PM Hanyu (Peter) Zheng
wrote:
> If we use WithDescendingKeys() to generate a RangeQuery to do the
> reveseQuery, how do we achieve the methods like withRange, withU
Hi Hanyu,
Thanks a lot for the KIP! I agree with Bruno's comments about fluent
API / keeping the query classes immutable. Apart from that, on the
technical side this is looking good already!
Two comments on the KIP itself (not the technical part):
- The motivation section could be extended by a
Hi Hanyu,
I agree with what others said about having a `withDescendingOrder()`
method and about to document how the results are ordered.
I would not add a reverse flag and adding a parameter to each method in
RangeQuery. This makes the API less fluent and harder to maintain since
the flag wo
If we use WithDescendingKeys() to generate a RangeQuery to do the
reveseQuery, how do we achieve the methods like withRange, withUpperBound,
and withLowerBound only in this method?
On Tue, Oct 3, 2023 at 8:01 PM Hanyu (Peter) Zheng
wrote:
> I believe there's no need to introduce a method like W
I believe there's no need to introduce a method like WithDescendingKeys().
Instead, we can simply add a reverse flag to RangeQuery. Each method within
RangeQuery would then accept an additional parameter. If the reverse is set
to true, it would indicate the results should be reversed.
Initially, I
Hi, Colt,
The underlying structure of inMemoryKeyValueStore is treeMap.
Sincerely,
Hanyu
On Tue, Oct 3, 2023 at 4:34 PM Hanyu (Peter) Zheng
wrote:
> Hi Bill,
> 1. I will update the KIP in accordance with the PR and synchronize their
> future updates.
> 2. I will use that name.
> 3. you mean add
Hi Bill,
1. I will update the KIP in accordance with the PR and synchronize their
future updates.
2. I will use that name.
3. you mean add something about ordering at the motivation section?
Sincerely,
Hanyu
On Tue, Oct 3, 2023 at 4:29 PM Hanyu (Peter) Zheng
wrote:
> Hi, Walker,
>
> 1. I will
Hi, Walker,
1. I will update the KIP in accordance with the PR and synchronize their
future updates.
2. I will use that name.
3. I'll provide additional details in that section.
4. I intend to utilize rangeQuery to achieve what we're referring to as
reverseQuery. In essence, reverseQuery is merely
Ok, I will change it back to following the code, and update them together.
On Tue, Oct 3, 2023 at 2:27 PM Walker Carlson
wrote:
> Hello Hanyu,
>
> Looking over your kip things mostly make sense but I have a couple of
> comments.
>
>
>1. You have "withDescandingOrder()". I think you mean "des
Ok, I just talked about it with Matthias, I will change the text back to
following the code, and update them together.
Sincerely,
Hanyu
On Tue, Oct 3, 2023 at 3:49 PM Bill Bejeck wrote:
> Hi Hanyu,
>
> Thanks for the KIP, overall it's looking good, but I have a couple of
> comments
>
>- In
Hi Hanyu,
Thanks for the KIP, overall it's looking good, but I have a couple of
comments
- In the “Proposed Changes” section there's a reference to
`setReverse()` but the code example has “withDescendingOrder()” so I think
the text needs an update to reflect the code example.
- I pref
Hello Hanyu,
Looking over your kip things mostly make sense but I have a couple of
comments.
1. You have "withDescandingOrder()". I think you mean "descending" :)
Also there are still a few places in the do where its called "setReverse"
2. Also I like "WithDescendingKeys()" better
3.
Hello Hanyu,
Thank you for the KIP. I agree with Matthias' proposal to keep the naming
convention consistent with KIP-969. I favor the `.withDescendingKeys()`
name.
I am curious about one thing. RocksDB guarantees that records returned
during a range scan are lexicographically ordered by the byte
ok, I will update it. Thank you Matthias
Sincerely,
Hanyu
On Tue, Oct 3, 2023 at 11:23 AM Matthias J. Sax wrote:
> Thanks for the KIP Hanyu!
>
>
> I took a quick look and it think the proposal makes sense overall.
>
> A few comments about how to structure the KIP.
>
> As you propose to not add
Thanks for the KIP Hanyu!
I took a quick look and it think the proposal makes sense overall.
A few comments about how to structure the KIP.
As you propose to not add `ReverseRangQuery` class, the code example
should go into "Rejected Alternatives" section, not in the "Proposed
Changes" secti
29 matches
Mail list logo
