From 6a92f415851c36c721a984e4a413dbc67d1de8da Mon Sep 17 00:00:00 2001 From: Dan Flippo Date: Thu, 22 Sep 2022 14:44:20 -0400 Subject: [PATCH] Mostly changes for syntax on compound not null --- integration_tests/dbt_project.yml | 1 + macros/create_constraints.sql | 24 ++++----- macros/oracle__create_constraints.sql | 64 +++++++++++++++++++--- macros/postgres__create_constraints.sql | 67 ++++++++++++++---------- macros/snowflake__create_constraints.sql | 19 ++++--- packages.yml | 3 -- 6 files changed, 120 insertions(+), 58 deletions(-) delete mode 100644 packages.yml diff --git a/integration_tests/dbt_project.yml b/integration_tests/dbt_project.yml index 31e6c36..34588e5 100644 --- a/integration_tests/dbt_project.yml +++ b/integration_tests/dbt_project.yml @@ -48,3 +48,4 @@ models: seeds: +quote_columns: false + +full_refresh: false diff --git a/macros/create_constraints.sql b/macros/create_constraints.sql index 32a54ff..3f83340 100644 --- a/macros/create_constraints.sql +++ b/macros/create_constraints.sql @@ -99,6 +99,11 @@ {{ return(adapter.dispatch('truncate_relation')(relation)) }} {% endmacro %} +{#- Override dbt's drop_relation macro to allow us to create adapter specific versions that drop constraints -#} + +{% macro drop_relation(relation) -%} + {{ return(adapter.dispatch('drop_relation')(relation)) }} +{% endmacro %} @@ -169,13 +174,12 @@ {#- Find the table models that are referenced by this test. These models must be physical tables and cannot be sources -#} {%- set table_models = [] -%} - {%- for node in graph.nodes.values() - | selectattr("resource_type", "equalto", "model") - | selectattr("unique_id", "in", test_model.depends_on.nodes) - if node.config.materialized in( ("table", "incremental", "snapshot") ) -%} + {%- for node in graph.nodes.values() | selectattr("unique_id", "in", test_model.depends_on.nodes) + if node.resource_type in ( ( "model", "snapshot") ) + if node.config.materialized in( ("table", "incremental", "snapshot") ) -%} - {#- Append to our list of models for this test -#} - {%- do table_models.append(node) -%} + {#- Append to our list of models &or snapshots for this test -#} + {%- do table_models.append(node) -%} {% endfor %} @@ -225,8 +229,8 @@ identifier=table_models[0].alias ) -%} {%- if dbt_constraints.table_columns_all_exist(table_relation, column_names) -%} {%- if test_model.test_metadata.name == "primary_key" -%} - {%- do dbt_constraints.create_primary_key(table_relation, column_names, ns.verify_permissions, quote_columns) -%} {%- do dbt_constraints.create_not_null(table_relation, column_names, ns.verify_permissions, quote_columns) -%} + {%- do dbt_constraints.create_primary_key(table_relation, column_names, ns.verify_permissions, quote_columns) -%} {%- else -%} {%- do dbt_constraints.create_unique_key(table_relation, column_names, ns.verify_permissions, quote_columns) -%} {%- endif -%} @@ -307,7 +311,7 @@ {%- else -%} {%- do log("Skipping foreign key because a we couldn't find the child table: model=" ~ fk_model_names ~ " or source=" ~ fk_source_names, info=true) -%} {%- endif -%} - + {#- We only create NN if there is one model referenced by the test and if all the columns exist as physical columns on the table -#} {%- elif 1 == table_models|count @@ -380,7 +384,3 @@ {{ return(false) }} {%- endif -%} {%- endmacro -%} - - -{%- macro drop_constraints() -%} -{%- endmacro -%} diff --git a/macros/oracle__create_constraints.sql b/macros/oracle__create_constraints.sql index a96a837..1bf4514 100644 --- a/macros/oracle__create_constraints.sql +++ b/macros/oracle__create_constraints.sql @@ -132,21 +132,31 @@ END; {# Oracle specific implementation to create a not null constraint #} {%- macro oracle__create_not_null(table_relation, column_names, verify_permissions, quote_columns=false) -%} - {%- set columns_csv = dbt_constraints.get_quoted_column_csv(column_names, quote_columns) -%} + {%- set columns_list = dbt_constraints.get_quoted_column_list(column_names, quote_columns) -%} {%- if dbt_constraints.have_ownership_priv(table_relation, verify_permissions) -%} - {%- set query -%} - ALTER TABLE {{table_relation}} MODIFY ( {{columns_csv}} NOT NULL); - {%- endset -%} - {%- do log("Creating not null constraint for: " ~ columns_csv ~ " in " ~ table_relation, info=true) -%} - {%- do run_query(query) -%} + {%- set modify_statements= [] -%} + {%- for column in columns_list -%} + {%- set modify_statements = modify_statements.append( column ~ " NOT NULL" ) -%} + {%- endfor -%} + {%- set modify_statement_csv = modify_statements | join(", ") -%} + {%- set query -%} +BEGIN + EXECUTE IMMEDIATE 'ALTER TABLE {{table_relation}} MODIFY ( {{ modify_statement_csv }} )'; +EXCEPTION + WHEN OTHERS THEN + DBMS_OUTPUT.ENABLE(BUFFER_SIZE => NULL); + DBMS_OUTPUT.PUT_LINE('Unable to create constraint: ' || SQLERRM); +END; + {%- endset -%} + {%- do log("Creating not null constraint for: " ~ columns_list | join(", ") ~ " in " ~ table_relation, info=true) -%} + {%- do run_query(query) -%} {%- else -%} - {%- do log("Skipping not null constraint for " ~ columns_csv ~ " in " ~ table_relation ~ " because of insufficient privileges: " ~ table_relation, info=true) -%} + {%- do log("Skipping not null constraint for " ~ columns_list | join(", ") ~ " in " ~ table_relation ~ " because of insufficient privileges: " ~ table_relation, info=true) -%} {%- endif -%} {%- endmacro -%} - {#- This macro is used in create macros to avoid duplicate PK/UK constraints and to skip FK where no PK/UK constraint exists on the parent table -#} {%- macro oracle__unique_constraint_exists(table_relation, column_names) -%} @@ -223,3 +233,41 @@ order by 1, 2 {%- macro oracle__have_ownership_priv(table_relation, verify_permissions) -%} {{ return(true) }} {%- endmacro -%} + +{% macro oracle__drop_referential_constraints(relation) -%} + {%- call statement('drop_constraint_cascade') -%} +BEGIN + FOR REC IN ( + SELECT constraint_name + FROM all_constraints cons + WHERE cons.constraint_type IN ('P', 'U', 'R') + AND upper(cons.owner) = upper('{{relation.schema}}') + AND upper(cons.table_name) = upper('{{relation.identifier}}') + ORDER BY 1 + ) LOOP + BEGIN + EXECUTE IMMEDIATE 'ALTER TABLE {{relation}} DROP CONSTRAINT "'||REC.CONSTRAINT_NAME||'" CASCADE'; + EXCEPTION + WHEN OTHERS THEN + DBMS_OUTPUT.ENABLE(BUFFER_SIZE => NULL); + DBMS_OUTPUT.PUT_LINE('Unable to drop constraint: ' || SQLERRM); + END; + END LOOP; +END; + {%- endcall -%} + +{% endmacro %} + +{#- Oracle will error if you try to truncate tables with FK constraints or tables with PK/UK constraints + referenced by FK so we will drop all constraints before truncating tables -#} +{% macro oracle__truncate_relation(relation) -%} + {{ oracle__drop_referential_constraints(relation) }} + {{ return(adapter.dispatch('truncate_relation', 'dbt')(relation)) }} +{% endmacro %} + +{#- Oracle will error if you try to drop tables with FK constraints or tables with PK/UK constraints + referenced by FK so we will drop all constraints before dropping tables -#} +{% macro oracle__drop_relation(relation) -%} + {{ oracle__drop_referential_constraints(relation) }} + {{ return(adapter.dispatch('drop_relation', 'dbt')(relation)) }} +{% endmacro %} diff --git a/macros/postgres__create_constraints.sql b/macros/postgres__create_constraints.sql index be4b2d1..7fe70ca 100644 --- a/macros/postgres__create_constraints.sql +++ b/macros/postgres__create_constraints.sql @@ -17,11 +17,10 @@ {%- if dbt_constraints.have_ownership_priv(table_relation, verify_permissions) -%} - {%- set query -%} - ALTER TABLE {{table_relation}} ADD CONSTRAINT {{constraint_name}} PRIMARY KEY ( {{columns_csv}} ) - {%- endset -%} {%- do log("Creating primary key: " ~ constraint_name, info=true) -%} - {%- do run_query(query) -%} + {%- call statement('add_pk', fetch_result=False, auto_begin=True) -%} + ALTER TABLE {{table_relation}} ADD CONSTRAINT {{constraint_name}} PRIMARY KEY ( {{columns_csv}} ) + {%- endcall -%} {{ adapter.commit() }} {%- else -%} @@ -55,11 +54,10 @@ {%- if dbt_constraints.have_ownership_priv(table_relation, verify_permissions) -%} - {%- set query -%} - ALTER TABLE {{table_relation}} ADD CONSTRAINT {{constraint_name}} UNIQUE ( {{columns_csv}} ) - {%- endset -%} {%- do log("Creating unique key: " ~ constraint_name, info=true) -%} - {%- do run_query(query) -%} + {%- call statement('add_uk', fetch_result=False, auto_begin=True) -%} + ALTER TABLE {{table_relation}} ADD CONSTRAINT {{constraint_name}} UNIQUE ( {{columns_csv}} ) + {%- endcall -%} {{ adapter.commit() }} {%- else -%} @@ -74,18 +72,23 @@ {# PostgreSQL specific implementation to create a not null constraint #} {%- macro postgres__create_not_null(table_relation, column_names, verify_permissions, quote_columns=false) -%} - {%- set columns_csv = dbt_constraints.get_quoted_column_csv(column_names, quote_columns) -%} + {%- set columns_list = dbt_constraints.get_quoted_column_list(column_names, quote_columns) -%} {%- if dbt_constraints.have_ownership_priv(table_relation, verify_permissions) -%} - {%- set query -%} - ALTER TABLE {{table_relation}} ALTER COLUMN {{columns_csv}} SET NOT NULL; - {%- endset -%} - {%- do log("Creating not null constraint for: " ~ columns_csv ~ " in " ~ table_relation, info=true) -%} - {%- do run_query(query) -%} + {%- set modify_statements= [] -%} + {%- for column in columns_list -%} + {%- set modify_statements = modify_statements.append( "ALTER COLUMN " ~ column ~ " SET NOT NULL" ) -%} + {%- endfor -%} + {%- set modify_statement_csv = modify_statements | join(", ") -%} + {%- do log("Creating not null constraint for: " ~ columns_list | join(", ") ~ " in " ~ table_relation, info=true) -%} + {%- call statement('add_nn', fetch_result=False, auto_begin=True) -%} + ALTER TABLE {{table_relation}} {{ modify_statement_csv }}; + {%- endcall -%} + {{ adapter.commit() }} {%- else -%} - {%- do log("Skipping not null constraint for " ~ columns_csv ~ " in " ~ table_relation ~ " because of insufficient privileges: " ~ table_relation, info=true) -%} + {%- do log("Skipping not null constraint for " ~ columns_list | join(", ") ~ " in " ~ table_relation ~ " because of insufficient privileges: " ~ table_relation, info=true) -%} {%- endif -%} {%- endmacro -%} @@ -110,11 +113,10 @@ {%- if dbt_constraints.have_ownership_priv(fk_table_relation, verify_permissions) and dbt_constraints.have_references_priv(pk_table_relation, verify_permissions) -%} - {%- set query -%} - ALTER TABLE {{fk_table_relation}} ADD CONSTRAINT {{constraint_name}} FOREIGN KEY ( {{fk_columns_csv}} ) REFERENCES {{pk_table_relation}} ( {{pk_columns_csv}} ) ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED - {%- endset -%} {%- do log("Creating foreign key: " ~ constraint_name ~ " referencing " ~ pk_table_relation.identifier ~ " " ~ pk_column_names, info=true) -%} - {%- do run_query(query) -%} + {%- call statement('add_fk', fetch_result=False, auto_begin=True) -%} + ALTER TABLE {{fk_table_relation}} ADD CONSTRAINT {{constraint_name}} FOREIGN KEY ( {{fk_columns_csv}} ) REFERENCES {{pk_table_relation}} ( {{pk_columns_csv}} ) ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED + {%- endcall -%} {{ adapter.commit() }} {%- else -%} @@ -246,11 +248,7 @@ {%- endmacro -%} - - -{#- PostgreSQL will error if you try to truncate tables with FK constraints or tables with PK/UK constraints - referenced by FK so we will drop all constraints before truncating tables -#} -{% macro postgres__truncate_relation(relation) -%} +{% macro postgres__drop_referential_constraints(relation) -%} {%- set lookup_query -%} select constraint_name from information_schema.table_constraints @@ -261,12 +259,25 @@ {%- set constraint_list = run_query(lookup_query) -%} {%- for constraint_name in constraint_list.columns["constraint_name"].values() -%} - {%- set drop_statement -%} - ALTER TABLE {{relation}} DROP CONSTRAINT "{{constraint_name}}" CASCADE - {%- endset -%} {%- do log("Dropping constraint: " ~ constraint_name ~ " from table " ~ relation, info=false) -%} - {%- do run_query(drop_statement) -%} + {%- call statement('drop_constraint_cascade', fetch_result=False, auto_begin=True) -%} + ALTER TABLE {{relation}} DROP CONSTRAINT IF EXISTS "{{constraint_name}}" CASCADE + {%- endcall -%} + {{ adapter.commit() }} {% endfor %} +{% endmacro %} + +{#- PostgreSQL will error if you try to truncate tables with FK constraints or tables with PK/UK constraints + referenced by FK so we will drop all constraints before truncating tables -#} +{% macro postgres__truncate_relation(relation) -%} + {{ postgres__drop_referential_constraints(relation) }} {{ return(adapter.dispatch('truncate_relation', 'dbt')(relation)) }} {% endmacro %} + +{#- PostgreSQL will get deadlocks if you try to drop tables with FK constraints or tables with PK/UK constraints + referenced by FK so we will drop all constraints before dropping tables -#} +{% macro postgres__drop_relation(relation) -%} + {{ postgres__drop_referential_constraints(relation) }} + {{ return(adapter.dispatch('drop_relation', 'dbt')(relation)) }} +{% endmacro %} diff --git a/macros/snowflake__create_constraints.sql b/macros/snowflake__create_constraints.sql index 5bcf642..152b10d 100644 --- a/macros/snowflake__create_constraints.sql +++ b/macros/snowflake__create_constraints.sql @@ -88,18 +88,23 @@ {# Snowflake specific implementation to create a not null constraint #} {%- macro snowflake__create_not_null(table_relation, column_names, verify_permissions, quote_columns=false) -%} - {%- set columns_csv = dbt_constraints.get_quoted_column_csv(column_names, quote_columns) -%} + {%- set columns_list = dbt_constraints.get_quoted_column_list(column_names, quote_columns) -%} {%- if dbt_constraints.have_ownership_priv(table_relation, verify_permissions) -%} - {%- set query -%} - ALTER TABLE {{table_relation}} MODIFY COLUMN {{columns_csv}} SET NOT NULL; - {%- endset -%} - {%- do log("Creating not null constraint for: " ~ columns_csv ~ " in " ~ table_relation, info=true) -%} - {%- do run_query(query) -%} + {%- set modify_statements= [] -%} + {%- for column in columns_list -%} + {%- set modify_statements = modify_statements.append( "COLUMN " ~ column ~ " SET NOT NULL" ) -%} + {%- endfor -%} + {%- set modify_statement_csv = modify_statements | join(", ") -%} + {%- set query -%} + ALTER TABLE {{table_relation}} MODIFY {{ modify_statement_csv }}; + {%- endset -%} + {%- do log("Creating not null constraint for: " ~ columns_list | join(", ") ~ " in " ~ table_relation, info=true) -%} + {%- do run_query(query) -%} {%- else -%} - {%- do log("Skipping not null constraint for " ~ columns_csv ~ " in " ~ table_relation ~ " because of insufficient privileges: " ~ table_relation, info=true) -%} + {%- do log("Skipping not null constraint for " ~ columns_list | join(", ") ~ " in " ~ table_relation ~ " because of insufficient privileges: " ~ table_relation, info=true) -%} {%- endif -%} {%- endmacro -%} diff --git a/packages.yml b/packages.yml deleted file mode 100644 index 538aeec..0000000 --- a/packages.yml +++ /dev/null @@ -1,3 +0,0 @@ -packages: - - package: dbt-labs/dbt_utils - version: [">=0.8.0", "<0.9.0"]