Skip to content

ES|QL: Remove implicit limit for FORK#145429

Merged
ioanatia merged 14 commits intoelastic:mainfrom
ioanatia:fork_rm_implicit_limit
Apr 8, 2026
Merged

ES|QL: Remove implicit limit for FORK#145429
ioanatia merged 14 commits intoelastic:mainfrom
ioanatia:fork_rm_implicit_limit

Conversation

@ioanatia
Copy link
Copy Markdown
Member

@ioanatia ioanatia commented Apr 1, 2026

related

#121652

Currently each FORK branch receives an implicit LIMIT.

Take for example:

FROM test
| FORK (WHERE name:"aaa")
             (WHERE name:"bbb")
| STATS count1 = COUNT(*) WHERE _fork == "fork1", count2 = COUNT(*) WHERE _fork == "fork2"

this is actually equivalent to:

FROM test
| FORK (WHERE name:"aaa" | LIMIT 1000) 
             (WHERE name:"bbb" | LIMIT 1000)
| STATS count1 = COUNT(*) WHERE _fork == "fork1", count2 = COUNT(*) WHERE _fork == "fork2"

This can be a surprising behaviour for users.
It also limits the utility of FORK.

In this PR, we remove adding the implicit LIMIT to each fork branch.
This is one the last changes before we GA FORK.

@ioanatia ioanatia added >non-issue Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch :Search Relevance/ES|QL Search functionality in ES|QL labels Apr 1, 2026
@ioanatia ioanatia marked this pull request as ready for review April 7, 2026 09:47
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

public static final Setting<String> EXTERNAL_DISTRIBUTION = Setting.simpleString("external_distribution", "adaptive");

public static final Setting<Boolean> FORK_IMPLICIT_LIMIT = Setting.boolSetting("fork_implicit_limit", true);
public static final Setting<Boolean> FORK_IMPLICIT_LIMIT = Setting.boolSetting("fork_implicit_limit", false);
Copy link
Copy Markdown
Member Author

@ioanatia ioanatia Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove this in a follow up PR, together with the Analyzer rule that adds the implicit limit based on the pragma value.

@ioanatia ioanatia requested a review from tteofili April 7, 2026 14:16
Copy link
Copy Markdown
Contributor

@tteofili tteofili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one step closer! 🚀

containsString("FUSE can only be used on a limited number of rows. Consider adding a LIMIT before FUSE.")
);

defaultAnalyzer().error(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change - I think it's ok as we're tech preview, but maybe we need to add a release note about it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good thinking, will adapt the docs in a follow up as well

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

@ioanatia ioanatia mentioned this pull request Apr 7, 2026
18 tasks
"function cannot operate on \\[from .*\\], which is not a field from an index mapping",
// https://github.com/elastic/elasticsearch/issues/145570
"function cannot operate on \\[\\], which is not a field from an index mapping",
"function cannot operate on \\[.*\\], which is not a field from an index mapping",
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to this PR, just fixes GenerativeIT for which we kept getting failures on this branch

@ioanatia ioanatia merged commit beaaa81 into elastic:main Apr 8, 2026
35 checks passed
@ioanatia ioanatia deleted the fork_rm_implicit_limit branch April 8, 2026 09:47
mromaios pushed a commit to mromaios/elasticsearch that referenced this pull request Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>feature :Search Relevance/ES|QL Search functionality in ES|QL Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants