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

ESQL: Make explicit when METRICS Aggregate requires @timestamp field #112473

Open
alex-spies opened this issue Sep 3, 2024 · 1 comment
Open
Labels
:Analytics/ES|QL AKA ESQL >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@alex-spies
Copy link
Contributor

EsqlSession.fieldNames determines what fields from an index are required to compute a query, and is used to construct a field caps request before further analyzing the query.

For METRICS commands, it always determines that the @timestamp field will be required. Currently, UnresolvedRelation.references contains this logic (and it's being moved into EsqlSession.fieldNames in #111413).

However, the exact relationship to the METRICS command is not immediately clear from the code; one has to know that the only way for an UnresolvedRelation to have indexMode TIME_SERIES, it needs to come from parsing a METRICS command.

Maybe this logic shouldn't be placed inside UnresolvedRelation, but instead inside Aggregate - or maybe MERTRICS should use a special QueryPlan class that inherits from Aggregate, rather than being expressed in Aggregate.aggregateType.

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Sep 3, 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 >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

No branches or pull requests

2 participants