-
Notifications
You must be signed in to change notification settings - Fork 112
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
(feat): add support for matview #123
Conversation
Thanks for your PR! It would be extremely helpful if you could add a test for the new Materialized View functionality; if you don't have a chance to that I'll try to get to it later this week. |
@genzgd sure would be great if you can add tests or I can try to get to it when I have time. I'm already using this feature on a local branch for a project and it works well. |
Thanks -- at the moment the PR is failing a test and I haven't had the chance to dig into it. If you any thoughts as to why that failure is happening that would be helpful. In any case I'll try to get to it this coming week. |
@genzgd The test fails with this error: Table dbt_clickhouse_4727_test_basic_1672594971825.swappable already exists. (TABLE_ALREADY_EXISTS) I don't think my PR should cause that failure. No new code is actually running unless {% if matview %}
{{ clickhouse__create_matview_as(target_relation, sql) }}
{% else %}
{{ get_create_view_as_sql(target_relation, sql) }}
{% endif %} And tests aren't using |
I felt the same way :) but I haven't had a chance to figure out the problem. There are no code changes since tests last successfully ran against the |
{% if matview %} | ||
{{ clickhouse__create_matview_as(target_relation, sql) }} | ||
{% else %} | ||
{{ get_create_view_as_sql(target_relation, sql) }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is what's breaking tests -- the original references backup_relation
but the new line is target_relations
Hey! Any updates on this PR? |
1 similar comment
Hey! Any updates on this PR? |
The short answer is there are no updates. The broken test was not fixed and the ClickHouse team hasn't prioritized this functionality in dbt due to other work. There is also concern about how materialized views will work on replicated tables and they are almost certain to break dbt incremental materializations (DELETEs, for example, are not propagated to materialized views in ClickHouse), so official support would require testing around these use cases. |
Closed in favor of #207 |
Introduction
Supports a
target_table
(TO table
) for Materialized View, or it'd use its own table, which is fully configurable: engine, order by, partition by, etc... defaults toMergeTree() order by tuple()
.Usage
A model to define a target table:
A mode to define a materialized view: