Repository: databricks/dbt-databricks
Pull request #1079 (2025-07-01 15:44:09)

Add cloning for the external tables
<!-- Please review our pull request review process in CONTRIBUTING.md before your proceed. -->

### Description

This PR adds the ability to clone the external tables to the package. Without that, the `dbt clone` command fails as Databricks requires providing the location of the target if we are cloning the external table.

To achieve that, I've introduced the `is_external_table` property of the `DatabricksRelation` that provides the information if the table is external. Then, I've added the new macro named `databricks__create_or_replace_clone_external` that handles the logic of the external cloning and uses the `location_clause` macro to determine where to perform the clone based on the target table configs.

The decision which macro to choose is happening in the `clone` macro itself: if the `is_external_table` attribute of the relation returns True, we call the `_external` macro. If not, we perform the call of the already existing cloning macro.

The second macro was added based on the security considerations: I wouldn't want to break the existing cloning procedures in my release, thus I prefer to have completely separate branch for the new functionality which is more or less independent of already existing code.

### Checklist

- [ ] I have run this code in development and it appears to resolve the stated issue
- [ ] 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.


mi-volodin: Also would add a minor question - on which version (branch) should we rebase the change? 
benc-db: > Also would add a minor question - on which version (branch) should we rebase the change? If the change is feasible and would be welcomed, of course.

Branching from main is correct, and looking at the PR, this seems like something we are happy to take.  My understanding is that basically if a table is asked to be cloned, we have been limited previously for external tables because the syntax for the clone requires the location; this PR detects if the source is external and modifies the syntax to accommodate it, right?

> Should I do something with them? Do you have any suggestions on how can I ensure that this change is robust and works?

The problem with external locations is that a.) Databricks doesn't clean up the files because they are treated as unmanaged (i.e. dropping a table does not delete the files backing the table), and b.) the process of clean up outside of Databricks might be a function of what cloud the files are written in.  This makes it hard to write portable tests that don't leave artifacts lying around.

If possible, can you look at the macro unit tests and write a unit test that validates that the macro output looks exactly like what you're expecting?  You could write an E2E test as well, but it will almost certainly leave files lying around.
samgans: Understood, will write the macro test. Thanks for the input!
samgans: Hey @benc-db, I've created the tests for this functionality, but to be able to do this I needed to introduce some more changes:

1. The `clone.sql` file now only the helper function `databricks__can_clone_table` and the definition of the materialization.
2. The clone strategies are moved to separate `strategies.sql` file.
3. Everything related to the clone is moved to separate `clone/` directory inside `materializations/`.

I needed to do this to get rid of the issue that pops ups when trying to test the macro defined in the same file where `materialization` tag present: `tag materialization doesn't exist`.

I've taken the approach which I've found with the `incremental` materialization and replicated this. Hope this is acceptable.

Additionally, I've introduced some small changes to the unit tests:

1. Moved `unity_relation` function to the `utils.py` as it is currently reused by two unit tests.
2. Adapted `test_table_macros` to the usage of the common `unity_relation`.

And to the clone macros themselves:

1. It seems to be expected to pass the CatalogRelation to the `location_clause`, which is now generated in the `databricks__create_or_replace_clone_external`.
2. We call `render()` function of the relations to render their names instead of their implicit passing to be able to test this (I've replicated this from the `insert` macros, so not sure if this is something which is expected to be done).

And finally, I've added the `test_clone_macros.py` code which replicates the logic of `test_table_macros`, but tests the cloning itself.

Open for the feedback.
samgans: Hm, I see that functional tests related to Streaming Tables are failing. Do you think it is related to any of my changes? I can't really assess, it seems like no, but I'm not too deep into the code base.
samgans: Hm I'm wondering if this is related to this change somehow:

https://github.com/databricks/dbt-databricks/pull/1082/files

I'm not sure if I need to merge the main branch into mine.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d055227d..9607c304 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@
 
 - Support column tags (with Materialization V2) ([649](https://github.com/databricks/dbt-databricks/issues/649))
 - Support column masking (with Materialization V2) ([670](https://github.com/databricks/dbt-databricks/issues/670))
+- Add cloning support for the the external tables (thanks @samgans!) ([1079](https://github.com/databricks/dbt-databricks/pull/1079))
 
 ### Fixes
 
diff --git a/dbt/adapters/databricks/relation.py b/dbt/adapters/databricks/relation.py
index f7152b47..7b384948 100644
--- a/dbt/adapters/databricks/relation.py
+++ b/dbt/adapters/databricks/relation.py
@@ -93,6 +93,10 @@ class DatabricksRelation(BaseRelation):
     def is_streaming_table(self) -> bool:
         return self.type == DatabricksRelationType.StreamingTable
 
+    @property
+    def is_external_table(self) -> bool:
+        return self.type == DatabricksRelationType.External
+
     @property
     def is_dlt(self) -> bool:
         return self.is_materialized_view or self.is_streaming_table
diff --git a/dbt/include/databricks/macros/materializations/clone.sql b/dbt/include/databricks/macros/materializations/clone/clone.sql
similarity index 86%
rename from dbt/include/databricks/macros/materializations/clone.sql
rename to dbt/include/databricks/macros/materializations/clone/clone.sql
index 74cb11fd..229c382f 100644
--- a/dbt/include/databricks/macros/materializations/clone.sql
+++ b/dbt/include/databricks/macros/materializations/clone/clone.sql
@@ -2,11 +2,6 @@
     {{ return(True) }}
 {% endmacro %}
 
-{% macro databricks__create_or_replace_clone(this_relation, defer_relation) %}
-    create or replace
-    table {{ this_relation }}
-    shallow clone {{ defer_relation }}
-{% endmacro %}
 
 {%- materialization clone, adapter='databricks' -%}
 
@@ -43,9 +38,15 @@
       {% endif %}
 
       -- as a general rule, data platforms that can clone tables can also do atomic 'create or replace'
-      {% call statement('main') %}
-          {{ create_or_replace_clone(target_relation, defer_relation) }}
-      {% endcall %}
+      {% if other_existing_relation.is_external %}
+          {% call statement('main') %}
+              {{ create_or_replace_clone_external(target_relation, defer_relation) }}
+          {% endcall %}
+      {% else %}
+          {% call statement('main') %}
+              {{ create_or_replace_clone(target_relation, defer_relation) }}
+          {% endcall %}
+      {% endif %}
 
       {% set should_revoke = should_revoke(existing_relation, full_refresh_mode=True) %}
       {% do apply_grants(target_relation, grant_config, should_revoke=should_revoke) %}
diff --git a/dbt/include/databricks/macros/materializations/clone/strategies.sql b/dbt/include/databricks/macros/materializations/clone/strategies.sql
new file mode 100644
index 00000000..2202df28
--- /dev/null
+++ b/dbt/include/databricks/macros/materializations/clone/strategies.sql
@@ -0,0 +1,14 @@
+{% macro databricks__create_or_replace_clone(this_relation, defer_relation) %}
+    create or replace
+    table {{ this_relation.render() }}
+    shallow clone {{ defer_relation.render() }}
+{% endmacro %}
+
+{% macro databricks__create_or_replace_clone_external(this_relation, defer_relation) %}
+    {%- set catalog_relation = adapter.build_catalog_relation(config.model) -%}
+
+    create or replace
+    table {{ this_relation.render() }}
+    shallow clone {{ defer_relation.render() }}
+    {{ location_clause(catalog_relation) }}
+{% endmacro %}


Test patch:
diff --git a/tests/unit/macros/materializations/test_clone_macros.py b/tests/unit/macros/materializations/test_clone_macros.py
new file mode 100644
index 00000000..493173af
--- /dev/null
+++ b/tests/unit/macros/materializations/test_clone_macros.py
@@ -0,0 +1,88 @@
+from unittest.mock import Mock
+
+import pytest
+
+from dbt.adapters.databricks import constants
+from tests.unit.macros.base import MacroTestBase
+from tests.unit.utils import unity_relation
+
+
+class TestCloneStrategies(MacroTestBase):
+    t_location_root = "/mnt/root_dev/"
+
+    def render_clone_macro(self, template_bundle, macro, s_relation, t_relation) -> str:
+        external_path = f"{self.t_location_root}{template_bundle.relation.identifier}"
+        adapter_mock = template_bundle.template.globals["adapter"]
+        adapter_mock.compute_external_path.return_value = external_path
+        return self.run_macro(
+            template_bundle.template,
+            macro,
+            t_relation,
+            s_relation,
+        )
+
+    @pytest.fixture(scope="class")
+    def template_name(self) -> str:
+        return "strategies.sql"
+
+    @pytest.fixture(scope="class")
+    def macro_folders_to_load(self) -> list:
+        return ["macros/materializations/clone", "macros/relations", "macros"]
+
+    @pytest.fixture(scope="class")
+    def databricks_template_names(self) -> list:
+        return ["location.sql"]
+
+    @pytest.fixture
+    def s_relation(self):
+        relation = Mock()
+        relation.database = "some_database"
+        relation.schema = "some_schema_prod"
+        relation.identifier = "some_table"
+        relation.render = Mock(return_value="`some_database`.`some_schema_prod`.`some_table`")
+        relation.without_identifier = Mock(return_value="`some_database`.`some_schema_prod`")
+        relation.type = "table"
+        return relation
+
+    @pytest.fixture
+    def catalog_relation(self, template_bundle):
+        t_relation = unity_relation(
+            file_format=constants.DELTA_FILE_FORMAT,
+            location_root=self.t_location_root,
+            location_path=template_bundle.relation.identifier,
+        )
+        template_bundle.context["adapter"].build_catalog_relation.return_value = t_relation
+        return t_relation
+
+    def test_create_or_replace_clone(self, template_bundle, s_relation):
+        sql = self.render_clone_macro(
+            template_bundle,
+            "databricks__create_or_replace_clone",
+            s_relation,
+            template_bundle.relation,
+        )
+
+        expected = self.clean_sql(
+            "create or replace "
+            f"table {template_bundle.relation.render()} "
+            f"shallow clone {s_relation.render()}"
+        )
+
+        assert expected == sql
+
+    def test_create_or_replace_clone_external(self, template_bundle, catalog_relation, s_relation):
+        sql = self.render_clone_macro(
+            template_bundle,
+            "databricks__create_or_replace_clone_external",
+            s_relation,
+            template_bundle.relation,
+        )
+
+        expected = self.clean_sql(
+            "create or replace "
+            f"table {template_bundle.relation.render()} "
+            f"shallow clone {s_relation.render()} "
+            f"location '{catalog_relation.location}'"
+        )
+
+        assert expected == sql
diff --git a/tests/unit/macros/relations/test_table_macros.py b/tests/unit/macros/relations/test_table_macros.py
index 5a37960e..1a6c372f 100644
--- a/tests/unit/macros/relations/test_table_macros.py
+++ b/tests/unit/macros/relations/test_table_macros.py
@@ -1,26 +1,8 @@
-from typing import Optional
-
 import pytest
 
-from dbt.adapters.databricks import catalogs, constants
+from dbt.adapters.databricks import constants
 from tests.unit.macros.base import MacroTestBase
-
-
-def unity_relation(
-    table_format: Optional[str] = None,
-    file_format: Optional[str] = None,
-    location_root: Optional[str] = None,
-    location_path: Optional[str] = None,
-) -> catalogs.DatabricksCatalogRelation:
-    catalog_integration = constants.DEFAULT_UNITY_CATALOG
-    return catalogs.DatabricksCatalogRelation(
-        catalog_type=catalog_integration.catalog_type,
-        catalog_name=catalog_integration.catalog_name,
-        table_format=table_format or catalog_integration.table_format,
-        file_format=file_format or catalog_integration.file_format,
-        external_volume=location_root or catalog_integration.external_volume,
-        location_path=location_path,
-    )
+from tests.unit.utils import unity_relation
 
 
 class TestCreateTableAs(MacroTestBase):
diff --git a/tests/unit/utils.py b/tests/unit/utils.py
index 6fdf86cd..164ed24c 100644
--- a/tests/unit/utils.py
+++ b/tests/unit/utils.py
@@ -1,6 +1,8 @@
 import os
 from copy import deepcopy
+from typing import Optional
 
+from dbt.adapters.databricks import catalogs, constants
 from dbt.config import Profile, Project, RuntimeConfig
 from dbt.config.project import PartialProject
 from dbt.config.renderer import DbtProjectYamlRenderer, ProfileRenderer
@@ -78,3 +80,21 @@ def config_from_parts_or_dicts(project, profile, packages=None, selectors=None,
     args.vars = cli_vars
     args.profile_dir = "/dev/null"
     return RuntimeConfig.from_parts(project=project, profile=profile, args=args)
+
+
+def unity_relation(
+    table_format: Optional[str] = None,
+    file_format: Optional[str] = None,
+    location_root: Optional[str] = None,
+    location_path: Optional[str] = None,
+) -> catalogs.DatabricksCatalogRelation:
+    catalog_integration = constants.DEFAULT_UNITY_CATALOG
+
+    return catalogs.DatabricksCatalogRelation(
+        catalog_type=catalog_integration.catalog_type,
+        catalog_name=catalog_integration.catalog_name,
+        table_format=table_format or catalog_integration.table_format,
+        file_format=file_format or catalog_integration.file_format,
+        external_volume=location_root or catalog_integration.external_volume,
+        location_path=location_path,
+    )