Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ES|QL: Mute test for #116003 #116005

Merged

Conversation

luigidellaquila
Copy link
Contributor

Muting test for #116003.

Apparently there are planning errors for

FROM idx
| EVAL ip = to_ip(host), x = to_string(host), y = to_string(host)
| INLINESTATS max(id)
java.lang.IllegalStateException: can't find input for [max(id){r}#464]

@luigidellaquila luigidellaquila added >test-mute Use for PR that only mute tests :Analytics/ES|QL AKA ESQL auto-merge labels Oct 31, 2024
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.0.0 labels Oct 31, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

ivancea
ivancea approved these changes Oct 31, 2024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a correct name to those tests, so we can mute them in the typical way (And as a general good practice too).
This test is quite small, so not really important anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'm merging it to unlock people, but I'll follow up with some improvements

@luigidellaquila luigidellaquila merged commit 681f509 into elastic:main Oct 31, 2024
6 checks passed
elasticsearchmachine pushed a commit that referenced this pull request Oct 31, 2024
* 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]>
@costin
Copy link
Member

costin commented Oct 31, 2024

Backport to 8.x through #116045

jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test-mute Use for PR that only mute tests v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants