Use column.comment from catalog/database if column.description is missing #263
Conversation
…sing This will display the column description fetched from the database if there is no column description in the DBT config. Partially fixes dbt-labs#128, partially because the table description is still missing. Adding the table description would require more substantial changes, since the table description (as far as I could tell) is not found in catalog.json at the moment.
|
@halvorlu thank you so much for this contribution! A lot has changed since the creation of the original issue, but this is still something that could be valuable to The table description is actually included in the catalog. The above is from the catalog schema. Depending on the adapter/database being used, it may not be consistent but it should be there. For example, in postgres, we're grabbing the table-level comment here. I'm not sure which adapter you're using but that could be why you may not see the the value coming back in the catalog. It's definitely worth checking that the field is there, even if it is null. Would you be able to add the table description in to your PR as well? |
We don't merge description and comment, we choose the first one that isn't empty/null.
This allows us to display the description found in the database if we haven't entered a description in the dbt config.
|
@emmyoop Thanks for useful feedback! I wasn't aware that the table description was in the catalog. I think the BigQuery adapter (which I use) doesn't fetch it from the database. Anyway, I fixed the code so that we use the comment from the catalog if it is present. |
|
Should I/we document somewhere that the descriptions are fetched from the database if not provided in the dbt config? This behavior is probably not obvious to most users. |
|
@emmyoop Anything more that needs to be done in this PR before merging? |
|
We should definitely add some documentation around this functionality. Good call bringing it up! We plan to release this feature with our next minor release, |
There was a problem hiding this comment.
@halvorlu thanks for your patience in getting back to you on this!
I looked over the code and it's generally looking good. However, it's applying the override to the column descriptions everywhere (models, seeds, snapshots, sources) but the table override is only happening for sources. It's important this be consistent and predictable.
Since sources are the only object that dbt does not generate/own, it's the only object we want to override.
Can you please update the code so that column comments are only used for sources?
jtcohen6
added
the
ready_for_review
|
This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days. |
|
Although we are closing this PR as stale, it can still be reopened to continue development. Just add a comment to notify the maintainers. |
Partially resolves #128 (see below)
Description
This will display the column description fetched from the database if there is no column description in the DBT config.
Partially resolves #128, partially because the table description is still missing.
Adding the table description would require more substantial changes, since the table description (as far as I could tell) is not found in catalog.json at the moment.
Checklist
CHANGELOG.mdand added information about my change to the "dbt next" section.