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

Avoid errors when missing column in yaml doc #4285

Merged
merged 3 commits into from
Nov 16, 2021

Conversation

kadero
Copy link
Contributor

@kadero kadero commented Nov 16, 2021

resolves #4151

Description

As described in the issue, we just need to update the postgres/redshift adapter.

I reproduced the issue described in the issue:
image

With the fix, the dbt run pass:
image

And the wrong fiied is not persisted:

image

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

@cla-bot cla-bot bot added the cla:yes label Nov 16, 2021
@kadero kadero changed the title Update postgres__alter_column_comment Better error when persist docs Nov 16, 2021
@kadero kadero marked this pull request as draft November 16, 2021 10:28
@kadero kadero marked this pull request as ready for review November 16, 2021 10:55
@kadero kadero changed the title Better error when persist docs Avoid errors when missing column in yaml doc Nov 16, 2021
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

@kadero Looks great! Any chance you could add an integration test case for this behavior? It could look exactly like the one in the Snowflake adapter here

@kadero
Copy link
Contributor Author

kadero commented Nov 16, 2021

@kadero Looks great! Any chance you could add an integration test case for this behavior? It could look exactly like the one in the Snowflake adapter here

Done ✔️,

Thanks for for the link to the test and your support @jtcohen6 🙂 !

🙏

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

@kadero Thank you for the contribution, as always!

@@ -95,3 +95,32 @@ def run_has_comments_pglike(self):
def test_postgres_comments(self):
self.run_has_comments_pglike()

class TestPersistDocsColumnMissing(BasePersistDocsTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Confirmed this test fails without the fix, and passes with it

@jtcohen6 jtcohen6 merged commit 2241698 into dbt-labs:main Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Need more helpful error message when persist_docs encounters invalid column names
2 participants