dbt-labs / dbt-labs/dbt-adapters
[CT-3409] rename_relation macro should call get_rename_sql instead of defaulting to alter table
- Dominant language
- Python
- Stars
- 233
- Forks
- 362
- Avg merge
- 3d 22h
- Merged PRs (30d)
- 9
Description
### Housekeeping
- [ ] I am a maintainer of dbt-core
### Short description
_Sorry, could not really find a good match in the issue templates for this._
The issue is about [this line](https://github.com/dbt-labs/dbt-core/blob/1c9cec17878ca0fec537a4cc13812f91ee97b29b/core/dbt/include/global_project/macros/relations/rename.sql#L33C1-L34C1) in the `default__rename_relation` macro.
The name of the macro implies that it would be able to rename a relation, but in fact, it's only able to rename tables as it's calling `alter table`.
We do have a better version of this in the codebase already, namely the `get_rename_sql` macro.
[That version](https://github.com/dbt-labs/dbt-core/blob/1c9cec17878ca0fec537a4cc13812f91ee97b29b/core/dbt/include/global_project/macros/relations/rename.sql#L7) is much more extensive and is capable of renaming all supported relations.
My proposal would be to forward the `alter table` call in `rename_relation` to `get_rename_sql`, so that we end up with something like this:
```sql+jinja
{% macro default__rename_relation(from_relation, to_relation) -%}
{% set target_name = adapter.quote_as_configured(to_relation.identifier, 'identifier') %}
{{ log('from_relation: ' ~ from_relation, info=True) }}
{% call statement('rename_relation') -%}
{{ get_rename_sql(from_relation, target_name) }}
{%- endcall %}
{% endmacro %}
```
Right now, adapter maintainers who'd like to simply rename relations without having to deal with the hassle of calling `get_rename_sql` directly, have to create such an override in their own adapter. That is a bit tedious and probably not intended.
### Acceptance criteria
- All current tests still succeed
### Impact to Other Teams
I don't foresee any impact except that adapter maintainers can drop their custom override. The original behavior where the macro calls `alter table` in the end will still work. The difference in behavior would be that it no longer calls `alter table` for views, but `alter view` instead and likewise for materialized views.
### Will backports be required?
no
### Context
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.