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

Fix: Create table statement parameter ordering when configured with backup & dist/sort #63

Merged
merged 13 commits into from
Mar 3, 2022
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Fixes
- Fix test related to preventing coercion of boolean values (True,False) to numeric values (0,1) in query results ([#58](https://github.com/dbt-labs/dbt-redshift/blob/1.0.latest/CHANGELOG.md))
- Fix table creation statement ordering when including both the BACKUP parameter as well as the dist/sort keys ([#23](https://github.com/dbt-labs/dbt-redshift/issues/60))

## dbt-redshift 1.0.0 (December 3, 2021)

Expand Down
2 changes: 1 addition & 1 deletion dbt/include/redshift/macros/adapters.sql
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@

create {% if temporary -%}temporary{%- endif %} table
{{ relation.include(database=(not temporary), schema=(not temporary)) }}
{% if backup == false -%}backup no{%- endif %}
{{ dist(_dist) }}
{{ sort(_sort_type, _sort) }}
{% if backup == false -%}backup no{%- endif %}
as (
{{ sql }}
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{{
config(
materialized='table', backup=True, dist='distkey'
)
}}

select 1 as distkey
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{{
config(
materialized='table', backup=True, sort='sortkey'
)
}}

select 1 as sortkey
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,16 @@ def check_backup_param_template(self, test_table_name, backup_is_expected):
with open('target/run/test/models/{}.sql'.format(test_table_name), 'r') as ddl_file:
ddl_statement = ddl_file.readlines()
self.assertEqual('backup no' not in ' '.join(ddl_statement).lower(), backup_is_expected)
VersusFacit marked this conversation as resolved.
Show resolved Hide resolved
if backup_is_expected:
distkey_index = ' '.join(ddl_statement).lower().find('distkey')
VersusFacit marked this conversation as resolved.
Show resolved Hide resolved
sortkey_index = ' '.join(ddl_statement).lower().find('sortkey')
backup_index = ' '.join(ddl_statement).lower().find('backup no')
self.assertEqual((backup_index < distkey_index) or distkey_index == -1, backup_is_expected)
self.assertEqual((backup_index < sortkey_index) or sortkey_index == -1, backup_is_expected)

@use_profile('redshift')
def test__redshift_backup_table_option(self):
self.assertEqual(len(self.run_dbt()), 4)
self.assertEqual(len(self.run_dbt()), 6)

# model_backup_undefined should not contain a BACKUP NO parameter in the table DDL
self.check_backup_param_template('model_backup_undefined', True)
Expand All @@ -44,6 +50,11 @@ def test__redshift_backup_table_option(self):
# Any view should not contain a BACKUP NO parameter, regardless of the specified config (create will fail)
self.check_backup_param_template('model_backup_true_view', True)

# model_backup_param_before_distkey should contain a BACKUP NO parameter which precedes a DISTKEY in the table ddl
self.check_backup_param_template('model_backup_param_before_distkey', True)

# model_backup_param_before_sortkey should contain a BACKUP NO parameter which precedes a SORTKEY in the table ddl
self.check_backup_param_template('model_backup_param_before_sortkey', True)

class TestBackupTableOptionProjectFalse(DBTIntegrationTest):
@property
Expand Down Expand Up @@ -72,10 +83,16 @@ def check_backup_param_template(self, test_table_name, backup_is_expected):
with open('target/run/test/models/{}.sql'.format(test_table_name), 'r') as ddl_file:
ddl_statement = ddl_file.readlines()
self.assertEqual('backup no' not in ' '.join(ddl_statement).lower(), backup_is_expected)
if backup_is_expected:
distkey_index = ' '.join(ddl_statement).lower().find('distkey')
sortkey_index = ' '.join(ddl_statement).lower().find('sortkey')
backup_index = ' '.join(ddl_statement).lower().find('backup no')
self.assertEqual((backup_index < distkey_index) or distkey_index == -1, backup_is_expected)
self.assertEqual((backup_index < sortkey_index) or sortkey_index == -1, backup_is_expected)

@use_profile('redshift')
def test__redshift_backup_table_option_project_config_false(self):
self.assertEqual(len(self.run_dbt()), 4)
self.assertEqual(len(self.run_dbt()), 6)

# model_backup_undefined should contain a BACKUP NO parameter in the table DDL
self.check_backup_param_template('model_backup_undefined', False)
Expand All @@ -88,3 +105,9 @@ def test__redshift_backup_table_option_project_config_false(self):

# Any view should not contain a BACKUP NO parameter, regardless of the specified config (create will fail)
self.check_backup_param_template('model_backup_true_view', True)

# model_backup_param_before_distkey should contain a BACKUP NO parameter which precedes a DISTKEY in the table ddl
self.check_backup_param_template('model_backup_param_before_distkey', True)

# model_backup_param_before_sortkey should contain a BACKUP NO parameter which precedes a SORTKEY in the table ddl
self.check_backup_param_template('model_backup_param_before_sortkey', True)