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

Add documentation for terms_set minimum_should_match parameter #113043

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mspielberg
Copy link

Elasticsearch has supported the minimum_should_match parameter for the terms_set query since 8.10.0 (PR #96082). As mentioned in the original issue ticket (#94095) the additional parameter is not currently documented, so this PR adds documentation.

This PR also removes the incorrect documented constraint that the terms parameter must consist solely of strings. In fact each term can be any valid FieldValue, as in the normal terms query.

For example, when querying against a field mapped as type date the term may be specified with a JSON number as described at (https://www.elastic.co/guide/en/elasticsearch/reference/8.15/date.html).

Copy link
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.0.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Sep 17, 2024
@kingherc kingherc added :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team >docs General docs changes labels Sep 18, 2024
@elasticsearchmachine elasticsearchmachine added Team:Docs Meta label for docs team and removed needs:triage Requires assignment of a team area label labels Sep 18, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@leemthompo
Copy link
Contributor

@elasticmachine test this

@leemthompo leemthompo self-assigned this Sep 18, 2024
@leemthompo
Copy link
Contributor

run docs-build

@leemthompo
Copy link
Contributor

@pquentin approved the corresponding spec changes, does this look good to you? :)

@mark-vieira mark-vieira added auto-backport Automatically create backport pull requests when merged and removed auto-backport-and-merge labels Oct 4, 2024
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me, I just added two nits about the documented types.

Sorry for the reviewing delay.

(Required, array of strings) Array of terms you wish to find in the provided
(Required, array) Array of terms you wish to find in the provided
Copy link
Member

Choose a reason for hiding this comment

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

Why remove "array of strings" here?

Copy link
Author

Choose a reason for hiding this comment

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

See the PR description. The elements are not restricted to string, but can be anything parsable to a field value, including JSON “number” types. Documenting as “array of strings” is incorrect and misleading.

Comment on lines +174 to +175
(Optional) Specification for the number of matching terms required to return
a document.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(Optional) Specification for the number of matching terms required to return
a document.
(Optional, integer) Specification for the number of matching terms required to return
a document.

Copy link
Author

Choose a reason for hiding this comment

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

Restricting to “integer” is incorrect. The value can be anything described on the linked documentation page, some of which are integers, but also many of which are strings with very specific formats:

https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-minimum-should-match.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >docs General docs changes external-contributor Pull request authored by a developer outside the Elasticsearch team :Search/Search Search-related issues that do not fall into other categories Team:Docs Meta label for docs team Team:Search Meta label for search team v8.10.0 v8.11.0 v8.12.0 v8.13.0 v8.14.0 v8.15.0 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants