Lookup join on multiple join fields not yet supported #118858
Conversation
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Hi @astefan, I've created a changelog YAML for you. |
| @@ -564,6 +564,11 @@ public PlanFactory visitJoinCommand(EsqlBaseParser.JoinCommandContext ctx) { | |||
| } | |||
| } | |||
|
|
|||
| var matchFieldsCount = joinFields.size(); | |||
| if (matchFieldsCount > 1) { | |||
There was a problem hiding this comment.
Should we check (above) that a same field isn't already added? Specifying a join key multiple times doesn't actually modify the output, so we could allow that (like we do for groups now, for instance, or drop or keep).
There was a problem hiding this comment.
Do you mean lookup join test on languages, languages should be considered as lookup join test on languages?
There was a problem hiding this comment.
Yes, that was what I was thinking. Or, when we might support more, ON x, y, x be same as ON x, y.
But can be left as is as well.
There was a problem hiding this comment.
Imo, we are hiding a behavior (eliminating duplicates) without the user being notified of, what is in essence, an incorrect query. I think for now I will leave this as is in this PR and relax things later, if we identify this as a drawback. Thanks.
💚 Backport successful
|
lookup joins shouldn't work on multiple join fields at the moment.This PR also adds tests for another existent limitation: joining fields must be only actual index fields (not constants for example).