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

Review/update/fix query definitions #486

Merged
merged 2 commits into from
Jul 26, 2021
Merged

Review/update/fix query definitions #486

merged 2 commits into from
Jul 26, 2021

Conversation

swallez
Copy link
Member

@swallez swallez commented Jul 7, 2021

This PR is an exhaustive review/update/fix of the 53 query definitions.

Fixing/updating query definitions

Query definitions follow one of 3 different patterns, driven by where the base query fields (boost and _name) appear in the data structure:

  • a regular structure extending QueryBase: all queries that do not target a single field (e.g. boolean query)
  • a SinglekeyDictionary<Field, SomeQuery>: queries where the structure is a single property for the field name, with a nested structure that holds the base fields (e.g. term query)
  • a class implementing AdditionalProperty<Field, SomeFieldQuery>: queries where the top-level structure contains a single field name and also the base query fields. Used mostly in geo queries, e.g. geo polygon query.

The specification of the query definitions has been verified:

  • by looking a the doc and trying the examples,
  • by reading the ES server source code to disambiguate things that were not apparent in the doc (e.g. location of base query fields),
  • and have been verified to pass the flight recorder validation.

There are however 3 additional validation errors coming from Nest tests where Nest is overly flexible, allowing numbers where strings are the only values that semantically make sense. These tests succeed in Nest because ES is overly lenient and (most often) accepts any primitive type where it expects a string, but we should not encode this leniency in the spec where it doesn't make sense from a semantic perspective.

AdditionalProperty behavior

This PR introduces a new behavior, AdditionalProperty, which is to SingleKeyDictionary what AdditionalProperties is to Dictionary: it is used to model field queries, where only one single field can be the target of a query.

This distinction is important for statically typed languages where dictionaries/records with a single key can be code-generated differently from regular dictionaries.

Shortcut properties

This PR introduces a new @shortcut_property jsdoc tag: it indicates that a property of a class can be used as a shortcut. This is used for many field query types that have single required field, such as term query: {"term": {"some_field": {"value": "some_text"}}} can also be written as {"term": {"some_field": "some_text"}}. The class is then defined as follows:

/** @shortcut_property value */
export class TermQuery extends QueryBase {
  value: string | float | boolean
  case_insensitive?: boolean
}

Defining the shortcut in the class definition rather than on fields using it is consistent with the implementation in the ES code base: shortcut notations are implemented in the JSON deserialization of classes and not at the class usage location.

The TypeScript generator has been updated to expand shortcut properties as union types at the usage location.

Misc

This PR also adds a number of @server_default, @obsolete and @since annotations and adds or updates a few common types.

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

LGTM

@delvedor delvedor merged commit c7c5038 into main Jul 26, 2021
@delvedor delvedor deleted the fix-queries branch July 26, 2021 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants