-
Notifications
You must be signed in to change notification settings - Fork 506
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
Feature/add listagg macro #530
Merged
Merged
Changes from 36 commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
a7f4f51
Update README.md
joellabes 168396f
Mutually excl range examples in disclosure triangle
joellabes 5717b10
Fix union_relations error when no include/exclude provided
joellabes 3c83bf4
Add to_condition to relationships where
joellabes b000d8b
very minor nit - update "an new" to "a new" (#519)
JamieRosenberg-canva 9e32d9c
add quoting to split_part (#528)
patkearns10 d279542
add macro to get columns (#516)
patkearns10 a04bd8a
Add listagg macro and integration test
graciegoheen 127203c
remove type in listagg macro
graciegoheen 91dcdb7
updated integration test
graciegoheen 08a2345
Add redshift to listagg macro
graciegoheen 5ad068c
remove redshift listagg
graciegoheen 814dd3b
explicitly named group by column
graciegoheen 365cf79
updated default values
graciegoheen 0f19bb8
Updated example to use correct double vs. single quotes
graciegoheen a0f71d1
whitespace control
graciegoheen e140371
Added redshift specific macro
graciegoheen 31cb2be
Remove documentation
graciegoheen 71db890
Update integration test so less likely to accidentally work
graciegoheen c19343c
default everything but measure to none
graciegoheen 9f8e917
Merge branch 'feature/add_listagg_macro' of github.com:dbt-labs/dbt-u…
graciegoheen 55c9a49
added limit functionality for other dbs
graciegoheen 6c6fa8c
syntax bug for postgres
graciegoheen 270c123
update redshift macro
graciegoheen 05812fb
fixed block def control
graciegoheen f79b9a1
Fixed bug in redshift
graciegoheen 3ff594e
Bug fix redshift
graciegoheen 9cad420
remove unused group_by arg
graciegoheen 2a3d30b
Added additional test without order by col
graciegoheen a8928ec
updated to regex replace
graciegoheen f19727f
typo
graciegoheen d7a5a1d
added more integration_tests
graciegoheen c7db217
attempt to make redshift less complicated
graciegoheen ca702e6
typo
graciegoheen 5673f15
update redshift
graciegoheen e1c5050
replace to substr
graciegoheen 8c43858
More explicit versions with added complexity
graciegoheen 8787d84
handle special characters
graciegoheen 9e8b41f
Merge branch 'next/patch' into feature/add_listagg_macro
joellabes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
group_col,string_text,order_col | ||
1,a,1 | ||
1,b,2 | ||
1,c,3 | ||
2,a,2 | ||
2,1,1 | ||
2,p,3 | ||
3,g,1 | ||
3,g,2 | ||
3,g,3 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
group_col,expected,version | ||
1,"a,b,c",1 | ||
2,"1,a,p",1 | ||
3,"g,g,g",1 | ||
1,"a,b",2 | ||
2,"1,a",2 | ||
3,"g,g",2 | ||
3,"g,g,g",3 | ||
3,"g",4 | ||
3,"g,g,g",5 |
4 changes: 4 additions & 0 deletions
4
integration_tests/data/sql/data_filtered_columns_in_relation.csv
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
field_1,field_2,field_3 | ||
a,b,c | ||
d,e,f | ||
g,h,i |
2 changes: 2 additions & 0 deletions
2
integration_tests/data/sql/data_filtered_columns_in_relation_expected.csv
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
field_2,field_3 | ||
h,i |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
{% macro assert_equal_values(actual_object, expected_object) %} | ||
{% if not execute %} | ||
|
||
{# pass #} | ||
|
||
{% elif actual_object != expected_object %} | ||
|
||
{% set msg %} | ||
Expected did not match actual | ||
|
||
----------- | ||
Actual: | ||
----------- | ||
--->{{ actual_object }}<--- | ||
|
||
----------- | ||
Expected: | ||
----------- | ||
--->{{ expected_object }}<--- | ||
|
||
{% endset %} | ||
|
||
{{ log(msg, info=True) }} | ||
|
||
select 'fail' | ||
|
||
{% else %} | ||
|
||
select 'ok' {{ limit_zero() }} | ||
|
||
{% endif %} | ||
{% endmacro %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
with data as ( | ||
|
||
select * from {{ ref('data_listagg') }} | ||
|
||
), | ||
|
||
data_output as ( | ||
|
||
select * from {{ ref('data_listagg_output') }} | ||
|
||
), | ||
|
||
calculate as ( | ||
|
||
select | ||
group_col, | ||
{{ dbt_utils.listagg('string_text', "','", "order by order_col") }} as actual, | ||
1 as version | ||
from data | ||
group by group_col | ||
|
||
union all | ||
|
||
select | ||
group_col, | ||
{{ dbt_utils.listagg('string_text', "','", "order by order_col", 2) }} as actual, | ||
2 as version | ||
from data | ||
group by group_col | ||
|
||
union all | ||
|
||
select | ||
group_col, | ||
{{ dbt_utils.listagg('string_text', "','") }} as actual, | ||
3 as version | ||
from data | ||
where group_col = 3 | ||
group by group_col | ||
|
||
union all | ||
|
||
select | ||
group_col, | ||
{{ dbt_utils.listagg('DISTINCT string_text', "','") }} as actual, | ||
4 as version | ||
from data | ||
where group_col = 3 | ||
group by group_col | ||
|
||
union all | ||
|
||
select | ||
group_col, | ||
{{ dbt_utils.listagg('string_text') }} as actual, | ||
5 as version | ||
from data | ||
where group_col = 3 | ||
group by group_col | ||
|
||
) | ||
|
||
select | ||
calculate.actual, | ||
data_output.expected | ||
from calculate | ||
left join data_output | ||
on calculate.group_col = data_output.group_col | ||
and calculate.version = data_output.version |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I like that all of these are in the one model, but I'm wondering if we can move away from magic numbers, and instead show what the goal of the version is. Something like
comma_ordered
,comma_ordered_limited
,comma_unordered
,distinct_comma
,no_params
...Also, just noticed that all of these use
,
as their delimiter, which is also the default value, so again it'd be good to test it with something different to make sure that it's doing what we ask it to instead of being correct by accident.It'd be particularly worthwhile to use a multi-character/whitespace-using delimiter for a bit of novelty, e.g.
,
or something weird like_|_
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.
Love these ideas! Thanks for all of the feedback :)
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.
Note " , " won't work as a delimiter if you have whitespace in your
measure
column. This is in-line with the note that "if there are instances ofdelimiter_text
within your measure, you cannot include alimit_num
".