function_score type in QueryDslQueryContainer interface seems incorrect

[ohueter]

🐛 Bug Report

The function_score type in the QueryDslQueryContainer interface seems to be incorrect.

I implemented the first example from the Function score query docs in TypeScript:

import type { QueryDslQueryContainer } from '@elastic/elasticsearch/api/types'

const query: QueryDslQueryContainer = {
  function_score: {
    query: { match_all: {} },
    boost: 5,
    random_score: {},
    ^^^^^^^^^^^^^^^^^
    boost_mode: 'multiply',
  },
}

which results in the following type error:

(property) random_score: {}
Type '{ query: { match_all: {}; }; boost: number; random_score: {}; boost_mode: "multiply"; }' is not assignable to type 'QueryDslFunctionScoreQuery'.
  Object literal may only specify known properties, and 'random_score' does not exist in type 'QueryDslFunctionScoreQuery'.
types.d.ts(4824, 3): The expected type comes from property 'function_score' which is declared here on type 'QueryDslQueryContainer'

Expected behavior

No type error should occur. I'm not familiar with the code base, the correct fix seems to change the function_score type in the QueryDslQueryContainer interface from

interface QueryDslQueryContainer {
  function_score?: QueryDslFunctionScoreQuery
}

to

interface QueryDslQueryContainer {
  function_score?: QueryDslFunctionScoreQuery | QueryDslFunctionScoreContainer
}

Hoping this is the correct solution, I went ahead and created a PR for this change: elastic/elasticsearch-js#1745

It's tested to be working with a running Elastic instance :-)

Your Environment

• @elastic/elasticsearch version: >=7.16.0

[JoshMock]

@ohueter Just moved this to the elasticsearch-specification repo, since this concern you have with function score queries would apply to all client libraries.

The fix that you made in elastic/elasticsearch-js#1745 would be accomplished via the code generator by modifying the following line:

https://github.com/elastic/elasticsearch-specification/blob/main/specification/_types/query_dsl/abstractions.ts#L112

to be

function_score?: FunctionScoreQuery | FunctionScoreContainer

I'm not very familiar with this particular API, so I'm pinging @swallez to verify that this seems like an accurate change to that type.

If this is correct, you can rebase your PR and I'll merge it, then update the spec here to match.

[swallez]

This one is tricky... function_score?: FunctionScoreQuery | FunctionScoreContainer will not work since the second member would still need properties that exist in FunctionScoreQuery like boost_mode, query, etc.

So a correct fix would be to clone FunctionScoreContainer with the properties of FunctionScoreQuery added as container properties.

After renaming FunctionScoreQuery to MultiFunctionScoreQuery this can be:

function_score?: FunctionScoreQuery

/** @codegen_names single multi */
export type FunctionScore = SingleFunctionScoreContainer | MultiFunctionScoreQuery

/** @variants container */
export class SingleFunctionScoreContainer {
  exp?: DecayFunction
  gauss?: DecayFunction
  linear?: DecayFunction
  field_value_factor?: FieldValueFactorScoreFunction
  random_score?: RandomScoreFunction
  script_score?: ScriptScoreFunction

  /** @variant container_property */
  filter?: QueryContainer
  /** @variant container_property */
  weight?: double

  // WARNING: keep below properties in sync with SingleFunctionScoreContainer
  /** @variant container_property */
  boost_mode?: FunctionBoostMode
  /** @variant container_property */
  max_boost?: double
  /** @variant container_property */
  min_score?: double
  /** @variant container_property */
  query?: QueryContainer
  /** @variant container_property */
  score_mode?: FunctionScoreMode
}

This is quite complex to allow a shortcut with a single function, which can also be expressed using a single element function array with the current spec.

Also note that this would be a breaking change for languages (like Java) that do not have native unions. These languages will likely want to add a model preparation tweak that only keeps the multi-function variant.

Additional note: the ES server accepts the single-function variant as input but will always serialize as the multi-function variant.

[JoshMock]

Got another issue that appears to have the same underlying cause as this one. Need to get back to it soon!

[JoshMock]

@swallez After neglecting this issue that's been assigned to me for way too long, I now have a fix up in #2852. 😅