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 quote_identifiers parameter to unpivot to handle case-sensitive column names #792

Merged
merged 10 commits into from
Jul 11, 2024
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,7 @@ Boolean values are replaced with the strings 'true'|'false'
- `remove`: A list of columns to remove from the resulting table.
- `field_name`: column name in the resulting table for field
- `value_name`: column name in the resulting table for value
- `quote_identifiers` (optional, default=`False`): will encase selected columns and aliases in quotes according to your adapter's implementation of `adapter.quote` (e.g. `"field_name" as "field_name"`).

### width_bucket ([source](macros/sql/width_bucket.sql))

Expand Down
4 changes: 4 additions & 0 deletions integration_tests/data/sql/data_unpivot_quote.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Customer_Id,Created_At,sTaTuS,SEGMENT,Name
123,2017-01-01,active,tier 1,name 1
234,2017-02-01,active,tier 3,name 3
567,2017-03-01,churned,tier 2,name 2
7 changes: 7 additions & 0 deletions integration_tests/data/sql/data_unpivot_quote_expected.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Customer_Id,Created_At,Prop,Val
123,"2017-01-01","SEGMENT","tier 1"
123,"2017-01-01","sTaTuS","active"
234,"2017-02-01","SEGMENT","tier 3"
234,"2017-02-01","sTaTuS","active"
567,"2017-03-01","sTaTuS","churned"
567,"2017-03-01","SEGMENT","tier 2"
6 changes: 6 additions & 0 deletions integration_tests/dbt_project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ seeds:
num_buckets: integer
min_value: float
max_value: float

data_unpivot_quote:
+quote_columns: true

data_unpivot_quote_expected:
+quote_columns: true

schema_tests:
data_test_sequential_timestamps:
Expand Down
5 changes: 5 additions & 0 deletions integration_tests/models/sql/schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ models:
- dbt_utils.equality:
compare_model: ref('data_unpivot_bool_expected')

- name: test_unpivot_quote
data_tests:
- dbt_utils.equality:
compare_model: ref('data_unpivot_quote_expected')

- name: test_star
data_tests:
- dbt_utils.equality:
Expand Down
10 changes: 10 additions & 0 deletions integration_tests/models/sql/test_unpivot_quote.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

{{ dbt_utils.unpivot(
relation=ref('data_unpivot_quote'),
cast_to=type_string(),
exclude=['Customer_Id', 'Created_At'],
remove=['Name'],
field_name='Prop',
value_name='Val',
quote_identifiers=True,
) }}
17 changes: 9 additions & 8 deletions macros/sql/unpivot.sql
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ Arguments:
value_name: Destination table column name for the pivoted values
#}

{% macro unpivot(relation=none, cast_to='varchar', exclude=none, remove=none, field_name='field_name', value_name='value') -%}
{{ return(adapter.dispatch('unpivot', 'dbt_utils')(relation, cast_to, exclude, remove, field_name, value_name)) }}
{% macro unpivot(relation=none, cast_to='varchar', exclude=none, remove=none, field_name='field_name', value_name='value', quote_identifiers=False) -%}
Copy link
Contributor

Choose a reason for hiding this comment

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

In an ideal world, I would rather have the default be quote_identifiers=True so that it matches with pivot. But agree with your choice here to default it to False instead so we don't have any breaking behavior change.

{{ return(adapter.dispatch('unpivot', 'dbt_utils')(relation, cast_to, exclude, remove, field_name, value_name, quote_identifiers)) }}
{% endmacro %}

{% macro default__unpivot(relation=none, cast_to='varchar', exclude=none, remove=none, field_name='field_name', value_name='value') -%}
{% macro default__unpivot(relation=none, cast_to='varchar', exclude=none, remove=none, field_name='field_name', value_name='value', quote_identifiers=False) -%}

{% if not relation %}
{{ exceptions.raise_compiler_error("Error: argument `relation` is required for `unpivot` macro.") }}
Expand All @@ -43,18 +43,19 @@ Arguments:


{%- for col in include_cols -%}
{%- set current_col_name = adapter.quote(col.column) if quote_identifiers else col.column -%}
select
{%- for exclude_col in exclude %}
{{ exclude_col }},
{{ adapter.quote(exclude_col) if quote_identifiers else exclude_col }},
{%- endfor %}

cast('{{ col.column }}' as {{ dbt.type_string() }}) as {{ field_name }},
cast('{{ col.column }}' as {{ dbt.type_string() }}) as {{ adapter.quote(field_name) if quote_identifiers else field_name }},
cast( {% if col.data_type == 'boolean' %}
{{ dbt.cast_bool_to_text(col.column) }}
{{ dbt.cast_bool_to_text(current_col_name) }}
{% else %}
{{ col.column }}
{{ current_col_name }}
{% endif %}
as {{ cast_to }}) as {{ value_name }}
as {{ cast_to }}) as {{ adapter.quote(value_name) if quote_identifiers else value_name }}

from {{ relation }}

Expand Down