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

Leverage "show table extended" for Relation type metadata #50

Merged
merged 2 commits into from
Feb 11, 2020

Conversation

jtcohen6
Copy link
Contributor

@jtcohen6 jtcohen6 commented Jan 29, 2020

  • Following Saner approaches to getting metadata for Relations #49, leverage show table extended in ... like '*' statement to get metadata on all relational objects in each database/schema, including their Type
  • Naively check the information column result of the statement for the substring 'Type: VIEW'. If found in the text result, set relation.type to 'view', otherwise set to 'table'. (I am seeing Type: MANAGED for tables that dbt creates in Databricks.)
  • Catch Database ... not found exception if dbt runs the statement for a database that doesn't (yet) exist. I encountered this error after creating a dbt model with a custom schema and executing a dbt commands which did not run that model (e.g. dbt seed).
  • Fixes Fix is_incremental() macro #48

This was referenced Jan 29, 2020
@jtcohen6 jtcohen6 requested a review from drewbanin February 4, 2020 18:46
@Dandandan
Copy link
Contributor

Tested ithis on EMR/Glue data catalog.
Seems to work fine!


relations = []
quote_policy = {
'schema': True,
'identifier': True
}
for _database, name, _ in results:
for _database, name, _, information in results:
rel_type = ('view' if 'Type: VIEW' in information else 'table')
Copy link
Contributor

Choose a reason for hiding this comment

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

we've had issues in the past with other adapters where we assumed that a relation was either of type "table" or "view". In practice, some databases support other relation types, like materialized views or external tables. In this case, I think this logic looks A-OK, but I did want to flag it as something to keep in mind for the future!

dbt/include/spark/macros/adapters.sql Show resolved Hide resolved
dbt/include/spark/macros/materializations/incremental.sql Outdated Show resolved Hide resolved
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! Nice work on this one :)

@jtcohen6 jtcohen6 merged commit f247176 into master Feb 11, 2020
@jtcohen6 jtcohen6 deleted the feature/show-table-extended branch February 11, 2020 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix is_incremental() macro
3 participants