Addressing issue 189

microsoft · Jun 11, 2024 · 2c75abf · 2c75abf
1 parent 4ed06da
commit 2c75abf
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 37 deletions.
diff --git a/dbt/include/fabric/macros/materializations/models/table/create_table_as.sql b/dbt/include/fabric/macros/materializations/models/table/create_table_as.sql
@@ -1,12 +1,25 @@
 {% macro fabric__create_table_as(temporary, relation, sql) -%}
 
-    {% set tmp_relation = relation.incorporate(path={"identifier": relation.identifier ~ '__dbt_tmp_vw'}, type='view')-%}
-    {{ get_create_view_as_sql(tmp_relation, sql) }}
+    -- Make temp view relation
+    {% set tmp_vw_relation = relation.incorporate(path={"identifier": relation.identifier ~ '__dbt_tmp_vw'}, type='view')-%}
+
+    -- drop temp relation if exists
+    {% do adapter.drop_relation(tmp_vw_relation) %}
+
+    -- Fabric & Synapse adapters use temp relation because of lack of CTE support for CTE in CTAS, Insert
+    {{ get_create_view_as_sql(tmp_vw_relation, sql) }}
+
+    -- Temp relation will come in use when contract is enforced or not
+    -- Making a temp relation
+    {% set temp_relation = make_temp_relation(relation, '__dbt_temp') %}
+
+    -- Dropping a temp relation if it exists
+    {% do adapter.drop_relation(temp_relation) %}
 
     {% set contract_config = config.get('contract') %}
     {% if contract_config.enforced %}
 
-        CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}]
+        CREATE TABLE {{temp_relation}}
         {{ build_columns_constraints(relation) }}
         {{ get_assert_columns_equivalent(sql)  }}
         {% set listColumns %}
@@ -15,11 +28,44 @@
             {% endfor %}
         {%endset%}
 
-        INSERT INTO [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}]
-        ({{listColumns}}) SELECT {{listColumns}} FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}];
+        INSERT INTO {{temp_relation}} ({{listColumns}})
+        SELECT {{listColumns}} FROM {{tmp_vw_relation}};
 
     {%- else %}
-        EXEC('CREATE TABLE [{{relation.database}}].[{{relation.schema}}].[{{relation.identifier}}] AS (SELECT * FROM [{{tmp_relation.database}}].[{{tmp_relation.schema}}].[{{tmp_relation.identifier}}]);');
+        -- CTAS
+        {{ log("In CTAS") }}
+        {# EXEC('CREATE TABLE {{temp_relation}} AS SELECT * FROM {{tmp_vw_relation}} }}'); #}
+        SELECT 1
+        EXEC('CREATE TABLE [{{temp_relation.database}}].[{{temp_relation.schema}}].[{{temp_relation.identifier}}] AS (SELECT * FROM [{{tmp_vw_relation.database}}].[{{tmp_vw_relation.schema}}].[{{tmp_vw_relation.identifier}}]);');
     {% endif %}
-    {% do adapter.drop_relation(tmp_relation)%}
+
+    -- making a backup relation, this will come in use when contract is enforced or not
+    {%- set backup_relation = make_backup_relation(relation, 'table') -%}
+
+    -- Dropping a temp relation if it exists
+    {% do adapter.drop_relation(backup_relation) %}
+
+    {%- set check_if_relation_exists = adapter.get_relation(database=relation.database, schema=relation.schema, identifier=relation.identifier) -%}
+    {% if check_if_relation_exists is not none %}
+
+        {{ adapter.rename_relation(relation, backup_relation) }}
+
+        -- Renaming temp relation as main relation
+        {{ adapter.rename_relation(temp_relation, relation) }}
+
+        -- Drop backup relation
+        {% do adapter.drop_relation(backup_relation) %}
+
+    {%- else %}
+
+        -- Renaming temp relation as main relation
+        {{ adapter.rename_relation(temp_relation, relation) }}
+
+        -- Dropping temp relation
+        {% do adapter.drop_relation(temp_relation) %}
+
+    {% endif %}
+
+    -- Dropping temp view relation
+    {% do adapter.drop_relation(tmp_vw_relation)%}
 {% endmacro %}
diff --git a/dbt/include/fabric/macros/materializations/models/table/table.sql b/dbt/include/fabric/macros/materializations/models/table/table.sql
@@ -4,18 +4,11 @@
   {%- set target_relation = this.incorporate(type='table') %}
   {%- set existing_relation = adapter.get_relation(database=this.database, schema=this.schema, identifier=this.identifier) -%}
 
-  {%- set backup_relation = none %}
-  {% if (existing_relation != none and existing_relation.type == "table") %}
-      {%- set backup_relation = make_backup_relation(target_relation, 'table') -%}
-  {% elif (existing_relation != none and existing_relation.type == "view") %}
-      {%- set backup_relation = make_backup_relation(target_relation, 'view') -%}
-  {% endif %}
-
-  {% if (existing_relation != none) %}
-    -- drop the temp relations if they exist already in the database
-    {% do adapter.drop_relation(backup_relation) %}
-    -- Rename target relation as backup relation
-    {{ adapter.rename_relation(existing_relation, backup_relation) }}
+  {#-- Drop the relation if it was a view to "convert" it in a table. This may lead to
+    -- downtime, but it should be a relatively infrequent occurrence  #}
+  {% if existing_relation is not none and not old_relation.is_table %}
+    {{ log("Dropping relation " ~ existing_relation ~ " because it is of type " ~ existing_relation.type) }}
+    {% do adapter.drop_relation(existing_relation) %}
   {% endif %}
 
   -- grab current tables grants config for comparision later on
@@ -25,21 +18,11 @@
   -- `BEGIN` happens here:
   {{ run_hooks(pre_hooks, inside_transaction=True) }}
 
-  -- naming a temp relation
-  {% set tmp_relation = target_relation.incorporate(path={"identifier": target_relation.identifier ~ '__dbt_tmp_vw'}, type='view')-%}
-
-  -- Fabric & Synapse adapters use temp relation because of lack of CTE support for CTE in CTAS, Insert
-  -- drop temp relation if exists
-  {% do adapter.drop_relation(tmp_relation) %}
-
   -- build model
   {% call statement('main') -%}
-    {{ get_create_table_as_sql(False, target_relation, sql) }}
+    {{ create_table_as(False, target_relation, sql) }}
   {%- endcall %}
 
-  -- drop temp relation if exists
-  {% do adapter.drop_relation(tmp_relation) %}
-
   -- cleanup
   {{ run_hooks(post_hooks, inside_transaction=True) }}
 
@@ -48,13 +31,6 @@
   -- `COMMIT` happens here
   {{ adapter.commit() }}
 
-  {% if (backup_relation != none) %}
-    -- finally, drop the foreign key references if exists
-    {{ drop_fk_indexes_on_table(backup_relation) }}
-    -- drop existing/backup relation after the commit
-    {% do adapter.drop_relation(backup_relation) %}
-   {% endif %}
-
   -- Add constraints including FK relation.
   {{ build_model_constraints(target_relation) }}
   {{ run_hooks(post_hooks, inside_transaction=False) }}