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

[BUG] Duplicate hits when overriding id #100

Closed
aaronhuggins opened this issue Aug 22, 2022 · 7 comments
Closed

[BUG] Duplicate hits when overriding id #100

aaronhuggins opened this issue Aug 22, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@aaronhuggins
Copy link

Describe the bug
When overriding the property id of documents, search will return more hits than the actual count of hits in the search results. The additional hits will be duplicated documents.

To Reproduce
Steps to reproduce the behavior:

Use this sample script:

import { create, insert, search } from '@nearform/lyra';

const db = create({
	schema: {
		id: 'string',
		quote: 'string',
		author: 'string',
		what: 'string',
		who: 'string',
	},
});

insert(db, {
	id: '1.0',
	quote: 'It is during our darkest moments that we must focus to see the light.',
	author: 'Aristotle',
	what: '',
	who: '',
});

insert(db, {
	id: '2.0',
	quote: 'If you really look closely, most overnight successes took a long time.',
	author: 'Steve Jobs',
	what: '',
	who: '',
});

insert(db, {
	id: '3.0',
	quote: 'If you are not willing to risk the usual, you will have to settle for the ordinary.',
	author: 'Jim Rohn',
	what: '',
	who: '',
});

insert(db, {
	id: '4.0',
	quote: 'You miss 100% of the shots you don\'t take',
	author: 'Wayne Gretzky - Michael Scott',
	what: '',
	who: '',
});

const searchResult = search(db, {
	term: 'if',
	properties: '*',
});

console.log(searchResult);

Sample output:

{
  elapsed: 169640n,
  hits: [
    {
      id: '2.0',
      quote: 'If you really look closely, most overnight successes took a long time.',
      author: 'Steve Jobs',
      what: '',
      who: ''
    },
    {
      id: '3.0',
      quote: 'If you are not willing to risk the usual, you will have to settle for the ordinary.',
      author: 'Jim Rohn',
      what: '',
      who: ''
    },
    {
      id: '2.0',
      quote: 'If you really look closely, most overnight successes took a long time.',
      author: 'Steve Jobs',
      what: '',
      who: ''
    },
    {
      id: '3.0',
      quote: 'If you are not willing to risk the usual, you will have to settle for the ordinary.',
      author: 'Jim Rohn',
      what: '',
      who: ''
    },
    {
      id: '2.0',
      quote: 'If you really look closely, most overnight successes took a long time.',
      author: 'Steve Jobs',
      what: '',
      who: ''
    },
    {
      id: '3.0',
      quote: 'If you are not willing to risk the usual, you will have to settle for the ordinary.',
      author: 'Jim Rohn',
      what: '',
      who: ''
    },
    {
      id: '2.0',
      quote: 'If you really look closely, most overnight successes took a long time.',
      author: 'Steve Jobs',
      what: '',
      who: ''
    },
    {
      id: '3.0',
      quote: 'If you are not willing to risk the usual, you will have to settle for the ordinary.',
      author: 'Jim Rohn',
      what: '',
      who: ''
    }
  ],
  count: 2
}

Expected behavior
It is expected that the hits array contains no more than count: n and no duplicated documents.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Fedora 36
  • Browser: N/A
  • Version: @nearform/lyra 0.0.4, 0.0.5

Smartphone (please complete the following information):
N/A

Additional context
Bug is reproducible under Node 16, 17, and 18, and Deno 1.24.x (using import of https://cdn.skypack.dev/@nearform/lyra@v0.0.5?dts)

@micheleriva micheleriva added the bug Something isn't working label Aug 22, 2022
@micheleriva
Copy link
Member

Thank you @aaronhuggins ,
that looks like a bug. We're on it

@stearm
Copy link
Contributor

stearm commented Aug 23, 2022

@micheleriva In this case it would be necessary to decide whether to override the id that the engine inserts or keep them both and understand how to manage them. Am I right? Do you have something in mind?

@micheleriva
Copy link
Member

Personally, I would discourage allowing users to define their own IDs. I think that trying to override the id should throw an error.

@stearm
Copy link
Contributor

stearm commented Aug 23, 2022

Personally, I would discourage allowing users to define their own IDs. I think that trying to override the id should throw an error.

Yes totally, it is prone to errors as Lyra relies on the id internally. If you agree, I can take charge of this.

@micheleriva
Copy link
Member

@stearm pushing a commit right now 😄

@micheleriva
Copy link
Member

@aaronhuggins to be released in v0.0.6

@mateonunez
Copy link
Collaborator

mateonunez commented Aug 23, 2022

I did the same search but using properties:

const searchResult = search(db, {
	term: 'if',
	properties: ['quote']
});

Output:

{
  elapsed: 136800n,
  hits: [
    {
      id: '2.0',
      quote: 'If you really look closely, most overnight successes took a long time.',
      author: 'Steve Jobs',
      what: '',
      who: ''
    },
    {
      id: '3.0',
      quote: 'If you are not willing to risk the usual, you will have to settle for the ordinary.',
      author: 'Jim Rohn',
      what: '',
      who: ''
    }
  ],
  count: 2
}

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

No branches or pull requests

4 participants