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

Add Mode queries #4

Merged
merged 4 commits into from
Jan 22, 2019
Merged

Add Mode queries #4

merged 4 commits into from
Jan 22, 2019

Conversation

clrcrl
Copy link
Contributor

@clrcrl clrcrl commented Nov 26, 2018

No description provided.

@clrcrl clrcrl requested a review from jthandy November 26, 2018 21:41
Copy link
Member

@jthandy jthandy left a comment

Choose a reason for hiding this comment

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

Small stuff, all formatting! Take care of this stuff and it's totally ready to merge.

@@ -0,0 +1,18 @@
select 1

{% form %}
Copy link
Member

Choose a reason for hiding this comment

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

can you put this form section into audience_overview.sql? we might want to add other queries in the future that could potentially have different parameters.

sum(case when session_number = 1 then 1 else 0 end) as new_sessions,
sum(case when session_number > 1 then 1 else 0 end) as repeat_sessions

from dbt_claire.segment_web_sessions
Copy link
Member

Choose a reason for hiding this comment

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

you can use ref() in the analysis section as well!

@@ -0,0 +1,15 @@
select
Copy link
Member

Choose a reason for hiding this comment

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

this file should be located in analysis/ :) you probably haven't used that folder much! it's an oft-overlooked feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually thought about that, but then figured because there was the form liquid tag, it would break. I'll just wrap the form tag in a raw tag 👍

sum(case when session_number = 1 then 1 else 0 end) as new_sessions,
sum(case when session_number > 1 then 1 else 0 end) as repeat_sessions

from dbt_claire.segment_web_sessions
Copy link
Member

Choose a reason for hiding this comment

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

we still use the same SQL style guide for queries we write in mode. So--indentation, wrapping everything in CTEs, having an "import" CTE at the top (with sessions as...)

@@ -0,0 +1,56 @@
{#-
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@drewbanin had a look at this with me, and he also couldn't get it to work.
dbt-labs/dbt-core#1152

Copy link
Member

Choose a reason for hiding this comment

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

ok--maybe put this in a markdown file then? that way it won't attempt to be compiled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to keep it as the sql comment

@jthandy
Copy link
Member

jthandy commented Jan 4, 2019

@clrcrl hey! we should get this merged. I think these comments are all handled, right?

@clrcrl clrcrl merged commit b84760d into master Jan 22, 2019
@clrcrl clrcrl deleted the add-mode-queries branch February 14, 2019 14:49
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.

2 participants