Skip to content
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

feat: Sorting for search results #121

Merged
merged 3 commits into from Aug 15, 2022
Merged

feat: Sorting for search results #121

merged 3 commits into from Aug 15, 2022

Conversation

adilansari
Copy link
Contributor

@adilansari adilansari commented Aug 12, 2022

Interface to allow users to specify sort order for search results.

TigrisSort asc = Sort.ascending("a");
TigrisSort desc = Sort.descending("b");

SearchRequest req = SearchRequest.matchAll().withSort(asc, desc).build();

OR

TigrisSort asc = Sort.ascending("a");
TigrisSort desc = Sort.descending("b");
SortingOrder order = SortingOrder.withOrder(asc, desc).build()

SearchRequest req = SearchRequest.newBuilder().withQ("running").withSort(order).build();

@codecov-commenter
Copy link

codecov-commenter commented Aug 12, 2022

Codecov Report

Merging #121 (68d9ccc) into main (e942711) will increase coverage by 0.46%.
The diff coverage is 91.42%.

@@             Coverage Diff              @@
##               main     #121      +/-   ##
============================================
+ Coverage     83.00%   83.47%   +0.46%     
- Complexity      616      646      +30     
============================================
  Files            68       72       +4     
  Lines          1948     2015      +67     
  Branches        166      173       +7     
============================================
+ Hits           1617     1682      +65     
- Misses          296      297       +1     
- Partials         35       36       +1     
Impacted Files Coverage Δ
.../tigrisdata/db/client/search/FacetFieldsQuery.java 87.50% <50.00%> (-3.62%) ⬇️
.../com/tigrisdata/db/client/search/SortingOrder.java 86.66% <86.66%> (ø)
.../main/java/com/tigrisdata/db/client/FieldSort.java 100.00% <100.00%> (ø)
...t/src/main/java/com/tigrisdata/db/client/Sort.java 100.00% <100.00%> (ø)
.../main/java/com/tigrisdata/db/client/SortOrder.java 100.00% <100.00%> (ø)
...n/java/com/tigrisdata/db/client/TypeConverter.java 93.56% <100.00%> (+1.16%) ⬆️
...com/tigrisdata/db/client/search/SearchRequest.java 95.83% <100.00%> (+4.52%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

import java.util.Objects;

/** Represents a collection field and corresponding sort order */
public final class FieldSort implements TigrisSort {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In future we can extend this further

class GeoDistanceSort implements TigrisSort {}
Sort.geoDistance(String fieldName, GeoLocation loc)

}

@Override
public boolean equals(Object o) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: in other places in this library. I have been using IDE generated equals and hashCode (Java 7+) with java.util.Objects

* @return {@link SearchRequest.Builder}
*/
public Builder withSortOrder(SortOrder sortOrder) {
public Builder withSort(SortingOrder sortOrder) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the use cases when this method is useful vs the following?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a rpc method test to exercise the full trip?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow sharing of SortingOrder between search requests. Or if we introduce sorting for read.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense.

@adilansari adilansari merged commit edc16eb into main Aug 15, 2022
@adilansari adilansari deleted the sorting branch August 15, 2022 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants