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

Column and table comments for postgres/redshift (#2333) #2378

Merged
merged 3 commits into from
May 4, 2020

Conversation

beckjake
Copy link
Contributor

@beckjake beckjake commented Apr 30, 2020

resolves #2333

Description

This is very much based off the work being done by @snowflakeseitz in #2321
I intentionally copy+pasted the common adapter_macros to make merging easier.

With this PR, redshift and postgres now generate column and table comments during view and table creation. Their get_catalog macros also now include those comments in the output (instead of null) and the end up in the catalog itself.

I added tests as well, of course!

Redshift is a little special:

  • you can comment on late-binding views, but the comment isn't available anywhere as far as I can tell. I set it up so dbt still issues these commands, because maybe I just couldn't find them
  • you can't comment at all on late-binding view columns (it's an error!)

One annoying/weird quirk we might need to think about: When you do have comments, results lines show up as COMMENT in XXXs instead of CREATE TABLE in XXXs. That's a little goofy! Maybe comments should work like hooks do.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@cla-bot cla-bot bot added the cla:yes label Apr 30, 2020
@beckjake beckjake force-pushed the feature/pg-rs-persist-docs branch 3 times, most recently from 2d8f93a to d4ba35f Compare April 30, 2020 20:18
@beckjake beckjake requested a review from drewbanin April 30, 2020 20:18
Jacob Beck added 3 commits April 30, 2020 14:49
- add some table comment framework stuff
- have redshift/postgres catalogs include table comments
- have redshift/postgres add comments to columns/tables/views
- push some bigquery-specific formatting into bigquery
- add tests for table comments
…se it

 - also fixed the default incremental to include that it is creating a table
@drewbanin
Copy link
Contributor

One annoying/weird quirk we might need to think about: When you do have comments, results lines show up as COMMENT in XXXs instead of CREATE TABLE in XXXs. That's a little goofy! Maybe comments should work like hooks do.

I didn't think about this at all! That's kind of a big problem IMO. We're overdue for a rethink on how materializations work (especially w/r/t these create_X_as macros). I'm ok with this for the moment, but I think it would be good to prioritize materializations v2 in 0.18.0 :)

Reviewing this one now!

@beckjake beckjake mentioned this pull request May 1, 2020
4 tasks
Copy link
Contributor

@drewbanin drewbanin left a comment

Choose a reason for hiding this comment

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

This LGTM! The one thing that gives me pause is the change to the Redshift catalog query, but I tested it locally and it appeared to work well.

@beckjake beckjake merged commit e6bb060 into dev/octavius-catto May 4, 2020
@beckjake beckjake deleted the feature/pg-rs-persist-docs branch May 4, 2020 13:47
Copy link
Contributor

@McKnight-42 McKnight-42 left a comment

Choose a reason for hiding this comment

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

great example to pull from @dbeatty10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support persist docs on Postgres and Redshift: Relations and columns
3 participants