Restrict remote ENRICH after FORK #131945
Conversation
smalyshev changed the title 
| Hi @smalyshev, I've created a changelog YAML for you. |
elasticsearchmachine added the Team:Analytics | Pinging @elastic/es-analytical-engine (Team:Analytics) |
| * {@code FORK [WHERE content:"fox" ] [WHERE content:"dog"] } | ||
| */ | ||
| public class Fork extends LogicalPlan implements PostAnalysisPlanVerificationAware, TelemetryAware { | ||
| public class Fork extends LogicalPlan implements PostAnalysisPlanVerificationAware, TelemetryAware, PipelineBreaker { |
There was a problem hiding this comment.
Hmm, pipeline breakers are also being used in node-/cluster-level reduction here; I don't think this applies to FORK, as FORK cannot be applied in multiple steps (yet?); each FORK branch seems to be hooked up to the same exchange sink on the coordinator. @ioanatia , please correct me if I'm wrong.
We may be introducing a subtle bug here, or maybe not; in any case, it indicates that the PipelineBreaker interface needs further refinement, because for validation purposes FORK definitely is a pipeline breaker, even if it can't be used for node-/cluster-level reduction.
Maybe we should just add a method PipelineBreaker#isReducer or similar? It could default to true and return false for Fork.
There was a problem hiding this comment.
I am not sure adding PipelineBreaker has any effect at all.
PipelineBreaker is indeed used here - but I don't see in which case we would ever get to a point where a FragmentExec contains a logical plan that contains a Fork plan. Let me know if that's not the case.
There was a problem hiding this comment.
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
* Restrict remote LOOKUP JOIN after FORK (cherry picked from commit 24aefcc) # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java
…-tracking * upstream/main: (26 commits) Add release notes for v9.1.0 release (elastic#131953) Unmute multi_node generative tests (elastic#132021) Avoid re-enqueueing merge tasks (elastic#132020) Fix file entitlements for shared data dir (elastic#131748) ES|QL brute force l2_norm vector function (elastic#132025) Make ES|QL SAMPLE not a pipeline breaker (elastic#132014) Speed up tail computation in MemorySegmentES91OSQVectorsScorer (elastic#132001) Remove deprecated usages in `TransportPutFollowAction` (elastic#132038) Simulate impact of shard movement using shard-level write load (elastic#131406) Remove RemoteClusterService.getConnections() method (elastic#131948) Fix off by one in ValuesBytesRefAggregator (elastic#132032) Use unicode strings in data generation by default (elastic#132028) Adding index.refresh_interval as a data stream setting (elastic#131482) [ES|QL] Add more Min/MaxOverTime CSV tests (elastic#131070) Restrict remote ENRICH after FORK (elastic#131945) Fix decoding of non-ascii field names in ignored source (elastic#132018) [docs] Use centrally maintained version variables (elastic#131939) Configurable Inference timeout during Query time (elastic#131551) ESQL: Allow pruning columns added by InlineJoin (elastic#131204) ESQL: Fail `profile` on text response formats (elastic#128627) ...
4 participants
It doesn't work very well anyway (#131445) so it's better to be explicit about this at least until we fix it.
Closes #131445