Skip to content

[FIX]: Fix issue with cloning of external tables #1095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 12, 2025

Conversation

samgans
Copy link
Contributor

@samgans samgans commented Jul 10, 2025

Description

This PR resolves the bugs related to the external cloning introduced in #1079 (sorry for introducing them 🥲).

What are these bugs?

The cloning of the external tables still doesn't work, because the {% if other_existing_relation.is_external %} branch isn't followed due to multiple issues:

  1. The property of the relation is named is_external_table instead of is_external. This is the typo that was introduced by me.
  2. The property is_external_table returns False for every external table due to the fact that the get_uc_tables_sql macro casts all the tables that are of 'EXTERNAL', 'MANAGED', 'MANAGED_SHALLOW_CLONE', 'EXTERNAL_SHALLOW_CLONE' types to the TABLE type thus making any external-related DatabricksRelationType not available as type attribute of the DatabricksRelation.

In addition to that, the databricks__create_or_replace_clone_external macro was not available for the usage due to the dispatch syntax used in its declaration. Instead, we should have declared it as create_or_replace_clone_external.

Bug 1

To fix the bug 1, I've simply fixed the call of the function.

File 1.

Bug 2

To fix the bug 2, I've changed the get_uc_tables_sql macro to also return an attribute named databricks_table_type which additionally classifies the databricks table. To make this work, I've created another enumeration DatabricksTableType which means the specific databricks table type and made this as a databricks_table_type attribute of the DatabricksRelation. The Relation uses this attribute to determine if it is an external table or not.

File 1.

Adding databricks_table_type attribute instead of simply updating the type attribute was done to keep the backwards compatibility of the attribute and not break potential assumptions about this attribute.

File 2.

Introducing the new attribute returned from the macro has also required me to fix the type definitions of the return rows, thus I've introduced the new named tuple DatabricksRelationInfo so that the return values of the query that describes the relation are better documented, at the same time keeping the backwards-compatible type.

File 3.

Bug 3

The dispatch of the external macro was fixed by removing the dispatch-based syntax in its definition.

File 1.

Testing

We've found a way to test these changes in our workspace, so we were able to run the clone operation successfully using the external tables:

image

image

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
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-databricks next" section.

@samgans samgans force-pushed the fix_issue_with_external_tables branch from d8d846d to 46dbcd5 Compare July 10, 2025 09:56
samgans added 7 commits July 10, 2025 11:59
Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
… branch

Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
@samgans samgans force-pushed the fix_issue_with_external_tables branch from 46dbcd5 to 5239635 Compare July 10, 2025 10:00
@samgans samgans marked this pull request as ready for review July 10, 2025 10:18
@samgans samgans requested review from benc-db and ericj-db as code owners July 10, 2025 10:18
@samgans
Copy link
Contributor Author

samgans commented Jul 10, 2025

@ericj-db @benc-db I see that ruff-format is failed for one file. I've fixed the issue and triggered pre-commit again: seems to be working.

@samgans
Copy link
Contributor Author

samgans commented Jul 10, 2025

image

@samgans
Copy link
Contributor Author

samgans commented Jul 10, 2025

Code quality run:

image

Signed-off-by: Anton Zhyltsou <antonzhiltsov2000@gmail.com>
@samgans samgans force-pushed the fix_issue_with_external_tables branch from 0f76ed8 to f379790 Compare July 10, 2025 19:27
@samgans
Copy link
Contributor Author

samgans commented Jul 10, 2025

Signed the last commit.

@ericj-db ericj-db mentioned this pull request Jul 10, 2025
3 tasks
Copy link
Collaborator

@ericj-db ericj-db left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@ericj-db ericj-db merged commit c59637e into databricks:main Jul 12, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants