Skip to content

Fix EuiSearchBar negated multi-value filter excluding only one value#9783

Open
gunjanjaswal wants to merge 3 commits into
elastic:mainfrom
gunjanjaswal:fix/searchbar-mustnot-array-demorgan
Open

Fix EuiSearchBar negated multi-value filter excluding only one value#9783
gunjanjaswal wants to merge 3 commits into
elastic:mainfrom
gunjanjaswal:fix/searchbar-mustnot-array-demorgan

Conversation

@gunjanjaswal

Copy link
Copy Markdown

Summary

What: Fixes EuiSearchBar / executeAst returning items that a negated multi-value field clause should exclude.

Why: A query like -name:(john or doe) parses to AST.Field.mustNot.eq('name', ['john', 'doe']). fieldClauseMatcher negated the operator per element and then combined the results with .some, computing (name !== 'john') OR (name !== 'doe'). By De Morgan, NOT (a OR b) must be (NOT a) AND (NOT b), so the code used the wrong quantifier and kept rows it should have hidden.

Concrete repro: filtering [{name:'john'},{name:'doe'},{name:'foo'}] with -name:(john or doe) returned all three rows instead of just foo. The scalar path (-name:john) and the positive-array path (name:(a or b)) were already correct; only must_not combined with an array value was wrong.

How: Compute the positive match first, then negate the aggregate result for must_not clauses (isMust ? positiveHit : !positiveHit) instead of negating each element. Scalar and positive-array behavior is unchanged.

API Changes

component / parent prop / child change description
None

Screenshots

Not applicable. This is a logic-only fix to the in-memory filter with no visual change.

Impact Assessment

  • 🧪 Test impact — Adds a Jest case for the must_not multi-value combination; existing search_bar tests are unchanged.

Impact level: 🟢 Low

Release Readiness

  • Documentation (no doc change)
  • Figma (not applicable)
  • Migration guide (bug fix, no migration)
  • Adoption plan (not a feature)

QA instructions for reviewer

  • In Storybook open an EuiSearchBar / in-memory table demo, or call executeAst directly.
  • Filter a dataset containing john, doe, foo with -name:(john or doe) and confirm only foo remains (before this PR all three remained).
  • Confirm -name:john, name:(a or b), and single positive clauses are unchanged.

Checklist before marking Ready for Review

  • Filled out all sections above
  • QA: light/dark, mobile, browsers, keyboard, screen reader (logic-only, no UI change)
  • QA: CodeSandbox / Kibana
  • QA: docs changes
  • Tests: Added a Jest case for the negated multi-value clause
  • Changelog: Added a changelog entry
  • Breaking changes (none)

@gunjanjaswal gunjanjaswal requested a review from a team as a code owner July 4, 2026 23:23
@cla-checker-service

cla-checker-service Bot commented Jul 4, 2026

Copy link
Copy Markdown

💚 CLA has been signed

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown

👋 Since this is a community submitted pull request, a Buildkite build has not been started automatically. Would an Elastic organization member please verify the contents of this pull request and kick off a build manually?

@github-actions github-actions Bot added the community contribution (Don't delete - used for automation) label Jul 4, 2026
@weronikaolejniczak weronikaolejniczak added the skip-vrt Skips Visual Regression Testing job in CI (clearly state why you're using it in your PR) label Jul 6, 2026
@weronikaolejniczak

Copy link
Copy Markdown
Contributor

buildkite test this

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a logic bug in EuiSearchBar’s in-memory filtering (executeAst) where negated multi-value field clauses (e.g. -name:(john or doe)) incorrectly allowed items that should be excluded. The change aligns the implementation with De Morgan’s law by negating the aggregated positive match instead of negating per-value matches.

Changes:

  • Updated fieldClauseMatcher to compute a single “positive hit” for array values and then negate it for must_not clauses.
  • Added a Jest test covering negated multi-value field clauses to prevent regressions.
  • Added a changelog entry documenting the bug fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/eui/src/components/search_bar/query/execute_ast.ts Fixes must_not + multi-value matching by negating the aggregate result instead of per-element negation.
packages/eui/src/components/search_bar/query/execute_ast.test.ts Adds regression test for -field:(a or b) behavior.
packages/eui/changelogs/upcoming/9783.md Notes the bug fix in the upcoming changelog.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@weronikaolejniczak

Copy link
Copy Markdown
Contributor

@gunjanjaswal before we proceed with the review of your PR, could you sign CLA?

@gunjanjaswal

Copy link
Copy Markdown
Author

@gunjanjaswal before we proceed with the review of your PR, could you sign CLA?

@weronikaolejniczak

I have signed the same.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

History

@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

History

@weronikaolejniczak weronikaolejniczak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The fix looks great! Thank you for contributing @gunjanjaswal 🙏🏻 I tested in EUI, there's no regression.

Please do not rebase your branch onto main unless there are merge conflicts needed to be solved.

We will merge and release this fix whenever possible. We try to scope our releases down so it may take a while. I'd also like to run tests against Kibana to make sure we're not breaking any expected behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community contribution (Don't delete - used for automation) skip-vrt Skips Visual Regression Testing job in CI (clearly state why you're using it in your PR)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants