-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(data-warehouse): Added the ability to use dashboard filters in BI queries #24033
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
python looks good
posthog/hogql/query.py
Outdated
if "filters" in placeholders_in_query or any( | ||
placeholder.startswith("filters.") for placeholder in placeholders_in_query | ||
): | ||
select_query = replace_filters(select_query, filters, team) | ||
placeholders_in_query.remove("filters") | ||
|
||
removed_placeholders: list[str] = [] | ||
for placeholder in placeholders_in_query: | ||
if placeholder == "filters": | ||
removed_placeholders.append(placeholder) | ||
elif placeholder.startswith("filters."): | ||
removed_placeholders.append(placeholder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We anyway iterate over placeholders_in_query
at least once in this block, so why not rewrite to only do it once, skipping over the irrelevant placeholders?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could still rewrite this block only have for placeholder in placeholders_in_query
once, but that's minor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, yep, missed this!
It looks like the code of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple bits left that would be nice to clean up, but overall
posthog/hogql/query.py
Outdated
if "filters" in placeholders_in_query or any( | ||
placeholder.startswith("filters.") for placeholder in placeholders_in_query | ||
): | ||
select_query = replace_filters(select_query, filters, team) | ||
placeholders_in_query.remove("filters") | ||
|
||
removed_placeholders: list[str] = [] | ||
for placeholder in placeholders_in_query: | ||
if placeholder == "filters": | ||
removed_placeholders.append(placeholder) | ||
elif placeholder.startswith("filters."): | ||
removed_placeholders.append(placeholder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could still rewrite this block only have for placeholder in placeholders_in_query
once, but that's minor
Co-authored-by: Michael Matloka <[email protected]>
It looks like the code of |
…into delete_test_fixes * 'delete_test_fixes' of https://github.com/PostHog/posthog: (22 commits) chore(deps): Update posthog-js to 1.153.0 (#24113) fix: series with different current and previous breakdowns are throwing errors (#24092) chore(deps): Update posthog-js to 1.152.1 (#24110) fix(bi): Use a better tooltip mode for graphs (#24101) feat(bi): Added support for decimal numbers in BI (#24103) fix: Site app url for early access (#24104) feat(cdp): Added mailjet templates (#24096) fix: dry run output was inaccurate (#24075) feat: Add batch utilization metric for blobby (#24094) chore(deps): Update posthog-js to 1.151.2 (#24106) fix: limit metadata reloading (#24099) chore: remove useless draft files (#24102) feat(data-warehouse): Added the ability to use dashboard filters in BI queries (#24033) chore(deps): Update posthog-js to 1.151.1 (#24098) fix: Pipeline activity logs (#24077) feat(BI): Series settings (#24082) feat(dev): add separate env var for disabling navigation hooks (#24097) fix(web-analytics): Fix empty channel types (#24087) fix: Make error contact point to support modal (#24048) feat(notebooks): add error boundary around components (#24095) ...
…I queries (PostHog#24033) Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Michael Matloka <[email protected]>
Problem
Changes
{filters.dateRange.from}
and{filters.dateRange.to}
in SQL queries when comparing with a datetimedateTo
is not set, then the comparison node gets effectively removed@Twixes @mariusandra some review of my C++ is code is very much needed please!! I've tried to join a
vector<string>
with a delimiter (.
) - I got to this code from googling and chatgpt, but please do correct me if I've done something horrifically wrong in the C++ world 🤓Usage:
TODO:
Does this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Tests to be added before this merges