Fix KQL usage in in STATS .. BY

[carlosdelest]

KQL can now be used in STATS .. BY

FROM airports 
| STATS c = COUNT(*) where kql("country: United States") BY scalerank
| SORT scalerank desc

This fixes the LuceneQueryEvaluator not doing a rewrite of the query to evaluate. As KQL function depends on rewriting for being executed, this meant no hits when it was being used directly as a filter in a STATS .. BY clause.

[elasticsearchmachine]

Hi @carlosdelest, I've created a changelog YAML for you.

[carlosdelest]

Queries should be rewritten prior to actual execution in the LuceneQueryEvaluator. Some queries like KNN or KQL depend on rewriting for being executed.

[svilen-mihaylov-elastic]

Question, why not always rewrite the query whenever we create a ShardConfig instance?

[carlosdelest]

Rewriting is only needed in this case, where the query is not pushed down to Lucene. Doing the rewriting on the ShardContexts.toQuery() method is not necessary for cases when the query is pushed down, and it's clearer that way for debugging as we don't have the rewritten query being pushed down.

We could do the rewriting on the FullTextFunction.toEvaluator() method, but I think it's cleaner to have just the query and let the LuceneQueryEvaluator deal with that internal detail instead of forcing the client to do that.

[ioanatia]

This change fixes the issue we have with KQL.
What I don't understand is why the rewrite does not already happen when we do:

elasticsearch/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java

Line 345 in 74d025e

shardConfigs[i++] = new ShardConfig(shardContext.toQuery(queryBuilder()), shardContext.searcher());

which calls toQuery which should already handle the rewrite?

elasticsearch/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java

Lines 557 to 565 in 74d025e

	public ParsedQuery toQuery(QueryBuilder queryBuilder) {
	reset();
	try {
	Query query = Rewriteable.rewrite(queryBuilder, this, true).toQuery(this);
	if (query == null) {
	query = Queries.newMatchNoDocsQuery("No query left after rewrite.");
	}
	return new ParsedQuery(query, copyNamedQueries());
	} catch (QueryShardException | ParsingException e) {

[carlosdelest]

Good question. The problem lies in the rewriting target.

toQuery() rewrites the QueryBuilder. This means that in KQL it generates a CaseInsensitiveTermQuery, which is the right thing to do.

However, CaseInsensitiveTermQueryis not a query we can create aWeight` on. It needs to be rewritten using the rewrite strategy by actually rewriting the query itself.

The LuceneOperator do this implicitly when creating a Lucene slice. But, the LuceneQueryEvaluator does not do that - it creates a Weight directly on the Query itself. In case the query has not been rewritten, it fails as CaseInsensitiveTermQuery needs to be rewritten first.

I will add some comments to explain this in more detail.

[carlosdelest]

Added tests for other functions as well that were missing

[elasticsearchmachine]

Pinging @elastic/es-analytical-engine (Team:Analytics)

[elasticsearchmachine]

Pinging @elastic/kibana-esql (ES|QL-ui)

[carlosdelest]

@elasticmachine run elasticsearch-ci/rest-compatibility