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

Occur::MustNot does not seem to be working as expected #2317

Open
hermeGarcia opened this issue Feb 14, 2024 · 2 comments
Open

Occur::MustNot does not seem to be working as expected #2317

hermeGarcia opened this issue Feb 14, 2024 · 2 comments

Comments

@hermeGarcia
Copy link

Describe the bug

  • What did you do?
    Write a negative query using Occur::MustNot and a BooleanQuery.

  • What happened?
    Seem like the negative query was not iterating through the candidates on its own. I added (Occur::Must, Box::new(AllQuery)) to the BooleanQuery and then got the expected behaviour.

  • What was expected?
    I would expect for a negative query to iterate through the candidates on its own, without needing an AllQuery.

Which version of tantivy are you using?
0.21.1

To Reproduce

    use tantivy::collector::DocSetCollector;
    use tantivy::query::{AllQuery, BooleanQuery, Occur, TermQuery};
    use tantivy::schema::IndexRecordOption;
    use tantivy::schema::Schema;
    use tantivy::schema::Term;
    use tantivy::schema::{STORED, STRING};
    use tantivy::Index;

    let dir = tempfile::tempdir().unwrap();
    let mut sb = Schema::builder();
    let uuid = sb.add_text_field("uuid", STRING | STORED);
    let schema = sb.build();
    let index_builder = Index::builder().schema(schema.clone());
    let index = index_builder
        .create_in_dir(&dir.path())
        .expect("Index directory should exist");

    let mut writer = index.writer_with_num_threads(1, 15000000).unwrap();
    let this_doc = tantivy::doc!(
        uuid => "this"
    );
    let that_doc = tantivy::doc!(
        uuid => "that"
    );
    writer.add_document(this_doc).unwrap();
    writer.add_document(that_doc).unwrap();
    writer.commit().unwrap();

    let reader = index.reader().unwrap();
    let searcher = reader.searcher();
    assert_eq!(searcher.num_docs(), 2);

    let query = TermQuery::new(
        Term::from_field_text(uuid, "this"),
        IndexRecordOption::Basic,
    );
    let collector = DocSetCollector;
    let results = searcher.search(&query, &collector).unwrap();
    assert_eq!(results.len(), 1);

    // For some reason this query yields no results, although I would expect one to be returned.
    let unexpected_result_query =
        BooleanQuery::new(vec![(Occur::MustNot, Box::new(query.clone()))]);
    let results = searcher
        .search(&unexpected_result_query, &collector)
        .unwrap();
    assert_eq!(results.len(), 0);

    // Turns out we can get the query to work as expected by adding an AllQuery
    let bypass = BooleanQuery::new(vec![
        (Occur::Must, Box::new(AllQuery)),
        (Occur::MustNot, Box::new(query)),
    ]);
    let results = searcher.search(&bypass, &collector).unwrap();
    assert_eq!(results.len(), 1);
@fulmicoton
Copy link
Collaborator

I think we should stick to that specification. I believe the query parser does the thing you expected however. (not 100% sure)

@adamreichold
Copy link
Contributor

I believe the query parser does the thing you expected however. (not 100% sure)

It fails with

Invalid query: Only excluding terms given

which does make sense IMHO, i.e. I agree to keep the current behaviour.

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

No branches or pull requests

3 participants