[FEATURE] ElasticSearch 8 took away support for "Nullable" fields for Term Query

[DR9885]

Is your feature request related to a problem? Please describe.
ElasticSearch 8 took away support for "Nullable" fields for Term Query

Describe the solution you'd like
Allow passing nullable values, if the value is null. Then the query is not generated.

Example: should not filter on "IsClosed"

        public async Task NullableExample(bool? isClosed)
        {
            var client = new ElasticsearchClient();
            var res = await client.SearchAsync<Account>(x => x.Index(AccountConstants.ViewId)
                .Query(q =>
                    q.Bool(b =>
                        b.Filter(
                            f => f.Term(m =>
                                m.Field(ff => ff.IsClosed)
                                    .Value(isClosed)
                            )
                        )
                    )
                ));
        }

Describe alternatives you've considered
Adding an extension method to support this. But should be fixed in framework.

    public static class ElasticQueryExtensions
    {
        public static TermQueryDescriptor<TDocument> Value<TDocument>(this TermQueryDescriptor<TDocument> query, object value)
        {
            if (value != null) query.Value(value);
            return query;
        }
    }

[DR9885]

Work around doesn't work because it sends the field as null.... would be nice if framework ignored nulls

Request failed to execute. Call: Status code 400 from: POST /tec_view_firm/_search. ServerError: Type: x_content_parse_exception Reason: "[1:74] [bool] failed to parse field [filter]" CausedBy: "Type: illegal_argument_exception Reason: "value cannot be null""

[flobernd]

@DR9885 I'm not sure if the proposed behavior is desirable in all cases (maybe somebody explicitly wants to query documents with null fields).

Regarding the workaround: You have to make sure to omit the call to Field as well and not only the call to Value. You could achieve this by adding an extension similar to this:

public TermQueryDescriptor<TDocument> FieldValue<T>(this TermQueryDescriptor<TDocument> query, Field field, T? value)
{
    if (value is null)
    {
        return query;
    }

    return  query.Field(field).Value(value);
}

[DR9885]

ES7/NEST client behaves that way though. where if Null is passed it doesn't query that field.

But I can see how you'd also want to find nulls... But "Exists" already supports that.

Example : of null value, to not query (same as NEST/ES7)

            string nullString = null;
            var res = await client.SearchAsync<Account>(x => x.Index(AccountConstants.ViewId)
                .Query(q =>
                    q.Bool(b =>
                        b.Filter(
                            f => f.Term(m => m.Field(ff => ff.IsClosed).Value(nullString))
                        )
                    )
                ));

Example : of getting only nulls with exists

            var res = await client.SearchAsync<Account>(x => x.Index(AccountConstants.ViewId)
                .Query(q =>
                    q.Bool(b =>
                        b.Filter(
                            f => !f.Exists(e => e.Field(f => f.IsClosed))
                        )
                    )
                ));

[DR9885]

Another example of why null support would be nice... this is overly verbose, and needs to be done for every min/max

   if(query.MinDate != null && query.MaxDate != null)
          f.Range(x => x.DateRange(dr =>
          {
              dr.Field(ff => ff.Date);

              if (query.MinDate != null)
                  dr.Gte(query.MinDate);
              if (query.MaxDate != null)
                  dr.Gte(query.MaxDate);
          }));

[Rayzbam]

@DR9885 Agree with all your posts.
Null support on term queries is a must have to be honest.

[nuzolx]

@flobernd It's the previous behavior see: #4050.
It's a very cool feature.
@stevejgordon There's a roadmap for this feature?

[flobernd]

@nuzolx Steve is no longer maintaining this project. I am the new maintainer and so far this is not on my roadmap. I can see how this feature is nice to have, but judging from the comments in the linked issue, my colleagues as well seem to prefer removing these conditionless queries. I will discuss this with Steve and Martijn in the next weeks and let you know my decision.

[DR9885]

An even better example of code before and after, its way more verbose without this check. Also to up grade to 8.x client, I would need to change a LOT of code in my company. I'm sure I'm not the only one.

Before:

public async Task<SearchResponse<TestModel>> Query(TestQuery query, int size)
        {
            return await _elastic.SearchAsync<TestModel>(s => s
                .Index(_index)
                .Size(size)
                .Query(q =>
                    q.Bool(b =>
                        b.Filter(
                            f =>
                            {
                                    f.Ids(x => x.Values(query.Ids));
                                    f.Terms(x => x.Field("name.keyword").Terms(query.Names));
                                    f.Terms(x => x.Field(ff => ff.Decimal).Terms(query.Decimals));
                                    f.Terms(x => x.Field(ff => ff.Double).Terms(query.Doubles));
                                    f.Terms(x => x.Field("enum.keyword").Terms(query.Enums));
                                    f.Range(x => x.DateRange(dr =>
                                    {
                                        dr.Field(ff => ff.Date).Gte(query.MinDate).Gte(query.MaxDate);
                                    }));
                            }
                        )
                    )),
            CancellationToken.None);
        }

After:

public async Task<SearchResponse<TestModel>> Query(TestQuery query, int size)
        {
            return await _elastic.SearchAsync<TestModel>(s => s
                .Index(_index)
                .Size(size)
                .Query(q =>
                    q.Bool(b =>
                        b.Filter(
                            f =>
                            {
                                if (query.Ids?.Length > 0)
                                    f.Ids(x => x.Values(query.Ids));
                                if (query.Names?.Length > 0)
                                    f.Terms(x => x.Field("name.keyword").Terms(query.Names));

                                if (query.Decimals?.Length > 0)
                                    f.Terms(x => x.Field(ff => ff.Decimal).Terms(query.Decimals));

                                if (query.Decimals?.Length > 0)
                                    f.Terms(x => x.Field(ff => ff.Double).Terms(query.Doubles));

                                if (query.MinDate != null && query.MaxDate != null)
                                    f.Range(x => x.DateRange(dr =>
                                    {
                                        dr.Field(ff => ff.Date);

                                        if (query.MinDate != null)
                                            dr.Gte(query.MinDate);
                                        if (query.MaxDate != null)
                                            dr.Gte(query.MaxDate);
                                    }));

                                if (query.Enums?.Length > 0)
                                    f.Terms(x => x.Field("enum.keyword").Terms(query.Enums));
                            }
                        )
                    )),
            CancellationToken.None);
        }

[flobernd]

I can definitely see how this improves readability and reduces code complexity. However I can as well see how this might be very unexpected and confusing behavior for new users. In general I prefer explicit behavior over implicit behavior when it comes to code libraries.

If this will be re-implemented, it will be an opt-in feature. It would as well require significant changes to the code generation for which I simply don't have the capacity to work on in the next months.