Repository: databricks/dbt-databricks
Pull request #765 (2024-08-12 21:32:25)

Update tblproperties on incremental runs
<!-- Please review our pull request review process in CONTRIBUTING.md before your proceed. -->

Addresses #760 (doesn't fully resolve because 760 also discusses comment updates)

<!---
  Include the number of the issue addressed by this PR above if applicable.

  Example:
    resolves #1234

  Please review our pull request review process in CONTRIBUTING.md before your proceed.
-->

### Description

Incremental tables have not been updating tblproperties on incremental runs.  This PR adds support for add/replace tblproperties on incremental run.  Delete was tested, but too risky/complicated, as Databricks sets some tblproperties for tables on its own.

### Checklist

- [x] I have run this code in development and it appears to resolve the stated issue
- [x] This PR includes tests, or tests are not required/relevant for this PR
- [x] I have updated the `CHANGELOG.md` and added information about my change to the "dbt-databricks next" section.


benc-db: Adding Eric, Yunbo to review since Jacky is out.  I will comment the code so that you have some context.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 47ce01b4..ecec7fbc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ### Fixes
 
 - Persist table comments for incremental models, snapshots and dbt clone (thanks @henlue!) ([750](https://github.com/databricks/dbt-databricks/pull/750))
+- Update tblproperties on incremental runs. Note: only adds/edits. Deletes are too risky/complex for now ([765](https://github.com/databricks/dbt-databricks/pull/765))
 
 ## dbt-databricks 1.8.5 (August 6, 2024)
 
diff --git a/dbt/adapters/databricks/impl.py b/dbt/adapters/databricks/impl.py
index 2f4f4a87..46cb11dd 100644
--- a/dbt/adapters/databricks/impl.py
+++ b/dbt/adapters/databricks/impl.py
@@ -817,4 +817,5 @@ class IncrementalTableAPI(RelationAPIBase[IncrementalTableConfig]):
 
         if not relation.is_hive_metastore():
             results["information_schema.tags"] = adapter.execute_macro("fetch_tags", kwargs=kwargs)
+        results["show_tblproperties"] = adapter.execute_macro("fetch_tbl_properties", kwargs=kwargs)
         return results
diff --git a/dbt/adapters/databricks/relation_configs/base.py b/dbt/adapters/databricks/relation_configs/base.py
index 79cfe9ed..6fa36ee9 100644
--- a/dbt/adapters/databricks/relation_configs/base.py
+++ b/dbt/adapters/databricks/relation_configs/base.py
@@ -38,11 +38,6 @@ class DatabricksComponentConfig(BaseModel):
         the complete config specified in the dbt project, so as to support rendering the `create`
         as well as the `alter` statements, for the case where a different component requires full
         refresh.
-
-        Consider updating tblproperties: we can apply only the differences to an existing relation,
-        but if some other modified component requires us to completely replace the relation, we
-        should still be able to construct appropriate create clause from the object returned by
-        this method.
         """
 
         if self != other:
diff --git a/dbt/adapters/databricks/relation_configs/incremental.py b/dbt/adapters/databricks/relation_configs/incremental.py
index 5fb786c2..3af6c3e0 100644
--- a/dbt/adapters/databricks/relation_configs/incremental.py
+++ b/dbt/adapters/databricks/relation_configs/incremental.py
@@ -5,10 +5,11 @@ from dbt.adapters.databricks.relation_configs.base import DatabricksComponentCon
 from dbt.adapters.databricks.relation_configs.base import DatabricksRelationChangeSet
 from dbt.adapters.databricks.relation_configs.base import DatabricksRelationConfigBase
 from dbt.adapters.databricks.relation_configs.tags import TagsProcessor
+from dbt.adapters.databricks.relation_configs.tblproperties import TblPropertiesProcessor
 
 
 class IncrementalTableConfig(DatabricksRelationConfigBase):
-    config_components = [TagsProcessor]
+    config_components = [TagsProcessor, TblPropertiesProcessor]
 
     def get_changeset(
         self, existing: "IncrementalTableConfig"
diff --git a/dbt/adapters/databricks/relation_configs/tags.py b/dbt/adapters/databricks/relation_configs/tags.py
index 3a53728a..ddd35690 100644
--- a/dbt/adapters/databricks/relation_configs/tags.py
+++ b/dbt/adapters/databricks/relation_configs/tags.py
@@ -22,7 +22,9 @@ class TagsConfig(DatabricksComponentConfig):
         for k in other.set_tags.keys():
             if k not in self.set_tags:
                 to_unset.append(k)
-        return TagsConfig(set_tags=self.set_tags, unset_tags=to_unset)
+        if self.set_tags or to_unset:
+            return TagsConfig(set_tags=self.set_tags, unset_tags=to_unset)
+        return None
 
 
 class TagsProcessor(DatabricksComponentProcessor[TagsConfig]):
diff --git a/dbt/include/databricks/macros/materializations/incremental/incremental.sql b/dbt/include/databricks/macros/materializations/incremental/incremental.sql
index 5f76c48e..faa59b0a 100644
--- a/dbt/include/databricks/macros/materializations/incremental/incremental.sql
+++ b/dbt/include/databricks/macros/materializations/incremental/incremental.sql
@@ -38,6 +38,10 @@
     {%- endcall -%}
     {% do persist_constraints(target_relation, model) %}
     {% do apply_tags(target_relation, tags) %}
+    {%- if language == 'python' -%}
+      {%- do apply_tblproperties(target_relation, tblproperties) %}
+    {%- endif -%}
+
     {% do persist_docs(target_relation, model, for_relation=language=='python') %}
   {%- elif existing_relation.is_view or existing_relation.is_materialized_view or existing_relation.is_streaming_table or should_full_refresh() -%}
     {#-- Relation must be dropped & recreated --#}
@@ -96,17 +100,20 @@
     {%- endif -%}
     {% do apply_liquid_clustered_cols(target_relation) %}
     {% if _configuration_changes is not none %}
-      {% set tags = _configuration_changes.changes["tags"] %}
+      {% set tags = _configuration_changes.changes.get("tags", None) %}
+      {% set tblproperties = _configuration_changes.changes.get("tblproperties", None) %}
       {% if tags is not none %}
         {% do apply_tags(target_relation, tags.set_tags, tags.unset_tags) %}
       {%- endif -%}
+      {% if tblproperties is not none %}
+        {% do apply_tblproperties(target_relation, tblproperties.tblproperties) %}
+      {%- endif -%}
     {%- endif -%}
     {% do persist_docs(target_relation, model, for_relation=True) %}
   {%- endif -%}
 
   {% set should_revoke = should_revoke(existing_relation, full_refresh_mode) %}
   {% do apply_grants(target_relation, grant_config, should_revoke) %}
-  {% do apply_tblproperties_python(target_relation, tblproperties, language) %}
   {% do optimize(target_relation) %}
 
   {{ run_hooks(post_hooks) }}
diff --git a/dbt/include/databricks/macros/materializations/table.sql b/dbt/include/databricks/macros/materializations/table.sql
index 667de26a..d33c47ae 100644
--- a/dbt/include/databricks/macros/materializations/table.sql
+++ b/dbt/include/databricks/macros/materializations/table.sql
@@ -29,8 +29,9 @@
 
   {% set should_revoke = should_revoke(old_relation, full_refresh_mode=True) %}
   {% do apply_grants(target_relation, grant_config, should_revoke) %}
-  {% do apply_tblproperties_python(target_relation, tblproperties, language) %}
-
+  {% if language=="python" %}
+    {% do apply_tblproperties(target_relation, tblproperties) %}
+  {% endif %}
   {%- do apply_tags(target_relation, tags) -%}
 
   {% do persist_docs(target_relation, model, for_relation=language=='python') %}
diff --git a/dbt/include/databricks/macros/relations/tblproperties.sql b/dbt/include/databricks/macros/relations/tblproperties.sql
index 446e7ba9..6fa18574 100644
--- a/dbt/include/databricks/macros/relations/tblproperties.sql
+++ b/dbt/include/databricks/macros/relations/tblproperties.sql
@@ -13,10 +13,14 @@
   {%- endif %}
 {%- endmacro -%}
 
-{% macro apply_tblproperties_python(relation, tblproperties, language) -%}
-  {%- if tblproperties and language == 'python' -%}
-    {%- call statement('python_tblproperties') -%}
-      alter table {{ relation }} set {{ tblproperties_clause() }}
+{% macro apply_tblproperties(relation, tblproperties) -%}
+  {% if tblproperties %}
+    {%- call statement('apply_tblproperties') -%}
+      ALTER {{ relation.type }} {{ relation }} SET TBLPROPERTIES (
+      {% for tblproperty in tblproperties -%}
+        '{{ tblproperty }}' = '{{ tblproperties[tblproperty] }}' {%- if not loop.last %}, {% endif -%}
+      {%- endfor %}
+      )
     {%- endcall -%}
-  {%- endif -%}
-{%- endmacro -%}
\ No newline at end of file
+  {% endif %}
+{%- endmacro -%}


Test patch:
diff --git a/tests/functional/adapter/incremental/fixtures.py b/tests/functional/adapter/incremental/fixtures.py
index 6ab16ec0..78a65e11 100644
--- a/tests/functional/adapter/incremental/fixtures.py
+++ b/tests/functional/adapter/incremental/fixtures.py
@@ -76,6 +76,36 @@ models:
         - name: color
 """
 
+tblproperties_a = """
+version: 2
+
+models:
+  - name: merge_update_columns_sql
+    config:
+        tblproperties:
+            a: b
+            c: d
+    columns:
+        - name: id
+        - name: msg
+        - name: color
+"""
+
+tblproperties_b = """
+version: 2
+
+models:
+  - name: merge_update_columns_sql
+    config:
+        tblproperties:
+            c: e
+            d: f
+    columns:
+        - name: id
+        - name: msg
+        - name: color
+"""
+
 liquid_clustering_a = """
 version: 2
 
@@ -314,6 +344,28 @@ models:
       http_path: "{{ env_var('DBT_DATABRICKS_UC_CLUSTER_HTTP_PATH') }}"
 """
 
+python_tblproperties_schema = """version: 2
+models:
+  - name: tblproperties
+    config:
+      tags: ["python"]
+      tblproperties:
+        a: b
+        c: d
+      http_path: "{{ env_var('DBT_DATABRICKS_UC_CLUSTER_HTTP_PATH') }}"
+"""
+
+python_tblproperties_schema2 = """version: 2
+models:
+  - name: tblproperties
+    config:
+      tags: ["python"]
+      tblproperties:
+        c: e
+        d: f
+      http_path: "{{ env_var('DBT_DATABRICKS_UC_CLUSTER_HTTP_PATH') }}"
+"""
+
 lc_python_schema = """version: 2
 models:
   - name: simple_python_model
diff --git a/tests/functional/adapter/incremental/test_incremental_tblproperties.py b/tests/functional/adapter/incremental/test_incremental_tblproperties.py
new file mode 100644
index 00000000..22625a9b
--- /dev/null
+++ b/tests/functional/adapter/incremental/test_incremental_tblproperties.py
@@ -0,0 +1,51 @@
+import pytest
+from dbt.tests import util
+from tests.functional.adapter.incremental import fixtures
+
+
+class TestIncrementalTblproperties:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "merge_update_columns_sql.sql": fixtures.merge_update_columns_sql,
+            "schema.yml": fixtures.tblproperties_a,
+        }
+
+    # For now, only add/change, no deletes
+    def test_changing_tblproperties(self, project):
+        util.run_dbt(["run"])
+        util.write_file(fixtures.tblproperties_b, "models", "schema.yml")
+        util.run_dbt(["run"])
+        results = project.run_sql(
+            "show tblproperties {database}.{schema}.merge_update_columns_sql",
+            fetch="all",
+        )
+        results_dict = {}
+        for result in results:
+            results_dict[result.key] = result.value
+        assert results_dict["c"] == "e"
+        assert results_dict["d"] == "f"
+
+
+@pytest.mark.skip_profile("databricks_cluster")
+class TestIncrementalPythonTblproperties:
+    @pytest.fixture(scope="class")
+    def models(self):
+        return {
+            "tblproperties.py": fixtures.simple_python_model,
+            "schema.yml": fixtures.python_tblproperties_schema,
+        }
+
+    def test_changing_tblproperties(self, project):
+        util.run_dbt(["run"])
+        util.write_file(fixtures.python_tblproperties_schema2, "models", "schema.yml")
+        util.run_dbt(["run"])
+        results = project.run_sql(
+            "show tblproperties {database}.{schema}.tblproperties",
+            fetch="all",
+        )
+        results_dict = {}
+        for result in results:
+            results_dict[result.key] = result.value
+        assert results_dict["c"] == "e"
+        assert results_dict["d"] == "f"
diff --git a/tests/unit/relation_configs/test_incremental_config.py b/tests/unit/relation_configs/test_incremental_config.py
index ec919b08..d52f8ac3 100644
--- a/tests/unit/relation_configs/test_incremental_config.py
+++ b/tests/unit/relation_configs/test_incremental_config.py
@@ -2,6 +2,7 @@ from agate import Table
 
 from dbt.adapters.databricks.relation_configs.incremental import IncrementalTableConfig
 from dbt.adapters.databricks.relation_configs.tags import TagsConfig
+from dbt.adapters.databricks.relation_configs.tblproperties import TblPropertiesConfig
 
 
 class TestIncrementalConfig:
@@ -13,7 +14,8 @@ class TestIncrementalConfig:
                     ["tag2", "value2"],
                 ],
                 column_names=["tag_name", "tag_value"],
-            )
+            ),
+            "show_tblproperties": Table(rows=[["prop", "f1"]], column_names=["key", "value"]),
         }
 
         config = IncrementalTableConfig.from_results(results)
@@ -21,5 +23,6 @@ class TestIncrementalConfig:
         assert config == IncrementalTableConfig(
             config={
                 "tags": TagsConfig(set_tags={"tag1": "value1", "tag2": "value2"}),
+                "tblproperties": TblPropertiesConfig(tblproperties={"prop": "f1"}),
             }
         )