Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Fix/redshift comp #36

Merged
merged 4 commits into from
Jul 20, 2020
Merged

Fix/redshift comp #36

merged 4 commits into from
Jul 20, 2020

Conversation

sanjanasen96
Copy link
Contributor

Description:

  • first_value window functions in Redshift require an explicit frame clause.

@sanjanasen96 sanjanasen96 requested a review from clrcrl July 17, 2020 02:14
Copy link
Contributor

@clrcrl clrcrl left a comment

Choose a reason for hiding this comment

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

Small formatting fix. Wondering if it's time to set up integration tests on this package (if we're going to continue to support it)

@@ -14,7 +14,7 @@ with base as (
coalesce(page_link,template_page_link) as url,
coalesce(page_link,template_page_link) as base_url,
'/' || {{ dbt_utils.get_url_path('coalesce(page_link,template_page_link)') }} as url_path,
row_number() over (partition by creative_id, page_link order by _FIVETRAN_SYNCED desc) as row_num
row_number() over (partition by creative_id, page_link order by _fivetran_synced desc) as row_num
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this will break Snowflake. I don't think it will 🤞

Copy link
Contributor Author

@sanjanasen96 sanjanasen96 Jul 20, 2020

Choose a reason for hiding this comment

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

Honestly, I just went ahead and switched it back.

macros/fivetran/base/fivetran_fb_url_tag.sql Outdated Show resolved Hide resolved
@sanjanasen96 sanjanasen96 requested a review from clrcrl July 20, 2020 20:43
@sanjanasen96
Copy link
Contributor Author

sanjanasen96 commented Jul 20, 2020

Small formatting fix. Wondering if it's time to set up integration tests on this package (if we're going to continue to support it)

Yeah, I'll defer to you @clrcrl on whether or not we will continue supporting them. If so, we should probably go all in soon. Setup integration tests, move logic out of macros, etc.

@sanjanasen96 sanjanasen96 merged commit 76b20d4 into bump/0-17-0 Jul 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants