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

Order of insertion of words with identical wordstem changes result set. #6

Open
SirGrandmasterr opened this issue Mar 31, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@SirGrandmasterr
Copy link

SirGrandmasterr commented Mar 31, 2022

Hi,
while using this package to include a little form of an elasticsearch-like prefix-suggester, I've noticed that some words would be omitted in the results in some cases. A specific example:
"suspenseful" and "suspense".

I've created two testcases using only those two words that should, as far as I understand it, yield the same expected result.
For some reason, one of those tests will return the expected two strings that were inserted, but the other will only return the longer one.
Is this intended behavior?
{ name: "Word order small => big", dict: []string{"suspense", "suspenseful"}, trie: New(), search: "susp", expected: []string{ "suspense", "suspenseful", }, }, { name: "Word order big => small", dict: []string{"suspenseful", "suspense"}, trie: New(), search: "susp", expected: []string{ "suspense", "suspenseful", }, },

Best Regards,
Phillip

@glaslos
Copy link
Collaborator

glaslos commented Apr 1, 2022

Hi Phillip,
That doesn't sound correct. Would you mind providing a PR with your tests?

@SirGrandmasterr
Copy link
Author

Hi @glaslos,
I already tried to do so and seem to not have the repo permissions to create an upstream branch. Hence the wonkily copied code fragments in the original comment. :D

@glaslos
Copy link
Collaborator

glaslos commented Apr 1, 2022

Hi @SirGrandmasterr ,
Yes, you would need to fork the project and then create a PR from the fork.
I have some time later today and I'll also try to reproduce the issue with your code snippet.

@glaslos glaslos added the bug Something isn't working label Apr 1, 2022
@SirGrandmasterr
Copy link
Author

Hi @glaslos,
I've forked and made the PR.
Thanks for taking a look!

@glaslos
Copy link
Collaborator

glaslos commented Mar 1, 2023

Just found your PR again, sorry for the silence. I had a look at the code and it seems this is intentional behavior (although questionable):

if you insert suspense and then suspenseful, the trie created looks like this:

suspense
        \ful

if you switch the order, you get

suspenseful

representing both provided values.
I think what should happen is the suspenseful node should be split up in two as in the previous example.

Would you be comfortable attempting to make that change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants