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

search on all token during where searches #473

Merged
merged 1 commit into from
Sep 3, 2023

Conversation

philippebeaulieu
Copy link
Contributor

@philippebeaulieu philippebeaulieu commented Aug 31, 2023

We discovered a bug while using search.where with multi token criterias. With the previous logic, only the first token was used for search.

My following pull request includes a test that was failing without this fix.

ex: defining these 4 documents:

const id1 = await insert(db, {
    name: 'super coffee maker',
    rating: 5,
    price: 900,
    meta: {
      sales: 100,
      finish: 'black matte',
    },
  })

  const id2 = await insert(db, {
    name: 'washing machine',
    rating: 5,
    price: 900,
    meta: {
      sales: 100,
      finish: 'gloss black',
    },
  })

  const id3 = await insert(db, {
    name: 'coffee maker',
    rating: 3,
    price: 30,
    meta: {
      sales: 25,
      finish: 'gloss blue',
    },
  })

  const id4 = await insert(db, {
    name: 'coffee maker deluxe',
    rating: 5,
    price: 45,
    meta: {
      sales: 25,
      finish: 'blue matte',
    },
  })

And doing this search:

const result = await search(db, {
      where: {
        'meta.finish': 'black matte',
      },
    })

resulted in only 2 results. When tokenized the where criteria was converted to ["black", "matte"] and on line searchByWhereClause:479 only the first token was used resulting on only document 1 and 2 being returned.

Expected values: doc 1, 2 and 4.

This PR modifies the behavior to search on all tokens.

@vercel
Copy link

vercel bot commented Aug 31, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
orama-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 31, 2023 7:04pm

Copy link
Member

@micheleriva micheleriva left a comment

Choose a reason for hiding this comment

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

that looks neat. Thanks! lgtm

@micheleriva micheleriva merged commit 3050356 into askorama:main Sep 3, 2023
3 checks passed
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

2 participants