Should conditionless queries be removed?

[russcam]

From the conversation in #4020, this issue serves as a discussion on whether to consider removing conditionless queries from the client in 8.0.

As background, conditionless queries have been a part of the high level client, NEST, since 1.x, and are intended to make writing complex queries easier, avoiding the need for the user to check inputs and conditionally add components of a query.

Consider the following terms query

var searchResponse = client.Search<Question>(s => s
	.Query(q => q
		.Terms(t => t
			.Field(f => f.Tags)
			.Terms("")
		)
	)
);

Conditionless behaviour would result in the following query being sent

POST http://localhost:9200/posts/_search
{}

because the terms query input is an array with a single empty string. A null terms input, an input with no items, or an input where all items are null or an empty string would all result in the same behaviour, that of the terms query being excluded from the serialized query JSON.

Conditionless behaviour can be overridden by marking a query as verbatim

var searchResponse = client.Search<Question>(s => s
	.Query(q => q
		.Terms(t => t
			.Field(f => f.Tags)
			.Terms("")
                        .Verbatim()
		)
	)
);

which would result in

POST http://localhost:9200/posts/_search
{"query":{"terms":{"tags":[""]}}}

Pros for conditionless queries

1. Make complex queries easier to write by not having to check inputs and conditionally construct a query.

Cons against conditionless queries

1. Not intuitive for new users

Thoughts @Mpdreamz, @codebrain, @philkra ?

[StingyJack]

Can you give an example of the "pro" listed and what it avoids?

[Mpdreamz]

I'm in favour of this but we should implement it so that folks relying on this behaviour can no longer create these conditionless queries.

This means creating constructors that take the required parameters and moving to descriptors that take these values as first arguments. This is a bit of a break with established patterns everywhere else

[bohdan-kolomiiets]

Explicit over implicit.

Only the developer who constructs queries knows is this a desired/intended behaviour when control flow resulted in passing empty/null values to query or this is a bug. Responsibility of the library is to safe user from hidden mistakes, do not allow to shoot yourself in the foot.

Perhaps it makes sense to move this settings to the global level to preserve the habits of folks and to allow developers to choose.
But I think default setting should be as much safe and explicit as possible.

[stevejgordon]

For the initial 8.0 alpha 1 release we have removed condition-less queries. A work item will review adding back support via a configuration option vs. ensuring we generate appropriate constructors.