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

2 phase comparison query #6337

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

2 phase comparison query #6337

wants to merge 7 commits into from

Conversation

pjain1
Copy link
Member

@pjain1 pjain1 commented Dec 30, 2024

Split comparison time range query into two phase -

  1. Run a groupby query on base time range
  2. Run another query which joins the inline results from first query with groupby comparison query having added filter from base query
  • This optimization is only possible when sorting by base column. (can extend to sort by comparison column but UI does not seem to use that anywhere)
  • CTE rewrite is disabled by default now as it cause double scan of CTE in Druid, Duckdb and CH. It can be enabled but does not take effect if 2 phase optimization is done. It can be enabled to run queries on very high cardinality (~1M or greater) when sorting by other than base column which may fail in engines like Druid with ResourceLimitException.

Side note

  • May be I can reuse the inline select concept for the null filling PR, also figured that druid support inline datasource with alternative syntax so that can be used and partial support with context keys can be removed

@pjain1
Copy link
Member Author

pjain1 commented Dec 31, 2024

Another approach is to do two separate topn queries for base and comparison time range and then a 3rd query with results from both queries to calculate measures and return the results, have a working code for this approach as well in a branch. However, this seems more cleaner and does only 2 queries.

@pjain1 pjain1 self-assigned this Dec 31, 2024
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.

1 participant