From 08d79cc324918e9ada44ec7864871c850995b9d2 Mon Sep 17 00:00:00 2001 From: Drew Banin Date: Fri, 5 Jul 2019 21:14:54 -0400 Subject: [PATCH] (#1588) fix for dupe check_cols values in snapshot strategy --- core/dbt/contracts/graph/parsed.py | 2 +- .../macros/materializations/snapshot/strategies.sql | 3 ++- core/dbt/parser/snapshots.py | 9 +++++++++ test/integration/004_simple_snapshot_test/seed_pg.sql | 6 ++++-- .../test-snapshots-invalid/snapshot.sql | 1 - .../004_simple_snapshot_test/test_simple_snapshot.py | 2 +- 6 files changed, 17 insertions(+), 6 deletions(-) diff --git a/core/dbt/contracts/graph/parsed.py b/core/dbt/contracts/graph/parsed.py index 62aa169b92d..df70c8b80f9 100644 --- a/core/dbt/contracts/graph/parsed.py +++ b/core/dbt/contracts/graph/parsed.py @@ -500,7 +500,7 @@ def config(self, value): ] }, 'required': [ - 'target_database', 'target_schema', 'unique_key', 'strategy', + 'target_schema', 'unique_key', 'strategy', ], } diff --git a/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql b/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql index b8097eeacf2..452cfbbd66c 100644 --- a/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql +++ b/core/dbt/include/global_project/macros/materializations/snapshot/strategies.sql @@ -106,7 +106,8 @@ ) {%- endset %} - {% set scd_id_expr = snapshot_hash_arguments(check_cols) %} + {% set scd_id_cols = [primary_key] + (check_cols | list) %} + {% set scd_id_expr = snapshot_hash_arguments(scd_id_cols) %} {% do return({ "unique_key": primary_key, diff --git a/core/dbt/parser/snapshots.py b/core/dbt/parser/snapshots.py index 8733e585efb..aa01d1b4179 100644 --- a/core/dbt/parser/snapshots.py +++ b/core/dbt/parser/snapshots.py @@ -8,6 +8,15 @@ def set_snapshot_attributes(node): + # Default the target database to the database specified in the target + # This line allows target_database to be optional in the snapshot config + if 'target_database' not in node.config: + node.config['target_database'] = node.database + + # Set the standard node configs (database+schema) to be the specified + # values from target_database and target_schema. This ensures that the + # database and schema names are interopolated correctly when snapshots + # are ref'd from other models config_keys = { 'target_database': 'database', 'target_schema': 'schema' diff --git a/test/integration/004_simple_snapshot_test/seed_pg.sql b/test/integration/004_simple_snapshot_test/seed_pg.sql index 9d3d47da51f..a22a2359c6b 100644 --- a/test/integration/004_simple_snapshot_test/seed_pg.sql +++ b/test/integration/004_simple_snapshot_test/seed_pg.sql @@ -26,9 +26,11 @@ create table {database}.{schema}.snapshot_expected ( -- seed inserts +-- use the same email for two users to verify that duplicated check_cols values +-- are handled appropriately insert into {database}.{schema}.seed (id, first_name, last_name, email, gender, ip_address, updated_at) values -(1, 'Judith', 'Kennedy', 'jkennedy0@phpbb.com', 'Female', '54.60.24.128', '2015-12-24 12:19:28'), -(2, 'Arthur', 'Kelly', 'akelly1@eepurl.com', 'Male', '62.56.24.215', '2015-10-28 16:22:15'), +(1, 'Judith', 'Kennedy', '(not provided)', 'Female', '54.60.24.128', '2015-12-24 12:19:28'), +(2, 'Arthur', 'Kelly', '(not provided)', 'Male', '62.56.24.215', '2015-10-28 16:22:15'), (3, 'Rachel', 'Moreno', 'rmoreno2@msu.edu', 'Female', '31.222.249.23', '2016-04-05 02:05:30'), (4, 'Ralph', 'Turner', 'rturner3@hp.com', 'Male', '157.83.76.114', '2016-08-08 00:06:51'), (5, 'Laura', 'Gonzales', 'lgonzales4@howstuffworks.com', 'Female', '30.54.105.168', '2016-09-01 08:25:38'), diff --git a/test/integration/004_simple_snapshot_test/test-snapshots-invalid/snapshot.sql b/test/integration/004_simple_snapshot_test/test-snapshots-invalid/snapshot.sql index 0e39d9aa739..88c0e72f0e4 100644 --- a/test/integration/004_simple_snapshot_test/test-snapshots-invalid/snapshot.sql +++ b/test/integration/004_simple_snapshot_test/test-snapshots-invalid/snapshot.sql @@ -1,7 +1,6 @@ {% snapshot no_target_database %} {{ config( - target_schema=schema, unique_key='id || ' ~ "'-'" ~ ' || first_name', strategy='timestamp', updated_at='updated_at', diff --git a/test/integration/004_simple_snapshot_test/test_simple_snapshot.py b/test/integration/004_simple_snapshot_test/test_simple_snapshot.py index a3385627648..f0bea1745da 100644 --- a/test/integration/004_simple_snapshot_test/test_simple_snapshot.py +++ b/test/integration/004_simple_snapshot_test/test_simple_snapshot.py @@ -353,7 +353,7 @@ def test__postgres__invalid(self): with self.assertRaises(dbt.exceptions.CompilationException) as exc: self.run_dbt(['compile'], expect_pass=False) - self.assertIn('target_database', str(exc.exception)) + self.assertIn('target_schema', str(exc.exception)) class TestCheckCols(TestSimpleSnapshotFiles):