Skip to content

Commit b8f9ba6

Browse files
Add more UTs and bugfixes
1 parent d253a55 commit b8f9ba6

File tree

10 files changed

+355
-49
lines changed

10 files changed

+355
-49
lines changed

.idea/runConfigurations/Debug_Elasticsearch__node_2_.xml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.idea/runConfigurations/Debug_Elasticsearch__node_3_.xml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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
@@ -118,6 +118,7 @@ public MultiClusterSpecIT(
118118
"LookupJoinOnTwoFieldsMultipleTimes",
119119
// Lookup join after LIMIT is not supported in CCS yet
120120
"LookupJoinAfterLimitAndRemoteEnrich",
121+
"LookupJoinExpressionAfterLimitAndRemoteEnrich",
121122
// Lookup join after FORK is not support in CCS yet
122123
"ForkBeforeLookupJoin"
123124
);

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

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,45 @@ warning:Line 3:3: evaluation of [LOOKUP JOIN multi_column_joinable_lookup ON lef
370370
warning:Line 3:3: java.lang.IllegalArgumentException: LOOKUP JOIN encountered multi-value
371371

372372

373+
left_id:integer | left_name:keyword | id_int:integer | name_str:keyword | is_active_left:boolean | is_active_bool:boolean
374+
1 | Alice | null | null | true | null
375+
[1, 19, 21] | Sophia | null | null | true | null
376+
2 | Bob | null | null | false | null
377+
3 | Charlie | 1 | Alice | true | true
378+
3 | Charlie | 1 | Alice | true | true
379+
4 | David | 2 | Bob | false | false
380+
4 | David | 3 | Charlie | false | false
381+
5 | Eve | 1 | Alice | true | true
382+
5 | Eve | 1 | Alice | true | true
383+
5 | Eve | 3 | Charlie | true | true
384+
6 | null | 1 | Alice | true | true
385+
6 | null | 1 | Alice | true | true
386+
6 | null | 3 | Charlie | true | true
387+
6 | null | 5 | Eve | true | true
388+
6 | null | 5 | Eve | true | true
389+
7 | Grace | 2 | Bob | false | false
390+
7 | Grace | 3 | Charlie | false | false
391+
7 | Grace | 4 | David | false | false
392+
8 | Hank | 1 | Alice | true | true
393+
8 | Hank | 1 | Alice | true | true
394+
;
395+
396+
lookupMultiColMixedGtEqSwapLeftRight
397+
required_capability: join_lookup_v12
398+
required_capability: lookup_join_on_boolean_expression
399+
400+
FROM multi_column_joinable
401+
| RENAME id_int AS left_id, name_str AS left_name, is_active_bool AS is_active_left
402+
| LOOKUP JOIN multi_column_joinable_lookup ON id_int < left_id AND is_active_left == is_active_bool
403+
| KEEP left_id, left_name, id_int, name_str, is_active_left, is_active_bool
404+
| SORT left_id, left_name, id_int, name_str, is_active_left, is_active_left
405+
| LIMIT 20
406+
;
407+
408+
warning:Line 3:3: evaluation of [LOOKUP JOIN multi_column_joinable_lookup ON id_int < left_id AND is_active_left == is_active_bool] failed, treating result as null. Only first 20 failures recorded.
409+
warning:Line 3:3: java.lang.IllegalArgumentException: LOOKUP JOIN encountered multi-value
410+
411+
373412
left_id:integer | left_name:keyword | id_int:integer | name_str:keyword | is_active_left:boolean | is_active_bool:boolean
374413
1 | Alice | null | null | true | null
375414
[1, 19, 21] | Sophia | null | null | true | null
@@ -512,3 +551,187 @@ left_id:integer | left_name:keyword | left_is_active:boolean | left_ip:ip | id_
512551
2 | Bob | false | 192.168.1.3 | 14 | Nina | true | 192.168.1.14
513552
2 | Bob | false | 192.168.1.3 | 16 | Paul | true | 192.168.1.16
514553
;
554+
555+
556+
lookupJoinOnSameLeftFieldTwice
557+
required_capability: join_lookup_v12
558+
required_capability: lookup_join_on_boolean_expression
559+
560+
FROM multi_column_joinable
561+
| RENAME id_int AS id_left, name_str AS name_left, is_active_bool AS is_active_left, ip_addr AS ip_addr_left
562+
| LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND name_left == name_str AND id_left < other2
563+
| KEEP id_left, name_left, extra1, other1, other2
564+
| SORT id_left, name_left, extra1, other1, other2
565+
;
566+
567+
warning:Line 3:3: evaluation of [LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND name_left == name_str AND id_left < other2] failed, treating result as null. Only first 20 failures recorded.
568+
warning:Line 3:3: java.lang.IllegalArgumentException: LOOKUP JOIN encountered multi-value
569+
570+
id_left:integer | name_left:keyword | extra1:keyword | other1:keyword | other2:integer
571+
1 | Alice | foo | alpha | 1000
572+
1 | Alice | foo | beta | 2000
573+
[1, 19, 21] | Sophia | zyx | null | null
574+
2 | Bob | bar | gamma | 3000
575+
3 | Charlie | baz | delta | 4000
576+
3 | Charlie | baz | epsilon | 5000
577+
4 | David | qux | zeta | 6000
578+
5 | Eve | quux | eta | 7000
579+
5 | Eve | quux | theta | 8000
580+
6 | null | corge | null | null
581+
7 | Grace | grault | kappa | 10000
582+
8 | Hank | garply | lambda | 11000
583+
9 | Ivy | waldo | null | null
584+
10 | John | fred | null | null
585+
12 | Liam | xyzzy | nu | 13000
586+
13 | Mia | thud | xi | 14000
587+
14 | Nina | foo2 | omicron | 15000
588+
15 | Oscar | bar2 | null | null
589+
[17, 18] | Olivia | xyz | null | null
590+
null | Kate | plugh | null | null
591+
;
592+
593+
lookupJoinOnSameRightFieldTwice
594+
required_capability: join_lookup_v12
595+
required_capability: lookup_join_on_boolean_expression
596+
597+
FROM multi_column_joinable
598+
| RENAME id_int AS id_left, name_str AS name_left, is_active_bool AS is_active_left, ip_addr AS ip_addr_left
599+
| LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND name_left == name_str AND extra2 > id_int
600+
| KEEP id_left, name_left, extra1, other1, other2
601+
| SORT id_left, name_left, extra1, other1, other2
602+
;
603+
604+
warning:Line 3:3: evaluation of [LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND name_left == name_str AND extra2 > id_int] failed, treating result as null. Only first 20 failures recorded.
605+
warning:Line 3:3: java.lang.IllegalArgumentException: LOOKUP JOIN encountered multi-value
606+
607+
608+
id_left:integer | name_left:keyword | extra1:keyword | other1:keyword | other2:integer
609+
1 | Alice | foo | alpha | 1000
610+
1 | Alice | foo | beta | 2000
611+
[1, 19, 21] | Sophia | zyx | null | null
612+
2 | Bob | bar | gamma | 3000
613+
3 | Charlie | baz | delta | 4000
614+
3 | Charlie | baz | epsilon | 5000
615+
4 | David | qux | zeta | 6000
616+
5 | Eve | quux | eta | 7000
617+
5 | Eve | quux | theta | 8000
618+
6 | null | corge | null | null
619+
7 | Grace | grault | kappa | 10000
620+
8 | Hank | garply | lambda | 11000
621+
9 | Ivy | waldo | null | null
622+
10 | John | fred | null | null
623+
12 | Liam | xyzzy | nu | 13000
624+
13 | Mia | thud | xi | 14000
625+
14 | Nina | foo2 | omicron | 15000
626+
15 | Oscar | bar2 | null | null
627+
[17, 18] | Olivia | xyz | null | null
628+
null | Kate | plugh | null | null
629+
;
630+
631+
lookupJoinOnSameRightFieldTwiceEval
632+
required_capability: join_lookup_v12
633+
required_capability: lookup_join_on_boolean_expression
634+
635+
FROM multi_column_joinable
636+
| RENAME id_int AS id_left, name_str AS name_left, is_active_bool AS is_active_left, ip_addr AS ip_addr_left
637+
| EVAL left_id2 = ((extra2 / 200)::integer)*2
638+
| LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND name_left == name_str AND left_id2 != id_int
639+
| KEEP id_left, name_left, extra1, other1, other2, left_id2
640+
| SORT id_left, name_left, extra1, other1, other2, left_id2
641+
;
642+
643+
warning:Line 4:3: evaluation of [LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND name_left == name_str AND left_id2 != id_int] failed, treating result as null. Only first 20 failures recorded.
644+
warning:Line 4:3: java.lang.IllegalArgumentException: LOOKUP JOIN encountered multi-value
645+
646+
id_left:integer | name_left:keyword | extra1:keyword | other1:keyword | other2:integer | left_id2:integer
647+
1 | Alice | foo | alpha | 1000 | 0
648+
1 | Alice | foo | beta | 2000 | 0
649+
[1, 19, 21] | Sophia | zyx | null | null | 210
650+
2 | Bob | bar | null | null | 2
651+
3 | Charlie | baz | delta | 4000 | 2
652+
3 | Charlie | baz | epsilon | 5000 | 2
653+
4 | David | qux | null | null | 4
654+
5 | Eve | quux | eta | 7000 | 4
655+
5 | Eve | quux | theta | 8000 | 4
656+
6 | null | corge | null | null | 6
657+
7 | Grace | grault | kappa | 10000 | 6
658+
8 | Hank | garply | null | null | 8
659+
9 | Ivy | waldo | null | null | 8
660+
10 | John | fred | null | null | 10
661+
12 | Liam | xyzzy | null | null | 12
662+
13 | Mia | thud | xi | 14000 | 12
663+
14 | Nina | foo2 | null | null | 14
664+
15 | Oscar | bar2 | null | null | 14
665+
[17, 18] | Olivia | xyz | null | null | 170
666+
null | Kate | plugh | null | null | 10
667+
;
668+
669+
twoLookupJoinsInSameQuery
670+
required_capability: join_lookup_v12
671+
required_capability: lookup_join_on_boolean_expression
672+
673+
FROM multi_column_joinable
674+
| WHERE id_int == 1
675+
| RENAME id_int AS id_left, name_str AS name_left
676+
| LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND name_left == name_str
677+
| RENAME other1 AS other1_from_first_join, id_int AS id_from_first_join, name_str AS name_from_first_join
678+
| LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND other1_from_first_join != other1
679+
| KEEP id_left, name_left, other1_from_first_join, other1
680+
| SORT id_left, name_left, other1_from_first_join, other1
681+
;
682+
683+
warning:Line 2:9: evaluation of [id_int == 1] failed, treating result as null. Only first 20 failures recorded.
684+
warning:Line 2:9: java.lang.IllegalArgumentException: single-value function encountered multi-value
685+
warning:Line 6:3: evaluation of [LOOKUP JOIN multi_column_joinable_lookup ON id_left == id_int AND other1_from_first_join != other1] failed, treating result as null. Only first 20 failures recorded.
686+
warning:Line 6:3: java.lang.IllegalArgumentException: LOOKUP JOIN encountered multi-value
687+
688+
id_left:integer | name_left:keyword | other1_from_first_join:keyword | other1:keyword
689+
1 | Alice | alpha | beta
690+
1 | Alice | beta | alpha
691+
;
692+
693+
lookupOnExpressionOnTheCoordinator
694+
required_capability: join_lookup_v12
695+
required_capability: lookup_join_on_boolean_expression
696+
697+
FROM employees
698+
| SORT emp_no
699+
| LIMIT 3
700+
| EVAL language_code_left = languages
701+
| LOOKUP JOIN languages_lookup ON language_code_left == language_code
702+
| KEEP emp_no, language_code_left, language_name
703+
;
704+
705+
emp_no:integer | language_code_left:integer | language_name:keyword
706+
10001 | 2 | French
707+
10002 | 5 | null
708+
10003 | 4 | German
709+
;
710+
711+
lookupJoinExpressionWithPushableFilterOnRight
712+
required_capability: join_lookup_v12
713+
required_capability: lookup_join_on_multiple_fields
714+
715+
FROM multi_column_joinable
716+
| RENAME id_int AS id_left, is_active_bool AS is_active_left
717+
| LOOKUP JOIN multi_column_joinable_lookup ON id_int == id_left and is_active_left == is_active_bool
718+
| WHERE other2 > 5000
719+
| KEEP id_int, name_str, extra1, other1, other2
720+
| SORT id_int, name_str, extra1, other1, other2
721+
| LIMIT 20
722+
;
723+
724+
warning:Line 3:3: evaluation of [LOOKUP JOIN multi_column_joinable_lookup ON id_int == id_left and is_active_left == is_active_bool] failed, treating result as null. Only first 20 failures recorded.
725+
warning:Line 3:3: java.lang.IllegalArgumentException: LOOKUP JOIN encountered multi-value
726+
727+
id_int:integer | name_str:keyword | extra1:keyword | other1:keyword | other2:integer
728+
4 | David | qux | zeta | 6000
729+
5 | Eve | quux | eta | 7000
730+
5 | Eve | quux | theta | 8000
731+
6 | null | corge | iota | 9000
732+
7 | Grace | grault | kappa | 10000
733+
8 | Hank | garply | lambda | 11000
734+
12 | Liam | xyzzy | nu | 13000
735+
13 | Mia | thud | xi | 14000
736+
14 | Nina | foo2 | omicron | 15000
737+
;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,11 @@ private LogicalPlan resolveLookup(Lookup l, List<Attribute> childrenOutput) {
720720
return l;
721721
}
722722

723-
private List<Expression> resolveJoinFilters(List<Expression> filters, List<Attribute> leftOutput, List<Attribute> rightOutput) {
723+
private List<Expression> resolveJoinFiltersAndSwapIfNeeded(
724+
List<Expression> filters,
725+
List<Attribute> leftOutput,
726+
List<Attribute> rightOutput
727+
) {
724728
if (filters.isEmpty()) {
725729
return emptyList();
726730
}
@@ -729,11 +733,39 @@ private List<Expression> resolveJoinFilters(List<Expression> filters, List<Attri
729733

730734
List<Expression> resolvedFilters = new ArrayList<>(filters.size());
731735
for (Expression filter : filters) {
732-
resolvedFilters.add(filter.transformUp(UnresolvedAttribute.class, ua -> maybeResolveAttribute(ua, childrenOutput)));
736+
Expression filterResolved = filter.transformUp(UnresolvedAttribute.class, ua -> maybeResolveAttribute(ua, childrenOutput));
737+
resolvedFilters.add(resolveAndOrientJoinCondition(filterResolved, leftOutput, rightOutput));
733738
}
734739
return resolvedFilters;
735740
}
736741

742+
private Expression resolveAndOrientJoinCondition(Expression condition, List<Attribute> leftOutput, List<Attribute> rightOutput) {
743+
if (condition instanceof EsqlBinaryComparison comp
744+
&& comp.left() instanceof Attribute leftAttr
745+
&& comp.right() instanceof Attribute rightAttr) {
746+
747+
boolean leftIsFromLeft = leftOutput.contains(leftAttr);
748+
boolean rightIsFromRight = rightOutput.contains(rightAttr);
749+
750+
if (leftIsFromLeft && rightIsFromRight) {
751+
return comp; // Correct orientation
752+
}
753+
754+
boolean leftIsFromRight = rightOutput.contains(leftAttr);
755+
boolean rightIsFromLeft = leftOutput.contains(rightAttr);
756+
757+
if (leftIsFromRight && rightIsFromLeft) {
758+
return comp.swapLeftAndRight(); // Swapped orientation
759+
}
760+
761+
// Invalid orientation (e.g., both from left or both from right)
762+
throw new EsqlIllegalArgumentException(
763+
"Join condition must be between attributes on the left and right side, but found: " + condition.sourceText()
764+
);
765+
}
766+
return condition; // Not a binary comparison between two attributes, no change needed.
767+
}
768+
737769
private Join resolveLookupJoin(LookupJoin join) {
738770
JoinConfig config = join.config();
739771
// for now, support only (LEFT) USING clauses
@@ -763,16 +795,19 @@ private Join resolveLookupJoin(LookupJoin join) {
763795
List<Expression> resolvedFilters = new ArrayList<>();
764796
List<Attribute> matchKeys;
765797
if (join.config().joinOnConditions() != null) {
766-
resolvedFilters = resolveJoinFilters(
798+
resolvedFilters = resolveJoinFiltersAndSwapIfNeeded(
767799
Predicates.splitAnd(join.config().joinOnConditions()),
768800
join.left().output(),
769801
join.right().output()
770802
);
771-
// build leftKeys and rightKeys using the left side of the resolvedFilters.
803+
// build leftKeys and rightKeys using the correct side of the resolvedFilters.
804+
// resolveJoinFiltersAndSwapIfNeeded already put the left and right on the correct side
772805
for (Expression expression : resolvedFilters) {
773-
if (expression instanceof EsqlBinaryComparison binaryComparison) {
774-
leftKeys.add((Attribute) binaryComparison.left());
775-
rightKeys.add((Attribute) binaryComparison.right());
806+
if (expression instanceof EsqlBinaryComparison binaryComparison
807+
&& binaryComparison.left() instanceof Attribute leftAttribute
808+
&& binaryComparison.right() instanceof Attribute rightAttribute) {
809+
leftKeys.add(leftAttribute);
810+
rightKeys.add(rightAttribute);
776811
} else {
777812
throw new EsqlIllegalArgumentException("Unsupported join filter expression: " + expression);
778813
}

0 commit comments

Comments
 (0)