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

Implement persist_docs #134

Open
telenieko opened this issue May 9, 2021 · 9 comments · May be fixed by #289
Open

Implement persist_docs #134

telenieko opened this issue May 9, 2021 · 9 comments · May be fixed by #289
Assignees
Milestone

Comments

@telenieko
Copy link

dbt has a configuration option to persist documentation to the database:
https://docs.getdbt.com/reference/resource-configs/persist_docs

Attempting to use it with the sql server adapter results in:

ERROR: alter_relation_comment macro not implemented for adapter sqlserver
ERROR: alter_column_comment macro not implemented for adapter sqlserver

Feature Request: implement the above

@dataders
Copy link
Collaborator

I agree that this feature would be cool, but how would one use it? This example using Extended Properties seems helpful, but how would a user of the database see this info in SSMS or Power BI?

I'm an Extended Properties newb, but the persist_docs feature makes a lot more sense for BigQuery and Snowflake where it's easier to see the docs.

@telenieko
Copy link
Author

I agree that this feature would be cool, but how would one use it? This example using Extended Properties seems helpful, but how would a user of the database see this info in SSMS or Power BI?

I'm an Extended Properties newb, but the persist_docs feature makes a lot more sense for BigQuery and Snowflake where it's easier to see the docs.

TBH I have no idea how one would use it.

As far as I know Extended Properties is the recomended way to handle in-database documentation. So it would be safe to assume that user tooling (SSMS, PowerBI, ...) will at some point do proper use of that documentation.

Maybe the way to make those tools do proper use of Extended Properties is to first put something in there so people can start complaining they cannot use their documentation on the tools 😂.

So, focusing this issue, maybe: Step 1 is to see wether Extended Properties is the place to persist_docs, and Step 2 would be to make it happen. Step 3 would be to pester MSFT & other tools to add proper UI for using those docs?

@jeroen-mostert
Copy link

jeroen-mostert commented Jun 25, 2021

There is a semi-standard in the form of the MS_Description extended property. This is what SSMS surfaces for columns if you use the table designer, and it surfaces it for tables and columns if you use the Diagram Designer and show the properties. Power BI appears to have no support, at least it doesn't show anything if you're browsing tables in the import. I have no idea how widespread support for MS_Description is among third-party tools, but as a pick for something to use for persisting a description it's certainly better than nothing.

@mahazza
Copy link

mahazza commented Nov 18, 2021

I had a need for this feature using MS_Description. A few of our tools use it as default to pull out descriptions such as redgate tools, azure purview and I assume a few others. It may be useful to make MS_Description configurable if there is a need. I've attached what is working for us...

{% macro sqlserver__alter_column_comment(relation, column_dict) -%}
    {%- set existing_columns = adapter.get_columns_in_relation(relation) | map(attribute="name") | list %}
    {%- for column_name in column_dict if (column_name in existing_columns) %}
        {%- do log('Alter extended property "MS_Description" to "' ~ column_dict[column_name]['description'] ~ '" for ' ~ relation ~ ' column "' ~ column_name ~ '"', info=false) %}
        IF NOT EXISTS (
            SELECT NULL 
            FROM SYS.EXTENDED_PROPERTIES AS ep

                INNER JOIN SYS.ALL_COLUMNS AS cols
                    ON cols.object_id = ep.major_id
                    AND cols.column_id = ep.minor_id

            WHERE   ep.major_id = OBJECT_ID('{{ relation }}') 
            AND     ep.name = N'MS_Description'
            AND		cols.name = N'{{ column_name }}'
        )
            EXECUTE sp_addextendedproperty      @name = N'MS_Description', @value = N'{{ column_dict[column_name]['description'] }}'
                                                , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                                , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}'
                                                , @level2type = N'COLUMN', @level2name = N'{{ column_name }}';
        ELSE
            EXECUTE sp_updateextendedproperty   @name = N'MS_Description', @value = N'{{ column_dict[column_name]['description'] }}'
                                                , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                                , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}'
                                                , @level2type = N'COLUMN', @level2name = N'{{ column_name }}';
    {%- endfor %}
{%- endmacro %}

{% macro sqlserver__alter_relation_comment(relation, relation_comment) -%}
   {% do log('Alter extended property "MS_Description" to "' ~ relation_comment ~ '" for ' ~ relation, info=false) %}

    IF NOT EXISTS (
        SELECT NULL 
        FROM SYS.EXTENDED_PROPERTIES AS ep
        WHERE   ep.major_id = OBJECT_ID('{{ relation }}')
        AND     ep.name = N'MS_Description'
        AND     ep.minor_id = 0
    )
        EXECUTE sp_addextendedproperty      @name = N'MS_Description', @value = N'{{ relation_comment }}'
                                            , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                            , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}';
    ELSE
        EXECUTE sp_updateextendedproperty   @name = N'MS_Description', @value = N'{{ relation_comment }}'
                                            , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                            , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}';
{% endmacro %}



@dataders
Copy link
Collaborator

I had a need for this feature using MS_Description. A few of our tools use it as default to pull out descriptions such as redgate tools, azure purview and I assume a few others. It may be useful to make MS_Description configurable if there is a need. I've attached what is working for us...

{% macro sqlserver__alter_column_comment(relation, column_dict) -%}
    {%- set existing_columns = adapter.get_columns_in_relation(relation) | map(attribute="name") | list %}
    {%- for column_name in column_dict if (column_name in existing_columns) %}
        {%- do log('Alter extended property "MS_Description" to "' ~ column_dict[column_name]['description'] ~ '" for ' ~ relation ~ ' column "' ~ column_name ~ '"', info=false) %}
        IF NOT EXISTS (
            SELECT NULL 
            FROM SYS.EXTENDED_PROPERTIES AS ep

                INNER JOIN SYS.ALL_COLUMNS AS cols
                    ON cols.object_id = ep.major_id
                    AND cols.column_id = ep.minor_id

            WHERE   ep.major_id = OBJECT_ID('{{ relation }}') 
            AND     ep.name = N'MS_Description'
            AND		cols.name = N'{{ column_name }}'
        )
            EXECUTE sp_addextendedproperty      @name = N'MS_Description', @value = N'{{ column_dict[column_name]['description'] }}'
                                                , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                                , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}'
                                                , @level2type = N'COLUMN', @level2name = N'{{ column_name }}';
        ELSE
            EXECUTE sp_updateextendedproperty   @name = N'MS_Description', @value = N'{{ column_dict[column_name]['description'] }}'
                                                , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                                , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}'
                                                , @level2type = N'COLUMN', @level2name = N'{{ column_name }}';
    {%- endfor %}
{%- endmacro %}

{% macro sqlserver__alter_relation_comment(relation, relation_comment) -%}
   {% do log('Alter extended property "MS_Description" to "' ~ relation_comment ~ '" for ' ~ relation, info=false) %}

    IF NOT EXISTS (
        SELECT NULL 
        FROM SYS.EXTENDED_PROPERTIES AS ep
        WHERE   ep.major_id = OBJECT_ID('{{ relation }}')
        AND     ep.name = N'MS_Description'
        AND     ep.minor_id = 0
    )
        EXECUTE sp_addextendedproperty      @name = N'MS_Description', @value = N'{{ relation_comment }}'
                                            , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                            , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}';
    ELSE
        EXECUTE sp_updateextendedproperty   @name = N'MS_Description', @value = N'{{ relation_comment }}'
                                            , @level0type = N'SCHEMA', @level0name = N'{{ relation.schema }}'
                                            , @level1type = N'{{ relation.type }}', @level1name = N'{{ relation.identifier }}';
{% endmacro %}

this is awesome! can you tell me more about what "working for us" means? i.e. Purview/redgate can pull out column descriptions no problem? Way cool!

@mahazza
Copy link

mahazza commented Nov 18, 2021

With Redgate Sql Prompt, it will display MS_Description in the tooltip. Redgate Sql Doc also pulls it as the main description. I know the SSMS designer uses it too for MS_Descriptions on columns, but I don't know anybody who actually uses the designer in SSMS! I asked my colleague about azure purview and it needed some work apparently to get it to take in MS_Description, so I don't think this standard helps there.

edit I forgot to add, SSMS Database Diagram also displays these descriptions in the properties pane when you select a table/view or column

@semcha
Copy link
Contributor

semcha commented Jan 23, 2022

@mahazza Thank you for sharing that code!

@dataders dataders added help wanted Extra attention is needed feature: extended Size: Medium enhancement New feature or request and removed help wanted Extra attention is needed labels Sep 1, 2022
@sdebruyn sdebruyn modified the milestones: v1.5.1, v1.5.0 May 19, 2023
@semcha
Copy link
Contributor

semcha commented May 21, 2023

@sdebruyn How else I can help?

@sdebruyn
Copy link
Member

@semcha

I just added all the new adapter tests to #390. The docs persistence tests in there should be failing right now.

Ideally, your PR #289 would be merged into this branch and would make those tests succeed. That way we are much more certain that both the implementation works and that it works according to how dbt-core expects it to.

So if you would find some time, change the target of your PR to the branch used in #390 and make those persist_docs tests succeed :)
Any help is much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment