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

Fix bigquery snowflake unknown relation types #1872

Merged

Conversation

beckjake
Copy link
Contributor

These fixes are kind of speculative, I can't come up with a nice way to reproduce them. But they make sense to me as errors and I am reasonably confident in the fixes.

For bigquery, instead of defaulting to None if the relation type isn't immediately recognized, we default to External.

Similarly, in the SQL adapters no longer pass along whatever the list_relations_without_caching macro returns. Instead, check that it's something we support and if it isn't, mark it "external".

Fixes #1868
Fixes #1869

Jacob Beck added 2 commits October 28, 2019 15:14
For bigquery:
  if a relation type is not known, use 'RelationType.External' in place of None
For snowflake/other sql adapters:
  if the relation type returned by list_relations_without_caching macro is not
     a known RelationType, treat it as External
@cla-bot cla-bot bot added the cla:yes label Oct 28, 2019
@drewbanin
Copy link
Contributor

I had assumed that the fix for #1868 would involve a change to the catalog query! I'm happy to replace None with "external" here (that's actually exactly correct for an external google sheets relation as noted in the issue) but we can also update the catalog query for at least the google sheets case if that feels cleaner.

@beckjake
Copy link
Contributor Author

I think this is the more correct fix actually. dbt really only cares about views and tables, and should just be able to treat everything else as "external". We support materialized views because eh, whatever, but I think this is more sustainable long-term instead of adding new stuff all the time.

Also it was hard to tell from the issue where the message actually came from, but I figured this is the only spot in dbt right now that's going to set a table type field that's declared as RelationType to None.

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.

LGTM

@beckjake beckjake merged commit 8239f9c into dev/louisa-may-alcott Oct 30, 2019
@beckjake beckjake deleted the fix/bigquery-snowflake-unknown-relation-types branch October 30, 2019 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants