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

Bugfix: Swap order of datediff input fields in performance timing page view context #106

Closed vlambertgrove closed 3 years ago

vlambertgrove commented 3 years ago

Description & motivation

While doing some debugging on the snowplow performance timing at Grove, I noticed that the jinja for this code was reversed from what the original data model has:

https://github.com/snowplow/snowplow-web-data-model/blob/master/redshift/sql/01-page-views/05-timing-context.sql#L83-L85

The effect of this is that when the three timestamps are 0 (representing ok values) rather than large negative numbers there are large positive numbers. This causes records to be filtered erroneously.

Swapping the order in the datediffs should be enough to resolve the bug.

Checklist

leahwicz commented 3 years ago

@jtcohen6 I'm approving this b/c it looks right to me but you made the change a year ago here that flipped them so if there was a reason, lets review this when you get back https://github.com/dbt-labs/snowplow/pull/89/files

jtcohen6 commented 3 years ago

@leahwicz You're totally right... I don't think I had meant to do that!

Thanks for the fix @vlambertgrove! This looks good to merge. I'll follow up by cutting a patch release of the package (v0.13.1).