Skip to content

Conversation

david-crespo
Copy link
Contributor

@david-crespo david-crespo commented Apr 1, 2025

Closes #7532. Instead of manipulating the query as a string to add the filters at the end, pass the silo and project IDs to the oximeter client, which has the parsed query AST on hand, and let it do the manipulation.

In case of simple queries, we just insert the filters after the initial get operation. For "grouped" queries (like { get a:b; get c:d | filter timestamp > @now() } | filter x > y), the query is a tree, so we recurse down it and insert the filters in every leaf node, which is a simple query.

@david-crespo david-crespo marked this pull request as ready for review April 2, 2025 18:00
@david-crespo david-crespo requested a review from bnaecker April 2, 2025 18:02
@david-crespo david-crespo changed the title OxQL project filter fix Insert project filters into OxQL query AST instead of raw string Apr 2, 2025
Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for doing it! I agree we should deduplicate the actual query-running method, but otherwise looks good. Thanks for the tests too.

@david-crespo david-crespo force-pushed the oxql-project-filter-fix branch from d33d83f to a274aa1 Compare April 2, 2025 20:22
Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@david-crespo david-crespo enabled auto-merge (squash) April 2, 2025 22:10
@david-crespo david-crespo merged commit be39374 into main Apr 3, 2025
17 checks passed
@david-crespo david-crespo deleted the oxql-project-filter-fix branch April 3, 2025 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Project-scoped timeseries interact poorly with grouping

2 participants