Skip to content
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

fix: replicated database materializations #407

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

canbekley
Copy link
Contributor

Summary

Previously, table and view materializations wouldn't work with a Replicated database configured, as they would run ON CLUSTER statements for some of the ddl's. This is not allowed in replicated databases and would lead to the following error:

DB::Exception: It's not initial query. ON CLUSTER is not allowed for Replicated database. (INCORRECT_QUERY) (version 24.11.1.2557 (official build)

This mr fixes these issues by adding conditions for on cluster statements with replicated databases.

Checklist

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG

@canbekley canbekley changed the title Fix/replicated database materializations fix: replicated database materializations Jan 13, 2025
@@ -59,14 +59,13 @@

{% macro clickhouse__drop_relation(relation, obj_type='table') -%}
{% call statement('drop_relation', auto_begin=False) -%}
{# drop relation on cluster by default if cluster is set #}
drop {{ obj_type }} if exists {{ relation }} {{ on_cluster_clause(relation.without_identifier(), True)}}
drop {{ obj_type }} if exists {{ relation }} {{ on_cluster_clause(relation, True)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you elaborate more on why to include back the identifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these macros are used to drop/rename tables and other relation types. If the relation doesn't contain an identifier, the ClickHouseRelation.should_on_cluster property will always return True, skipping a check on can_on_cluster. this was causing issues for tables in replicated databases, as they don't support on cluster statements.

{%- endcall %}
{% endmacro %}

{% macro clickhouse__rename_relation(from_relation, to_relation, obj_type='table') -%}
{% call statement('drop_relation') %}
drop {{ obj_type }} if exists {{ to_relation }} {{ on_cluster_clause(to_relation.without_identifier())}}
drop {{ obj_type }} if exists {{ to_relation }} {{ on_cluster_clause(to_relation)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here

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