ES|QL: Add support for LOOKUP JOIN on aliases #128519
Conversation
|
Hi @luigidellaquila, I've created a changelog YAML for you. |
| if (out.getTransportVersion().onOrAfter(TransportVersions.JOIN_ON_ALIASES)) { | ||
| out.writeString(indexPattern); | ||
| } else if (indexPattern.equals(shardId.getIndexName()) == false) { | ||
| throw new EsqlIllegalArgumentException("Aliases and index patterns are not allowed for LOOKUP JOIN []", indexPattern); |
There was a problem hiding this comment.
Should we throw an exception here?
The alternative is to just allow to execute the JOIN on the remote node based on the index name (rather than the alias name), that means different security constraints and no alias filters.
There was a problem hiding this comment.
Throwing is fine. It wasn't supported before that version.
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
elasticsearchmachine
added
the
Team:Analytics
| @@ -521,7 +553,7 @@ abstract static class TransportRequest extends AbstractTransportRequest implemen | |||
|
|
|||
| @Override | |||
| public final String[] indices() { | |||
| return new String[] { shardId.getIndexName() }; | |||
| return new String[] { indexPattern }; | |||
There was a problem hiding this comment.
This is the real fix to the Security aspects
| if (out.getTransportVersion().onOrAfter(TransportVersions.JOIN_ON_ALIASES)) { | ||
| out.writeString(indexPattern); | ||
| } else if (indexPattern.equals(shardId.getIndexName()) == false) { | ||
| throw new EsqlIllegalArgumentException("Aliases and index patterns are not allowed for LOOKUP JOIN []", indexPattern); |
There was a problem hiding this comment.
Throwing is fine. It wasn't supported before that version.
luigidellaquila
added
auto-backport
💔 Backport failed
You can use sqren/backport to manually backport by running |
There was a problem hiding this comment.
Hey @luigidellaquila , I think this accidentally disabled some bwc tests. Could you take a look please?
| @@ -6,7 +6,7 @@ setup: | |||
| - method: POST | |||
| path: /_query | |||
| parameters: [] | |||
| capabilities: [join_lookup_v12, join_lookup_skip_mv_warnings] | |||
| capabilities: [join_lookup_v12, enable_lookup_join_on_aliases] | |||
There was a problem hiding this comment.
@luigidellaquila , this skips the entire yaml test file if the capability is not present. Is this intended? This means no bwc tests between 9.1 and 9.0 in here.
There was a problem hiding this comment.
Good point, let me split it
Adding support for LOOKUP JOIN on aliases: