dbt-labs / snowplow

Data models for snowplow analytics.
https://hub.getdbt.com/dbt-labs/snowplow/latest/
Apache License 2.0
126 stars 45 forks source link

Add browser columns and change Unioned CTE #63

Closed tayloramurphy closed 5 years ago

tayloramurphy commented 5 years ago

This PR adds 3 columns for browser info to the page_views model. It also renames the unioned CTE to avoid a Snowflake-specific bug where the name of the CTE was creating a Recursive CTE error.

tayloramurphy commented 5 years ago

@drewbanin tagging for review.

drewbanin commented 5 years ago

Thanks for the PR @tayloramurphy! This broadly LGTM. Do you mind merging the latest version of master? I think that will fix these CI failures (it's not your fault).

I'm happy to merge the change to the unioned CTE names. Do you have any thoughts about how we should handle things like this more generally? I think it's ultimately a Snowflake bug, which is unfortunate, but it's been long-standing and painful enough that I'm not opposed to taking some more general action if required.

tayloramurphy commented 5 years ago

@drewbanin I think it's just something with their compiler and the naming. I really don't have any additional ideas on how to handle it except to just avoid the bad names.

I think I merged everything in right. Let me know if I missed something.