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

Fix profiling and open it back up to all queries #706

Merged
merged 4 commits into from
Apr 19, 2023

Conversation

joshtemple
Copy link
Collaborator

Change description

Fixes a bug where profiling did not work at all and expands profiling to be allowed for all queries again, not just fail fast queries.

It's important to note that the profiler results are a bit harder to interpret now that we use binary search. Previously, we had N + 1 queries for each Explore (one that selected all dimensions and one for each dimension). So a query either had exactly N dimensions or 1 dimension.

Now, a query can have any number of dimensions between 1 and N. I've basically handwaved this by returning "*" for the Dimension(s) column in the profiler output whenever a query has more than 1 dimension. It's worth documenting that this means "more than one" and not necessarily "all". I thought this was better than trying to print an arbitrarily long list of dimensions, but open to feedback.

Type of change

  • Bug fix (fixes an issue)
  • New feature (adds functionality)

Related issues

Closes #653.

Checklists

Security

  • Security impact of change has been considered
  • Code follows security best practices and guidelines

Code review

  • Pull request has a descriptive title and context useful to a reviewer

@joshtemple joshtemple merged commit 5bdea9d into master Apr 19, 2023
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.

SQL Validation failing when Profile is enabled
2 participants