Skip to content

Cap merged Document search results at the requested limit - #553

Open
raphyabak wants to merge 1 commit into
nextapps-de:masterfrom
raphyabak:fix/merge-results-respect-limit
Open

Cap merged Document search results at the requested limit#553
raphyabak wants to merge 1 commit into
nextapps-de:masterfrom
raphyabak:fix/merge-results-respect-limit

Conversation

@raphyabak

Copy link
Copy Markdown

Summary

Fixes #532.

Document.search() with { merge: true } runs each field's search independently with the given limit, then merges the per-field results by unique id. When different fields match different, non-overlapping sets of ids, the merged total can exceed limit by up to (number of fields × limit), since nothing re-applies the cap after merging:

const index = new FlexSearch.Document({
    document: {
        id: "id",
        index: ["name_1", "name_2", "name_3"]
    }
});

for (let i = 0; i < 30; i++) {
    index.add({ id: i, name_1: names[i % 2], name_2: names[(i + 1) % 2], name_3: names[i % 2] });
}

index.search("foo", { merge: true, limit: 10 }).length; // 20, not 10

Fix

The overall limit requested by the caller is captured before the per-field loop can override it for individual fields, and merge_fields() now stops collecting once that many unique results have been found.

Test plan

  • Added a test in test/document.js reproducing the reported case (three fields matching non-overlapping id sets, merge: true, limit: 10), asserting the merged result length is exactly 10.
  • Verified the regression: reverting the source change turns the new test from passing (10) into a failure (20), confirming it actually catches the bug.
  • Full suite passes locally: npx mocha test/*.js --exit — 129 passing, 1 pending (pre-existing, unrelated to this change).

Fixes nextapps-de#532.

Document.search() with { merge: true } applies "limit" to each field's
search independently, then merges the per-field results by unique id.
When different fields match different, non-overlapping sets of ids, the
merged total can exceed "limit" by up to (number of fields x limit),
since nothing re-applies the cap after merging.

Capture the overall requested limit before per-field options can
override it, and stop merge_fields() once that many unique results have
been collected.
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.

Using limit with merge while using document search can give more results than limit.

1 participant