Skip to content

Commit f99f52e

Browse files
authored
ESQL: Lookup join drop unused columns on lookup index (#120281)
Closes #118778 Unignore/Update some tests when dropping the lookup columns, and update PruneColumns rule to prune the columns even if only 1 remains
1 parent acb46af commit f99f52e

File tree

3 files changed

+86
-20
lines changed

3 files changed

+86
-20
lines changed

x-pack/plugin/esql/qa/testFixtures/src/main/resources/lookup-join.csv-spec

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -206,18 +206,15 @@ language_code:integer | language_name:keyword
206206
4 | German
207207
;
208208

209-
dropAllLookedUpFieldsOnTheDataNode-Ignore
210-
// Depends on
211-
// https://github.com/elastic/elasticsearch/issues/118778
212-
// https://github.com/elastic/elasticsearch/issues/118781
209+
keepFieldNotInLookup
213210
required_capability: join_lookup_v11
214211

215212
FROM employees
216213
| EVAL language_code = emp_no % 10
217214
| LOOKUP JOIN languages_lookup_non_unique_key ON language_code
218215
| WHERE emp_no == 10001
219216
| SORT emp_no
220-
| DROP language*
217+
| keep emp_no
221218
;
222219

223220
emp_no:integer
@@ -227,28 +224,22 @@ emp_no:integer
227224
10001
228225
;
229226

230-
dropAllLookedUpFieldsOnTheCoordinator-Ignore
231-
// Depends on
232-
// https://github.com/elastic/elasticsearch/issues/118778
233-
// https://github.com/elastic/elasticsearch/issues/118781
227+
dropAllFieldsUsedInLookup
234228
required_capability: join_lookup_v11
235229

236230
FROM employees
237-
| SORT emp_no
238-
| LIMIT 2
231+
| WHERE emp_no == 10001
232+
| keep emp_no
239233
| EVAL language_code = emp_no % 10
240234
| LOOKUP JOIN languages_lookup_non_unique_key ON language_code
241-
| DROP language*
235+
| DROP language_*, country*
242236
;
243237

244238
emp_no:integer
245239
10001
246240
10001
247241
10001
248242
10001
249-
10002
250-
10002
251-
10002
252243
;
253244

254245
###############################################

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,10 @@ public LogicalPlan apply(LogicalPlan plan) {
104104
}
105105
} else if (p instanceof EsRelation esRelation && esRelation.indexMode() == IndexMode.LOOKUP) {
106106
// Normally, pruning EsRelation has no effect because InsertFieldExtraction only extracts the required fields, anyway.
107-
// The field extraction for LOOKUP JOIN works differently, however - we extract all fields (other than the join key)
108-
// that the EsRelation has.
107+
// However, InsertFieldExtraction can't be currently used in LOOKUP JOIN right index,
108+
// it works differently as we extract all fields (other than the join key) that the EsRelation has.
109109
var remaining = removeUnused(esRelation.output(), used);
110-
// TODO: LookupFromIndexOperator cannot handle 0 lookup fields, yet. That means 1 field in total (key field + lookup).
111-
// https://github.com/elastic/elasticsearch/issues/118778
112-
if (remaining != null && remaining.size() > 1) {
110+
if (remaining != null) {
113111
p = new EsRelation(esRelation.source(), esRelation.index(), remaining, esRelation.indexMode(), esRelation.frozen());
114112
}
115113
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6223,6 +6223,83 @@ public void testLookupJoinPushDownDisabledForDisjunctionBetweenLeftAndRightField
62236223
var rightRel = as(join.right(), EsRelation.class);
62246224
}
62256225

6226+
/**
6227+
* When dropping lookup fields, the lookup relation shouldn't include them.
6228+
* At least until we can implement InsertFieldExtract there.
6229+
* Expects
6230+
* EsqlProject[[languages{f}#10]]
6231+
* \_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]]
6232+
* |_Project[[_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, gender{f}#9, hire_date{f}#14, job{f}#15, job.raw{f}#16, lang
6233+
* uages{f}#10, last_name{f}#11, long_noidx{f}#17, salary{f}#12, languages{f}#10 AS language_code]]
6234+
* | \_Limit[1000[INTEGER]]
6235+
* | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..]
6236+
* \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18]
6237+
*/
6238+
public void testLookupJoinKeepNoLookupFields() {
6239+
assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V11.isEnabled());
6240+
6241+
String commandDiscardingFields = randomBoolean() ? "| KEEP languages" : """
6242+
| DROP _meta_field, emp_no, first_name, gender, language_code,
6243+
language_name, last_name, salary, hire_date, job, job.raw, long_noidx
6244+
""";
6245+
6246+
String query = """
6247+
FROM test
6248+
| EVAL language_code = languages
6249+
| LOOKUP JOIN languages_lookup ON language_code
6250+
""" + commandDiscardingFields;
6251+
6252+
var plan = optimizedPlan(query);
6253+
6254+
var project = as(plan, Project.class);
6255+
assertThat(project.projections().size(), equalTo(1));
6256+
assertThat(project.projections().get(0).name(), equalTo("languages"));
6257+
6258+
var join = as(project.child(), Join.class);
6259+
var joinRightRelation = as(join.right(), EsRelation.class);
6260+
6261+
assertThat(joinRightRelation.output().size(), equalTo(1));
6262+
assertThat(joinRightRelation.output().get(0).name(), equalTo("language_code"));
6263+
}
6264+
6265+
/**
6266+
* Ensure a JOIN shadowed by another JOIN doesn't request the shadowed fields.
6267+
*
6268+
* Expected
6269+
* Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#20]]
6270+
* |_Join[LEFT,[language_code{r}#4],[language_code{r}#4],[language_code{f}#18]]
6271+
* | |_Eval[[languages{f}#10 AS language_code]]
6272+
* | | \_Limit[1000[INTEGER]]
6273+
* | | \_EsRelation[test][_meta_field{f}#13, emp_no{f}#7, first_name{f}#8, ge..]
6274+
* | \_EsRelation[languages_lookup][LOOKUP][language_code{f}#18]
6275+
* \_EsRelation[languages_lookup][LOOKUP][language_code{f}#20, language_name{f}#21]
6276+
*/
6277+
public void testMultipleLookupShadowing() {
6278+
assumeTrue("Requires LOOKUP JOIN", EsqlCapabilities.Cap.JOIN_LOOKUP_V11.isEnabled());
6279+
6280+
String query = """
6281+
FROM test
6282+
| EVAL language_code = languages
6283+
| LOOKUP JOIN languages_lookup ON language_code
6284+
| LOOKUP JOIN languages_lookup ON language_code
6285+
""";
6286+
6287+
var plan = optimizedPlan(query);
6288+
6289+
var finalJoin = as(plan, Join.class);
6290+
var finalJoinRightRelation = as(finalJoin.right(), EsRelation.class);
6291+
6292+
assertThat(finalJoinRightRelation.output().size(), equalTo(2));
6293+
assertThat(finalJoinRightRelation.output().get(0).name(), equalTo("language_code"));
6294+
assertThat(finalJoinRightRelation.output().get(1).name(), equalTo("language_name"));
6295+
6296+
var initialJoin = as(finalJoin.left(), Join.class);
6297+
var initialJoinRightRelation = as(initialJoin.right(), EsRelation.class);
6298+
6299+
assertThat(initialJoinRightRelation.output().size(), equalTo(1));
6300+
assertThat(initialJoinRightRelation.output().get(0).name(), equalTo("language_code"));
6301+
}
6302+
62266303
//
62276304
//
62286305
//

0 commit comments

Comments
 (0)