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))
- Add unique\_id field to docs generation test catalogs; a follow-on PR to core PR ([#4168](https://github.com/dbt-labs/dbt-core/pull/4618)) and core PR ([#4701](https://github.com/dbt-labs/dbt-core/pull/4701))

## 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=False, dist='distkey'
)
}}

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

select 1 as sortkey
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ def check_backup_param_template(self, test_table_name, backup_is_expected):
# Use raw DDL statement to confirm backup is set correctly on new table
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)
lowercase_statement = ' '.join(ddl_statement).lower()
self.assertEqual('backup no' not in lowercase_statement, 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,7 +45,6 @@ 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)


class TestBackupTableOptionProjectFalse(DBTIntegrationTest):
@property
def schema(self):
Expand All @@ -71,11 +71,12 @@ def check_backup_param_template(self, test_table_name, backup_is_expected):
# Use raw DDL statement to confirm backup is set correctly on new table
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)
lowercase_statement = ' '.join(ddl_statement).lower()
self.assertEqual('backup no' not in lowercase_statement, 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 +89,45 @@ 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)

class TestBackupTableOptionOrder(DBTIntegrationTest):
@property
def schema(self):
return 'backup_table_tests'

@staticmethod
def dir(path):
return os.path.normpath(path)

@property
def models(self):
return self.dir("models")

@property
def project_config(self):
return {
'config-version': 2
}

def check_backup_param_template(self, test_table_name, backup_flag_is_expected):
# Use raw DDL statement to confirm backup is set correctly on new table
with open('target/run/test/models/{}.sql'.format(test_table_name), 'r') as ddl_file:
ddl_statement = ddl_file.readlines()
lowercase_statement = ' '.join(ddl_statement).lower()
self.assertEqual('backup no' not in lowercase_statement, backup_flag_is_expected)
if backup_flag_is_expected:
distkey_index = lowercase_statement.find('distkey')
sortkey_index = lowercase_statement.find('sortkey')
backup_index = lowercase_statement.find('backup no')
self.assertEqual((backup_index < distkey_index) or distkey_index == -1, backup_flag_is_expected)
self.assertEqual((backup_index < sortkey_index) or sortkey_index == -1, backup_flag_is_expected)

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

# 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', False)

# 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', False)