8.15.6 - TermQuery / TermsQuery inconsistencies and feedback

[niemyjski]

Elastic.Clients.Elasticsearch version: 8.15.6

Elasticsearch version: 8.15.1

.NET runtime version: 8.x

Operating system version: Any

Description of the problem including expected versus actual behavior:

I noticed a lot of inconsistencies / verbosity when converting to the new api's. I feel like the old one was more consistent and easy to use.

Previous

if (ids is { Count: > 0 })
    node.Parent.SetQuery(new TermsQuery { Field = "id", Terms = ids });
else
    node.Parent.SetQuery(new TermQuery { Field = "id", Value = "none" });

New

if (ids is { Count: > 0 })
    node.Parent.SetQuery(new TermsQuery { Field = "id", Term = new TermsQueryField(ids.Select(FieldValue.String).ToArray()) });
else
    node.Parent.SetQuery(new TermQuery("id") { Value = "none" });

Expected behavior It doesn't make any sense why these are so much different, and all the extra casts (no implicit conversion for field values).

Reference: https://github.com/FoundatioFx/Foundatio.Parsers/pull/84

[flobernd]

Hi @niemyjski ,

It doesn't make any sense why these are so much differen

I agree from a developers perspective. These APIs are generated from our specification and in this particular case, Terms is defined as an union of FieldValue[] | TermsLookup. To model this in C#, we have to generate the TermsQueryField wrapper class.

TermsQueryField derives from `Union<IReadOnlyCollection, TermsLookup> and provides an implicit conversion operator from either variant.

FieldValue itself as well provides implicit conversion from string, int, etc.

Unfortunately, we are running into a C# limitation here, because of the collection.

This works (currently does NOT due to a different bug 😕):

FieldValue[] ids = ["a", "b", "c"]; // Collection elements are implicitly converted to FieldValue (one by one)
TermsQueryField term = ids;         // FieldValue[] is implicitly converted to TermsQueryField

but this one does not:

string[] ids = ["a", "b", "c"];
TermsQueryField term = ids; // The compiler does not know how to convert `string[]` to TermsQueryField

The only solution for this problem would be to generate transitive implicit conversion operators:

1. Generator sees that FieldValue can be constructed from string
2. Generator sees that TermsQueryField can be constructed from FieldValue[]
3. Generator creates a specialized implicit conversion operator for string[]

While this is possible in theory, it could quite easily cause the generation of 100 or 1000 conversion operators (depending on how deeply nested the hierarchy gets). Besides that, we would have a problem if e.g. TermsLookup would be implicitly constructible from string[] as well. In this case we end up with an ambiguity.

For these reasons, this is most likely a: won't fix from my side.

The easiest workaround would be to use a collection of FieldValue instead of a collection of string (I assume your ids is something like a string array).

Please let me know what you think.

[flobernd]

Edit:

I just noticed, that the implicit conversion operators on our Union<T1, T2> are not correctly working as well. I'll create a separate issue for that.

[niemyjski]

I'd optimize for the common use cases, add constructor overloads for TermsQueryField that take different collections and automatically creates field values and or I'd do implicit conversions for the type like you said. I don't think we should be limited by code generation (I've been working on a commercial code generation product for over 15 years).

[github-actions[bot]]

This issue is stale because it has been open 5 days with no activity. Remove stale label or comment or this will be closed in 2 days.

[flobernd]

Related to #7716

[flobernd]

Actually closing this as a duplicate of #7716. If we implement that feature on code generator level (specifically to the FieldValue type), this will as well solve the current issue.