Skip to content

Evaluate rows on addEmptyIndex - #105

Merged
JonoPrest merged 2 commits into
mainfrom
jp/fix-add-empty-index
Aug 6, 2024
Merged

Evaluate rows on addEmptyIndex#105
JonoPrest merged 2 commits into
mainfrom
jp/fix-add-empty-index

Conversation

@JonoPrest

@JonoPrest JonoPrest commented Aug 5, 2024

Copy link
Copy Markdown
Collaborator

Fixes edge case where values already exist in the inMemory store when the lookup by index gets run.

@JonoPrest
JonoPrest requested a review from DZakh August 5, 2024 11:47
@JonoPrest
JonoPrest force-pushed the jp/fix-add-empty-index branch from be16ec7 to a5df722 Compare August 5, 2024 15:32
@JonoPrest JonoPrest mentioned this pull request Aug 5, 2024
@JonoPrest
JonoPrest force-pushed the jp/fix-add-empty-index branch from a5df722 to 8895eb7 Compare August 6, 2024 07:33

@DZakh DZakh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice

Comment on lines 372 to 373
Assert.deepEqual(
mock.loadEntitiesByFieldCalls,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it correct that we still call loadEntitiesByField?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes if it's the first time adding the index 👍🏼

@JonoPrest
JonoPrest merged commit 7a3bdb4 into main Aug 6, 2024
@JonoPrest
JonoPrest deleted the jp/fix-add-empty-index branch August 6, 2024 08:33
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.

2 participants