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

Specify the operator option when generating term queries. #6409

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

tobio
Copy link
Member

@tobio tobio commented Nov 18, 2022

This restores previous functionality when querying for multi-term values in analysed fields

Fixes #6399

Summary

Restores the previous match operator behaviour when the query value is split into multiple terms after analysis.

QA

General checklist

  • Added or updated **jest and [cypress]
  • A changelog entry exists and is marked appropriately

This restores previous functionality when querying for multi-term values in analysed fields
@tobio tobio added the bug label Nov 18, 2022
@tobio tobio self-assigned this Nov 18, 2022
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6409/

Copy link
Contributor

@PhaedrusTheGreek PhaedrusTheGreek left a comment

Choose a reason for hiding this comment

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

This LGTM,

Prior to my change, we were simply concatenating terms with terms.join(' ')

   match: {
      [field]: {
        query: terms.join(' '),
        operator: andOr,
      },

When the associated field mapping was a keyword, things broke, so converting to a bool query solved that particular instance. Unfortunately I didn't take into consideration the possibility of each term also having spaces in it, which in retrospect (of course), is clear.

With this change, we have the best of both worlds. Each individual term is broken out into it's respective match query inside a bool, with the desired operator (and/or) giving us must/should, plus now we also add the appropriate operator: and/or inside each individual match, to catch cases where the individual term also contains a space (or whatever tokenization).

Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Fantastic, thank you for the approval and verbal walk through @PhaedrusTheGreek, and of course thank you for the contribution @tobio! I'll go ahead and approve and merge this in as Chandler is out sick this week.

@cee-chen cee-chen merged commit 9ffddab into elastic:main Nov 22, 2022
@tobio tobio deleted the elevator-operator branch November 22, 2022 18:56
1Copenut added a commit to elastic/kibana that referenced this pull request Dec 2, 2022
`eui@70.2.4` ⏩ `eui@70.4.0`

- "Fixed EuiButtonGroup firing onChange twice" required changing some
tests from `click` to `change`
___

## [`70.4.0`](https://github.com/elastic/eui/tree/v70.4.0)

- Updated `EuiTourStep.footerAction` type to accept `ReactNode[]`
([#6384](elastic/eui#6384))
- Vertically aligned all footer content so that `euiTourStepIndicator`
is always centered ([#6384](elastic/eui#6384))
- Added `filterInCircle` glyph to `EuiIcon`
([#6385](elastic/eui#6385))
- Added `color` prop to `EuiBeacon`
([#6420](elastic/eui#6420))
- Added the `euiMaxBreakpoint` and `euiMinBreakpoint` CSS-in-JS
utilities for creating min/max-width media queries
([#6431](elastic/eui#6431))

**Bug fixes**

- Restores the previous match operator behaviour when the query value is
split into multiple terms after analysis.
([#6409](elastic/eui#6409))
- Fixed missing slide-in animation on `EuiCollapsibleNav`s and left-side
`EuiFlyout`s ([#6422](elastic/eui#6422))
- Fix bug in `EuiCard` where footer were not aligned to the bottom of
the card ([#6424](elastic/eui#6424))
- Fixed multiple component media queries for consumers with custom theme
breakpoints ([#6431](elastic/eui#6431))

## [`70.3.0`](https://github.com/elastic/eui/tree/v70.3.0)

- `EuiSearchBar` now automatically wraps special characters not used by
query syntax in quotes
([#6356](elastic/eui#6356))
- Added `alignment` prop to `EuiBetaBadge`
([#6361](elastic/eui#6361))
- `EuiButton` now accepts `minWidth={false}`
([#6373](elastic/eui#6373))

**Bug fixes**

- Fixed `EuiPageTemplate` not correctly passing the `component` prop to
the inner main content wrapper.
([#6352](elastic/eui#6352))
- `EuiSkipLink` now correctly calls `onClick` even when
`fallbackDestination` is invalid
([#6355](elastic/eui#6355))
- Permanently fixed `EuiModal` to not cause scroll-jumping issues on
modal open ([#6360](elastic/eui#6360))
- Re-fixed `EuiPageSection` not correctly merging `contentProps.css`
([#6365](elastic/eui#6365))
- Fixed `EuiTab` not defaulting to size `m`
([#6366](elastic/eui#6366))
- Fixed the shadow sizes of `.eui-yScrollWithShadows` and
`.eui-xScrollWithShadows`
([#6374](elastic/eui#6374))
- Fixed bug in `EuiCard` where the inner content in vertical cards was
not growing 100% in width
([#6377](elastic/eui#6377))
- Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex`
CSS gap change ([#6380](elastic/eui#6380))
- Fixed visual bug in nested `EuiFlexGroup`s, where the parent
`EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not
([#6381](elastic/eui#6381))

**CSS-in-JS conversions**

- Converted `EuiModal` to Emotion
([#6321](elastic/eui#6321))

**Fixes**

- `EuiButton` no longer outputs unnecessary inline styles for
`minWidth={0}` or `minWidth={false}`
([#6373](elastic/eui#6373))
- `EuiFacetButton` no longer reports type issues when passing props
accepted by `EuiButton`
([#6373](elastic/eui#6373))

Co-authored-by: Constance Chen <constance.chen@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SearchBar] Multi-term queries on analysed fields no longer specify and/or operators.
4 participants