ESQL - Remove restrictions for disjunctions in full text functions

[carlosdelest]

Full Text functions are currently limited - they cannot be used as part of disjunctions, as there is not a reliable way of understanding if an expression is pushable to Lucene on the coordinator node.

Something we can do in order to lift that restriction is to ensure that when a full text function is used as part of a disjunction, then all the elements in the disjunction are full text functions, so we know for sure that they can be pushed to Lucene.

This PR checks the above, and adds testing to it.

[elasticsearchmachine]

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

[elasticsearchmachine]

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

[ioanatia]

We might need to add a new esql capability if the bwc tests fail 🤔

[carlosdelest]

I was too optimistic...

[tteofili]

I love this ! 🤟

[ChrisHegarty]

This is a great improvement. I left a few small comments.

[tteofili]

LGTM, great work Carlos!

[ChrisHegarty]

LGTM.

Just a final thing to consider (which could be just my misunderstanding, I didn't double check!). Are we ok when the filter has a top-level conjunction, followed by a disjunction within the right-size clause? e.g.

| WHERE content:"fox" AND ( match(foo, "xx") OR to_upper(content) == "FOX")`

[carlosdelest]

re we ok when the filter has a top-level conjunction, followed by a disjunction within the right-size clause? e.g.
| WHERE content:"fox" AND ( match(foo, "xx") OR to_upper(content) == "FOX")`

@ChrisHegarty , that's an error and is checked by this test.

[fang-xing-esql]

Is my understanding right, that the goal of checkFullTextSearchDisjunctions is - if there is FullTextFunction exists under Or, all of the children of the Or have to be FullTextFunctions? If this is true, can this algorithm be simplified, instead of checking onlyFullTextFunctionsInExpression, can we check nonFullTextFunctionExists on either sides of the Or?

[carlosdelest]

Sorry, I'm not following - can you please elaborate with an example?

[fang-xing-esql]

I'm thinking if we can simplify the logic here. Will something like this work? Do we need to check both sides separately?

            boolean hasFullText = or.anyMatch(FullTextFunction.class::isInstance);
            boolean hasOnlyFullText = onlyFullTextFunctionsInExpression(or);
            if (hasFullText) {
                if (hasOnlyFullText) {
                    // succeed
                } else {
                    // fail
                }
            } else {
                // succeed
            }

[carlosdelest]

Thanks for explaining, I see your point. I was trying to get the sub-expression that is at fault, so we can add that to the failure message. But maybe that's not super helpful and we can just error out with:

Invalid condition [first_name:"Anna" or starts_with(first_name, "Anne")]. Full text functions can be used in an OR condition, but only if just full text functions are used in the OR condition

instead of the previous:

Invalid condition [first_name:"Anna" or starts_with(first_name, "Anne")]. [:] operator can be used in an OR condition, but only if just full text functions are used in the OR condition

I have simplified this in 8901591, LMKWYT and I can keep or revert it to the previous one.

[fang-xing-esql]

Thank you @carlosdelest, LGTM

[carlosdelest]

@elasticmachine run elasticsearch-ci/part-4

[elasticsearchmachine]

💔 Backport failed

Status	Branch	Result
❌	8.x	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 118544

[carlosdelest]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Questions ?

Please refer to the Backport tool documentation