[ES|QL] pushing down eval expression when it requires data access

[pmpailis]

We want to ensure that the SCORE function will be executed on data nodes even when dealing with eval FUNCTION. To that end, in this PR we update the PushDownAndCombineLimits rule so that LIMIT is not pushed further from an EVAL expression if:

• any of the expressions is SCORE
• rule is applied on the coordinator node

By doing so, the eval is pushed to the data node, where the check above does not hold, so that the limit is pushed and reaches the underlying LuceneOperator.

Closes #133462.

[elasticsearchmachine]

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

[elasticsearchmachine]

Pinging @elastic/es-search-relevance (Team:Search Relevance)

[pmpailis]

This also works with the fork command, e.g.

FROM index 
| FORK 
        (EVAL score1 = SCORE(MATCH(TITLE, "LOTR"))) 
        (EVAL score2 = SCORE(MATCH(TITLE, "ring")))

Should I add an explicit test-case for fork as well?

[carlosdelest]

not sure that's necessary, as other functions are not explicitly tested with FORK. @ioanatia do you think this is necessary?

[pmpailis]

Thanks Carlos! Will proceed with merging and we can add additional tests in a follow up if needed!

[carlosdelest]

LGTM. A couple of comments on testing but looks 💯

[carlosdelest]

I wonder if this test is reproducible if we don't have a SORT 🤔 , as the order is undefined and we may get some score back...

[pmpailis]

Yeap, fair point. Tried this with 1000 iterations and got no failures but will change the query term to aardvark to make sure!

[carlosdelest]

I think it would be interesting to add a test case for using the EVAL result in the Top N, something like:

            FROM test
            | EVAL s = SCORE(MATCH(first_name, "foo"))
            | WHERE last_name IS NOT NULL
            | SORT s
            | LIMIT 42
            | SORT last_name
            | LIMIT 2

[carlosdelest]

Nit - you're using a Dequeue for traversing the tree. I think it would be more idiomatic if you do something like:

            Holder<Boolean> hasScore = new Holder<>(false);
            p.forEachExpression(expr -> {
                if (expr instanceof Score) {
                    hasScore.set(true);
                }
            });
            return hasScore.get();

[pmpailis]

Yeap, makes sense! Will update as suggested !