Skip to content

Annotated text field type - add test for significant_text agg support#64491

Merged
markharwood merged 1 commit intoelastic:masterfrom
markharwood:fix/64202
Nov 9, 2020
Merged

Annotated text field type - add test for significant_text agg support#64491
markharwood merged 1 commit intoelastic:masterfrom
markharwood:fix/64202

Conversation

@markharwood
Copy link
Copy Markdown
Contributor

@markharwood markharwood commented Nov 2, 2020 •
edited
Loading

Adds missing test for annotated text field supporting significant_text agg.

Closes #64202

@markharwood markharwood added the WIP label Nov 2, 2020
@markharwood markharwood self-assigned this Nov 2, 2020
@markharwood
Copy link
Copy Markdown
Contributor Author

markharwood commented Nov 2, 2020 •
edited
Loading

@romseygeek I added a test but it looks to have revealed a bug,
The significant tokens returned by default in this agg test are iphone and Apple Inc but if I add "exclude": ["iphone"] to the agg to remove the query terms from the results it does the opposite - Apple Inc is excluded from the results and only iphone is returned instead. This used to work OK in 6.6.
I suspect that will need another issue - there's not much point merging this PR until that problem is fixed.

@markharwood
Copy link
Copy Markdown
Contributor Author

markharwood commented Nov 3, 2020

I opened #64519 for the include/exclude bug

@markharwood markharwood force-pushed the fix/64202 branch 3 times, most recently from 9c931a8 to 4f5dbc2 Compare November 5, 2020 09:40
@markharwood markharwood added >enhancement >test Issues or PRs that are addressing/adding tests v8.0.0 and removed WIP labels Nov 9, 2020
romseygeek
romseygeek approved these changes Nov 9, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM!

@markharwood markharwood merged commit ef810ba into elastic:master Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@romseygeek romseygeek romseygeek approved these changes

Assignees

@markharwood markharwood

Labels

>enhancement >test Issues or PRs that are addressing/adding tests v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add tests for aggregations over annotated text

3 participants

@markharwood @romseygeek @jakelandis