-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make HazelcastJsonValue work with PagingPredicate #19880
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it looks good.
I believe we should document the sorting behaviour in the API documentation and reference manual, possibly here in the reference manual.
@@ -35,7 +37,7 @@ | |||
* Ill-formatted JSON strings may cause false positive or false negative | |||
* results in queries. {@code null} string is not allowed. | |||
*/ | |||
public final class HazelcastJsonValue { | |||
public final class HazelcastJsonValue implements Comparable<HazelcastJsonValue> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change all languages that has HazelcastJsonValue will be able use PagingPredicate with HazelcastJsonValue, is it right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is right.
I think it might be good to have some ideas regarding this from the SQL team as well. @ivanthescientist can you comment on this? |
I don't think this will affect SQL in any way. Every operation related to JSON will extract String value first anyway. |
HazelcastJsonValue is made Comparable to work with PagingPredicate fixes hazelcast#14888
24ab7f5
to
072f00c
Compare
HazelcastJsonValue is made Comparable to work with PagingPredicate
fixes #14888