ESQL: Prevent search functions work with a non-STANDARD index by bpintea · Pull Request #130638 · elastic/elasticsearch

bpintea · 2025-07-04T13:48:41Z

This introduces verifications to prevent search functions work on fields introduced by a LOOKUP JOIN righthand-side index.

This should be a temporary fix until we can either push these filters down also on the righthand-side of a JOIN or have these functions execute within the engine.

Closes #130561
Closes #129778

This introduces verifications to prevent search functions work on fields introduced by a LOOKUP JOIN righthand-side index.

elasticsearchmachine · 2025-07-04T13:50:00Z

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

elasticsearchmachine · 2025-07-04T13:51:24Z

Hi @bpintea, I've updated the changelog YAML for you.

elasticsearchmachine · 2025-07-04T15:03:05Z

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

carlosdelest · 2025-07-04T15:07:49Z

have these functions execute within the engine.

@bpintea search functions can be executed within the compute engine. Is there something specific we should do on the LOOKUP JOIN side for the engine to use them?

carlosdelest

LGTM from the prevention side - Would like to know more about how we can enable these functions in LOOKUP JOIN 🙂

bpintea · 2025-07-04T16:10:26Z

Is there something specific we should do on the LOOKUP JOIN side for the engine to use them?

@carlosdelest, it looks like it'll require dedicated work: from the quick look I had, the ShardContext that the evaluators use refer the lefthand-side of the JOIN index only. So the search functions queries end up as MatchNoDocsQuery. And that's why they do execute "fine", but never match.

PS. Since (some of?) these can be evaluated internally, should we enable the EVAL-uation on them? Or having them run on a computed field (ex. | EVAL foo = CONCAT(...) | WHERE MATCH(foo, ...))?

carlosdelest · 2025-07-04T16:27:33Z

the ShardContext that the evaluators use refer the lefthand-side of the JOIN index only. So the search functions queries end up as MatchNoDocsQuery. And that's why they do execute "fine", but never match.

Got it, thanks! 👍

PS. Since (some of?) these can be evaluated internally, should we enable the EVAL-uation on them?

We could. There are a number of restrictions that we're looking to lift.

Or having them run on a computed field (ex. | EVAL foo = CONCAT(...) | WHERE MATCH(foo, ...))?

That's trickier - full text functions still depend on Lucene, even when being evaluated on the compute engine, so they depend on the field being indexed.

We could implement them on a computed field in case we created a Lucene in-memory index - something doable, but it hasn't been prioritized yet.

bpintea · 2025-07-04T16:32:08Z

That's trickier - full text functions still depend on Lucene, even when being evaluated on the compute engine, so they depend on the field being indexed.

Right, right. I think EVAL'ing might be handier, this the computed-value idea can wait (also for broader consensus for such a need).

alex-spies

Thanks a lot @bpintea ! Only minor comments from me, please consider them at your own discretion.

alex-spies · 2025-07-04T16:33:52Z

...ugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java

+            plan.forEachDown(p -> {
+                if (p instanceof EsRelation esRelation && esRelation.indexMode() != IndexMode.STANDARD) {
+                    // Check if this EsRelation supplies the field
+                    if (esRelation.output().stream().anyMatch(attr -> attr.id().equals(fieldAttribute.id()))) {


Wouldn't it be more efficient to do esRelation.outputSet().contains(fieldAttribute)?

It would, ofc, thx!

alex-spies · 2025-07-04T16:35:18Z

...ugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java

+        };
+    }
+
+    public static void fieldVerifier(LogicalPlan plan, FullTextFunction function, Supplier<Expression> fieldSupplier, Failures failures) {


nit: a more natural placement would maybe be inside FullTextFunction rather than Match, as other non-inheritors of Match also reach for this method?

Yes, was a lost intention, thanks.

alex-spies · 2025-07-04T16:36:58Z

...ugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java

+        };
+    }
+
+    public static void fieldVerifier(LogicalPlan plan, FullTextFunction function, Supplier<Expression> fieldSupplier, Failures failures) {


question: why is fieldSupplier a supplier, rather than using a simple Expression? Seems like we hand it () -> field every time.

It no longer needs to, refactored.

bpintea · 2025-07-04T17:00:21Z

Thanks Carlos, Alex!

…c#130638) This introduces verifications to prevent search functions work on fields introduced by a LOOKUP JOIN righthand-side index. This should be a temporary fix until we can either push these filters down also on the righthand-side of a JOIN or have these functions execute within the engine. Closes elastic#130561 Closes elastic#129778

elasticsearchmachine · 2025-07-04T18:46:30Z

💔 Backport failed

Status	Branch	Result
✅	9.1
❌	8.19	Commit could not be cherrypicked due to conflicts

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

…130638) (#130646) * ESQL: Prevent search functions work with a non-STANDARD index (#130638) This introduces verifications to prevent search functions work on fields introduced by a LOOKUP JOIN righthand-side index. This should be a temporary fix until we can either push these filters down also on the righthand-side of a JOIN or have these functions execute within the engine. Closes #130561 Closes #129778 * adapt test to 9.1

…130638) (#130704) * ESQL: Prevent search functions work with a non-STANDARD index (#130638) This introduces verifications to prevent search functions work on fields introduced by a LOOKUP JOIN righthand-side index. This should be a temporary fix until we can either push these filters down also on the righthand-side of a JOIN or have these functions execute within the engine. Closes #130561 Closes #129778 (cherry picked from commit 5c4d17d) * 8.19 test addaptation

Prevent search functions work with a LOOKUP index

78c3a16

This introduces verifications to prevent search functions work on fields introduced by a LOOKUP JOIN righthand-side index.

bpintea requested review from alex-spies, astefan, carlosdelest and ioanatia July 4, 2025 13:48

bpintea added >bug auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v9.2.0 v9.1.1 v8.19.1 labels Jul 4, 2025

bpintea changed the title ~~ESQL: Prevent search functions work with a LOOKUP index~~ Jul 4, 2025

Update docs/changelog/130638.yaml

d7fd388

Update docs/changelog/130638.yaml

0cadb17

elasticsearchmachine and others added 3 commits July 4, 2025 14:04

[CI] Auto commit changes from spotless

9dfe4fc

style

4f2e84d

Merge branch 'main' into fix/ftf_lj

9c6d560

bpintea marked this pull request as ready for review July 4, 2025 15:02

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 4, 2025

carlosdelest approved these changes Jul 4, 2025

View reviewed changes

test fix

9f9e68f

alex-spies approved these changes Jul 4, 2025

View reviewed changes

review suggestions

73e5e2b

bpintea added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jul 4, 2025

[CI] Auto commit changes from spotless

510030a

elasticsearchmachine merged commit 5c4d17d into elastic:main Jul 4, 2025
32 checks passed

bpintea deleted the fix/ftf_lj branch July 4, 2025 18:45

bpintea mentioned this pull request Jul 4, 2025

[9.1] ESQL: Prevent search functions work with a non-STANDARD index (#130638) #130646

Merged

elasticsearchmachine added the backport pending label Jul 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESQL: Prevent search functions work with a non-STANDARD index#130638

ESQL: Prevent search functions work with a non-STANDARD index#130638
elasticsearchmachine merged 9 commits intoelastic:mainfrom
bpintea:fix/ftf_lj

bpintea commented Jul 4, 2025 •

edited

Loading

elasticsearchmachine commented Jul 4, 2025

elasticsearchmachine commented Jul 4, 2025

elasticsearchmachine commented Jul 4, 2025

carlosdelest commented Jul 4, 2025

carlosdelest left a comment

bpintea commented Jul 4, 2025

carlosdelest commented Jul 4, 2025

bpintea commented Jul 4, 2025

alex-spies left a comment

alex-spies Jul 4, 2025

bpintea Jul 4, 2025

alex-spies Jul 4, 2025

bpintea Jul 4, 2025

alex-spies Jul 4, 2025

bpintea Jul 4, 2025

bpintea commented Jul 4, 2025

Uh oh!

elasticsearchmachine commented Jul 4, 2025

Labels

4 participants

Conversation

bpintea commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

elasticsearchmachine commented Jul 4, 2025

elasticsearchmachine commented Jul 4, 2025

elasticsearchmachine commented Jul 4, 2025

carlosdelest commented Jul 4, 2025

carlosdelest left a comment

Choose a reason for hiding this comment

bpintea commented Jul 4, 2025

carlosdelest commented Jul 4, 2025

bpintea commented Jul 4, 2025

alex-spies left a comment

Choose a reason for hiding this comment

alex-spies Jul 4, 2025

Choose a reason for hiding this comment

bpintea Jul 4, 2025

Choose a reason for hiding this comment

alex-spies Jul 4, 2025

Choose a reason for hiding this comment

bpintea Jul 4, 2025

Choose a reason for hiding this comment

alex-spies Jul 4, 2025

Choose a reason for hiding this comment

bpintea Jul 4, 2025

Choose a reason for hiding this comment

bpintea commented Jul 4, 2025

Uh oh!

elasticsearchmachine commented Jul 4, 2025

💔 Backport failed

Labels

4 participants

bpintea commented Jul 4, 2025 •

edited

Loading