Skip to content

Commit 7f1d2dc

Browse files
authored
Ban Limit + MvExpand before remote Enrich (#135051)
* Ban Limit + MvExpand before remote Enrich
1 parent b065f73 commit 7f1d2dc

File tree

3 files changed

+51
-0
lines changed

3 files changed

+51
-0
lines changed

docs/changelog/135051.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 135051
2+
summary: Ban Limit + `MvExpand` before remote Enrich
3+
area: ES|QL
4+
type: bug
5+
issues: []

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterEnrichIT.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,20 @@ public void testEnrichCoordinatorThenEnrichRemote() {
466466
assertThat(error.getMessage(), containsString("ENRICH with remote policy can't be executed after [ENRICH _COORDINATOR"));
467467
}
468468

469+
public void testEnrichAfterMvExpandLimit() {
470+
String query = String.format(Locale.ROOT, """
471+
FROM *:events,events
472+
| SORT timestamp
473+
| LIMIT 2
474+
| eval ip= TO_STR(host)
475+
| MV_EXPAND host
476+
| WHERE ip != ""
477+
| %s
478+
""", enrichHosts(Enrich.Mode.REMOTE));
479+
var error = expectThrows(VerificationException.class, () -> runQuery(query, randomBoolean()).close());
480+
assertThat(error.getMessage(), containsString("MV_EXPAND after LIMIT is incompatible with remote ENRICH"));
481+
}
482+
469483
private static void assertCCSExecutionInfoDetails(EsqlExecutionInfo executionInfo) {
470484
assertThat(executionInfo.overallTook().millis(), greaterThanOrEqualTo(0L));
471485
assertTrue(executionInfo.isCrossClusterSearch());

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Enrich.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.common.lucene.BytesRefs;
1414
import org.elasticsearch.common.util.Maps;
1515
import org.elasticsearch.xpack.core.enrich.EnrichPolicy;
16+
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisVerificationAware;
1617
import org.elasticsearch.xpack.esql.capabilities.PostOptimizationVerificationAware;
1718
import org.elasticsearch.xpack.esql.capabilities.TelemetryAware;
1819
import org.elasticsearch.xpack.esql.common.Failures;
@@ -48,6 +49,7 @@ public class Enrich extends UnaryPlan
4849
implements
4950
GeneratingPlan<Enrich>,
5051
PostOptimizationVerificationAware,
52+
PostAnalysisVerificationAware,
5153
TelemetryAware,
5254
SortAgnostic,
5355
ExecutesOn {
@@ -284,6 +286,36 @@ private void checkForPlansForbiddenBeforeRemoteEnrich(Failures failures) {
284286
fails.forEach(f -> failures.add(fail(this, "ENRICH with remote policy can't be executed after [" + f.text() + "]" + f.source())));
285287
}
286288

289+
/**
290+
* Remote ENRICH (and any remote operation in fact) is not compatible with MV_EXPAND + LIMIT. Consider:
291+
* `FROM *:events | SORT @timestamp | LIMIT 2 | MV_EXPAND ip | ENRICH _remote:clientip_policy ON ip`
292+
* Semantically, this must take two top events and then expand them. However, this can not be executed remotely,
293+
* because this means that we have to take top 2 events on each node, then expand them, then apply Enrich,
294+
* then bring them to the coordinator - but then we can not select top 2 of them - because that would be pre-expand!
295+
* We do not know which expanded rows are coming from the true top rows and which are coming from "false" top rows
296+
* which should have been thrown out. This is only possible to execute if MV_EXPAND executes on the coordinator
297+
* - which contradicts remote Enrich.
298+
* This could be fixed by the optimizer by moving MV_EXPAND past ENRICH, at least in some cases, but currently we do not do that.
299+
*/
300+
private void checkMvExpandAfterLimit(Failures failures) {
301+
this.forEachDown(MvExpand.class, u -> {
302+
u.forEachDown(p -> {
303+
if (p instanceof Limit || p instanceof TopN) {
304+
failures.add(fail(this, "MV_EXPAND after LIMIT is incompatible with remote ENRICH"));
305+
}
306+
});
307+
});
308+
309+
}
310+
311+
@Override
312+
public void postAnalysisVerification(Failures failures) {
313+
if (this.mode == Mode.REMOTE) {
314+
checkMvExpandAfterLimit(failures);
315+
}
316+
317+
}
318+
287319
@Override
288320
public void postOptimizationVerification(Failures failures) {
289321
if (this.mode == Mode.REMOTE) {

0 commit comments

Comments
 (0)