Skip to content

Fix: macro that persists docs for Replicated cluster setups #494

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.

Sign up for GitHub

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

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

Fix: macro that persists docs for Replicated cluster setups #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
555886a
Add failing replicated persist docs test
emirkmo Aug 14, 2025
bf4fe20
fix: Split persist docs into column and table comments
emirkmo Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions dbt/include/clickhouse/macros/persist_docs.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,45 @@
alter table {{ relation }} {{ on_cluster_clause(relation) }} modify comment '{{ comment }}'
{% endmacro %}

{% macro clickhouse__persist_docs(relation, model, for_relation, for_columns) %}
{%- set alter_comments = [] %}

{% macro clickhouse__persist_docs(relation, model, for_relation, for_columns) -%}
{# Persist table comment if enabled and description provided #}
{%- if for_relation and config.persist_relation_docs() and model.description -%}
{% set escaped_comment = clickhouse_escape_comment(model.description) %}
{% do alter_comments.append("modify comment {comment}".format(comment=escaped_comment)) %}
{{ _persist_table_comment(relation, model.description) }}
{%- endif -%}

{#- Persist column comments if enabled and columns defined -#}
{%- if for_columns and config.persist_column_docs() and model.columns -%}
{% set existing_columns = adapter.get_columns_in_relation(relation) | map(attribute="name") | list %}
{% for column_name in model.columns if (column_name in existing_columns) %}
{%- set comment = model.columns[column_name]['description'] -%}
{%- if comment %}
{% set escaped_comment = clickhouse_escape_comment(comment) %}
{% do alter_comments.append("comment column `{column_name}` {comment}".format(column_name=column_name, comment=escaped_comment)) %}
{%- endif %}
{%- endfor -%}
{{ _persist_column_comments(relation, model.columns) }}
{%- endif -%}
{%- endmacro %}

{#- Helper macro: persist the table comment for a ClickHouse relation. -#}
{% macro _persist_table_comment(relation, description) -%}
{#- Escape the description to be safe for ClickHouse #}
{%- set escaped_comment = clickhouse_escape_comment(description) -%}
{#- Build and run the ALTER TABLE ... MODIFY COMMENT statement -#}
{%- set sql = "modify comment {comment}".format(comment=escaped_comment) -%}
{{ run_query(one_alter_relation(relation, sql)) }}
{%- endmacro %}

{%- if alter_comments | length > 0 -%}
{% do run_query(one_alter_relation(relation, alter_comments|join(', '))) %}
{#- Helper macro: persist comments for multiple columns on a ClickHouse table.
relation: target table relation
columns: dict mapping column names to metadata (including 'description') -#}
{% macro _persist_column_comments(relation, columns) -%}
{#- Gather existing columns in the relation to avoid altering non-existent ones -#}
{%- set existing_columns = adapter.get_columns_in_relation(relation) | map(attribute="name") | list -%}
{#- Collect ALTER statements for each column with a description -#}
{%- set alterations = [] -%}
{%- for column_name, info in columns.items() if info.description and column_name in existing_columns -%}
{%- set escaped_comment = clickhouse_escape_comment(info.description) -%}
{%- do alterations.append("\ncomment column `{column_name}` {comment}".format(column_name=column_name, comment=escaped_comment)) -%}
{%- endfor -%}
{#- Execute a single ALTER TABLE statement for all column comments -#}
{%- if alterations -%}
{{ run_query(one_alter_relation(relation, alterations | join(", "))) }}
{%- endif -%}
{% endmacro %}
{%- endmacro %}


{#
By using dollar-quoting like this, users can embed anything they want into their comments
Expand Down
25 changes: 24 additions & 1 deletion tests/integration/adapter/clickhouse/test_clickhouse_comments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,21 @@

"""

ref_models__replicated_table_comment_sql = """
{{
config(
materialized = "table",
persist_docs = {"relation": true, "columns": true},
engine="ReplicatedMergeTree('/clickhouse/tables/{uuid}/one-shard', '{replica}' )"
)
}}

select
'foo' as first_name,
'bar' as second_name

"""
Copy link
Author

@emirkmo emirkmo Sep 6, 2025

Choose a reason for hiding this comment

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

This test fails without the changed macro, when running against test_replica cluster in the testing setup.

Copy link
Author

@emirkmo emirkmo Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The failure message shows that the comment isn't there :)

______________________________________________________________________ TestBaseComment.test_comment[replicated_table_comment] ______________________________________________________________________

self = <test_clickhouse_comments.TestBaseComment object at 0x7160e0b64770>, project = <dbt.tests.fixtures.project.TestProjInfo object at 0x7160dfd9d4c0>, model_name = 'replicated_table_comment'

    @pytest.mark.parametrize(
        'model_name',
        ["table_comment", "replicated_table_comment", "view_comment"],
    )
    def test_comment(self, project, model_name):
        if os.environ.get('DBT_CH_TEST_CLOUD', '').lower() in ('1', 'true', 'yes'):
            pytest.skip('Not running comment test for cloud')
        run_dbt(["run"])
        run_dbt(["docs", "generate"])
        with open("target/catalog.json") as fp:
            catalog_data = json.load(fp)
    
        assert "nodes" in catalog_data
        column_node = catalog_data["nodes"][f"model.test.{model_name}"]
        for column in column_node["columns"].keys():
            column_comment = column_node["columns"][column]["comment"]
>           assert column_comment.startswith("XXX")
                   ^^^^^^^^^^^^^^^^^^^^^^^^^
E           AttributeError: 'NoneType' object has no attribute 'startswith'

/home/emirk/Software/dbt-clickhouse/tests/integration/adapter/clickhouse/test_clickhouse_comments.py:104: AttributeError

Copy link
Author

@emirkmo emirkmo Nov 12, 2025

Choose a reason for hiding this comment

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

The test passes after the next commit (the new macro in this PR).


ref_models__view_comment_sql = """
{{
config(
Expand All @@ -43,6 +58,13 @@
description: "XXX first description"
- name: second_name
description: "XXX second description"
- name: replicated_table_comment
description: "YYY table"
columns:
- name: first_name
description: "XXX first description"
- name: second_name
description: "XXX second description"
- name: view_comment
description: "YYY view"
columns:
Expand All @@ -59,12 +81,13 @@ def models(self):
return {
"schema.yml": ref_models__schema_yml,
"table_comment.sql": ref_models__table_comment_sql,
"replicated_table_comment.sql": ref_models__replicated_table_comment_sql,
"view_comment.sql": ref_models__view_comment_sql,
}

@pytest.mark.parametrize(
'model_name',
['table_comment', 'view_comment'],
["table_comment", "replicated_table_comment", "view_comment"],
)
def test_comment(self, project, model_name):
if os.environ.get('DBT_CH_TEST_CLOUD', '').lower() in ('1', 'true', 'yes'):
Expand Down