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

sorting by multiple fields #11

Closed
jpmhouston opened this issue Feb 25, 2016 · 6 comments
Closed

sorting by multiple fields #11

jpmhouston opened this issue Feb 25, 2016 · 6 comments

Comments

@jpmhouston
Copy link

I don't know much about this Pod, or CLucene, but I'm faced with either adding multi-field sorting or abandoning this for another solution. Before I waste too much time, can someone say whether this would this work?

In CLuceneSearchService changing the sortBy argument in the internal searchWithQuery:sortBy:sortType:ascending: from an NSString to an NSArray, and its implementation like:

- (id<BRSearchResults>)searchWithQuery:(std::auto_ptr<Query>)query
                              sortBy:(NSArray *)sortFieldNames
                            sortType:(BRSearchSortType)sortType
                           ascending:(BOOL)ascending {
    std::tr1::shared_ptr<Searcher> s = [self searcher];
    if ( sortFieldNames != nil && sortFieldNames.count > 0 ) {
        SortField *sortFields[sortFieldNames.count + 2];
        int idx = 0;
        for (NSString *sortFieldName in sortFieldNames) {
            SortField *sortField = new SortField([sortFieldName asCLuceneString], (sortType == BRSearchSortTypeInteger
                                                                                   ? SortField::INT
                                                                                   : SortField::STRING), !ascending);
            sortFields[idx++];
        }
        SortField *docFieldSort = ascending ? SortField::FIELD_DOC() : new SortField(NULL, SortField::DOC, true);
        sortFields[idx++] = docFieldSort; // ensures we have consistent results if a sort field has duplicate values
        sortFields[idx] = NULL; // requires NULL last element
        std::auto_ptr<Sort> sort(new Sort(sortFields)); // assumes ownership of each SortField
        std::auto_ptr<Hits> hits(s->search(query.get(), sort.get()));
        return [[CLuceneSearchResults alloc] initWithHits:hits sort:sort query:query searcher:s];
    }
    std::auto_ptr<Hits> hits(s->search(query.get()));
    std::auto_ptr<Sort> sort;
    return [[CLuceneSearchResults alloc] initWithHits:hits sort:sort query:query searcher:s];
}

Then making the obvious changes to search:sortBy:.. and searchWithPredicate:sortBy:... to call it with ... sortBy:@[sortFieldName]..., and making additional methods search:sortByMultiple:.. and searchWithPredicate:sortByMultiple:....

I can do a pull request if this works.

@msqr
Copy link
Contributor

msqr commented Feb 25, 2016

Thanks for your interest in the project. As you discovered when looking at the CLucene API, it does support sorting on multiple fields. Updating the BRFTS API to support this has been an outstanding task in the back of my mind for some time, actually. To that end, check out the multi-sort branch, where I've added two new methods:

// parsed query with an array of sort descriptors
- (id <BRSearchResults> )search:(NSString *)query 
            withSortDescriptors:(nullable NSArray<id<BRSortDescriptor>> *)sorts;

// predicate query with an array of sort descriptors
- (id <BRSearchResults> )searchWithPredicate:(NSPredicate *)predicate 
                             sortDescriptors:(nullable NSArray<id<BRSortDescriptor>> *)sorts;

The BRSimpleSortDescriptor provides a basic implementation of BRSortDescriptor for you, and there is a unit test that shows this working. Also note Lucene has limitations on the fields you can sort on.

If you'd like to try out this branch with your project, let me know how it goes for you and if all is well I can merge this into the next release.

@jpmhouston
Copy link
Author

Thanks, I didn't see that branch. I'll investigate whether we can work within Lucene's limitations you mentioned, and if so, then indeed I'll be exercising this branch. Would you like me to email my feedback to you directly @msqr, or post it in this thread?

@msqr
Copy link
Contributor

msqr commented Feb 25, 2016

Great! Go ahead and post any feedback on this thread if you can.

@msqr
Copy link
Contributor

msqr commented Mar 10, 2016

Hi @jpmhouston did you have any luck using the multi-sort branch? I am likely to merge this in for the next release, but would be interested to hear if you tried it out.

@jpmhouston
Copy link
Author

Didn't get a chance yet, and now the project is being shelved & I'm moving on so it looks like I won't ever. :^(

@msqr
Copy link
Contributor

msqr commented Mar 10, 2016

Ah well, I have a few of unit tests in there at least, so I'll merge it in. Thanks!

@msqr msqr closed this as completed Mar 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants