Skip to content

Commit 8faa2a7

Browse files
ESQL: Refactor Join inside the planner (#115813) (#116045)
* ESQL: Refactor Join inside the planner (#115813) First PR that introduces a Join as a first class citizen in the planner. Previously the Join was modeled as a unary node, embedding the right side as a local relationship inside the node but not exposed as a child. This caused a lot the associated methods (like references, output and inputSet) to misbehave and the physical plan rules to pick incorrect information, such as trying to extract the local relationship fields from the underlying source - the fix was to the local relationship fields as ReferenceAttribute (which of course had its own set of issues). Essentially Join was acting both as a source and as a streaming operator. This PR looks to partially address this by: - refactoring Join into a proper binary node with left and right branches which are used for its references and input/outputSet. - refactoring InlineStats to prefer composition and move the Aggregate on the join right branch. This reuses the Aggregate resolution out of the box; in the process remove the Stats interface. - update some of the planner rules that only worked with Unary nodes. - refactor Mapper into (coordinator) Mapper and LocalMapper. - remove Phased interface by moving its functionality inside the planner (no need to unpack the phased classes, the join already indicates the two branches needed). - massage the Phased execution inside EsqlSession - improve FieldExtractor to handle binary nodes - fix incorrect references in Lookup - generalize ProjectAwayColumns rule Relates #112266 Not all inline and lookup tests are passing: - 2 lookup fields are failing due to name clashes (qualifiers should fix this) - 7 or so inline failures with a similar issue I've disabled the tests for now to have them around once we complete adding the functionality. (cherry picked from commit 4ee98e8) * ES|QL: Mute test for #116003 (#116005) (cherry picked from commit 681f509) --------- Co-authored-by: Luigi Dell'Aquila <[email protected]>
1 parent 47b05bb commit 8faa2a7

File tree

65 files changed

+1771
-1404
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+1771
-1404
lines changed

x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClusterSpecIT.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ protected void shouldSkipTest(String testName) throws IOException {
112112
);
113113
assumeFalse("INLINESTATS not yet supported in CCS", testCase.requiredCapabilities.contains("inlinestats"));
114114
assumeFalse("INLINESTATS not yet supported in CCS", testCase.requiredCapabilities.contains("inlinestats_v2"));
115+
assumeFalse("INLINESTATS not yet supported in CCS", testCase.requiredCapabilities.contains("join_planning_v1"));
115116
}
116117

117118
private TestFeatureService remoteFeaturesService() throws IOException {

x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ public void testProfileOrdinalsGroupingOperator() throws IOException {
360360
assertThat(signatures, hasItem(hasItem("OrdinalsGroupingOperator[aggregators=[\"sum of longs\", \"count\"]]")));
361361
}
362362

363+
@AwaitsFix(bugUrl = "disabled until JOIN infrastructrure properly lands")
363364
public void testInlineStatsProfile() throws IOException {
364365
assumeTrue("INLINESTATS only available on snapshots", Build.current().isSnapshot());
365366
indexTimestampData(1);

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,7 @@ public void testComplexFieldNames() throws IOException {
849849
* query. It's part of the "configuration" of the query.
850850
* </p>
851851
*/
852+
@AwaitsFix(bugUrl = "Disabled temporarily until JOIN implementation is completed")
852853
public void testInlineStatsNow() throws IOException {
853854
assumeTrue("INLINESTATS only available on snapshots", Build.current().isSnapshot());
854855
indexTimestampData(1);

x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.apache.lucene.document.InetAddressPoint;
1111
import org.apache.lucene.sandbox.document.HalfFloatPoint;
1212
import org.apache.lucene.util.BytesRef;
13+
import org.elasticsearch.common.Strings;
1314
import org.elasticsearch.common.breaker.CircuitBreaker;
1415
import org.elasticsearch.common.breaker.NoopCircuitBreaker;
1516
import org.elasticsearch.common.bytes.BytesReference;
@@ -600,7 +601,10 @@ else if (Files.isDirectory(path)) {
600601
Files.walkFileTree(path, EnumSet.allOf(FileVisitOption.class), 1, new SimpleFileVisitor<>() {
601602
@Override
602603
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
603-
if (Regex.simpleMatch(filePattern, file.toString())) {
604+
// remove the path folder from the URL
605+
String name = Strings.replace(file.toUri().toString(), path.toUri().toString(), StringUtils.EMPTY);
606+
Tuple<String, String> entrySplit = pathAndName(name);
607+
if (root.equals(entrySplit.v1()) && Regex.simpleMatch(filePattern, entrySplit.v2())) {
604608
matches.add(file.toUri().toURL());
605609
}
606610
return FileVisitResult.CONTINUE;

0 commit comments

Comments
 (0)