-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Bigquery catalog generation (#830) #857
Merged
Merged
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
046942b
Write out some SQL
jwerderits 4921294
Merge branch 'development' into snowflake-get-catalog
jwerderits 22d8ad6
Add snowflake tests, rework existing postgres to use dbt seed + dbt r…
jwerderits 5d97937
Add tests for dbt run generating a manifest
jwerderits 7832322
Implement get_catalog for snowflake, move adapter-side logic into the…
jwerderits b1ab19f
I never remember to run pep8
jwerderits 3bfa6bb
Fix up paths so they make work in CI as well asl ocally
jwerderits 85605dd
Merge branch 'development' into snowflake-get-catalog
jwerderits 461da2f
Remove misleading comment, explicity order postgres results like I di…
jwerderits 0e3edf1
Merge branch 'dev/isaac-asimov' into snowflake-get-catalog
jwerderits 85a4413
PR feedback: union all and dbt schemas only
jwerderits d4e2cfd
bigquery catalog/manifest support
jwerderits 1454572
pep8
jwerderits d4597df
Don't need to union anything here
jwerderits 94a4102
Merge branch 'snowflake-get-catalog' into bigquery-catalog-generation
jwerderits 26df721
Windows path nonsense
jwerderits 5d27b2b
Merge branch 'snowflake-get-catalog' into bigquery-catalog-generation
jwerderits f5d5bbc
Merge branch 'dev/isaac-asimov' into bigquery-catalog-generation
jwerderits 7fb6e95
Remove extra line
jwerderits ad55c4c
Merge branch 'dev/isaac-asimov' into bigquery-catalog-generation
jwerderits 694c508
Handle nested records properly in bigquery
jwerderits 75f0a22
Add a little decorator to do attr + use_profile
jwerderits e7a4641
Add new BQ test for nested records, manifest not complete
jwerderits ae9ee71
ok, tests now work again
jwerderits 251cb19
update a docstring to be true again
jwerderits File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should add another test for a table with nested records. i'm not sure how to do that in the model SQL but i think this is potentially a difference in how BQ works vs. other warehouses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really great point @cmcarthur!
I usually test with an array of structs, eg:
I don't think we'll be able to use a CSV file to construct nested or repeated records.
Regarding behavior, I think there are two options:
BigQuery exposes nested fields (ie. the
struct
s above) as:We have a function to flatten the nested fields here: https://github.com/fishtown-analytics/dbt/blob/development/dbt/schema.py#L103
The other option to to just show a single field, but render out a complete type. For
people
above, that type would be:There is new logic to build this type name in my
bq-incremental-and-archive
branch, over here: https://github.com/fishtown-analytics/dbt/blob/feature/bq-incremental-and-archive/dbt/schema.py#L170I think the first option is probably preferable, but I'm open to discussion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think I've implemented this test and fixed the code sucessfully - good thing you asked, it definitely did not work and I fixed it. I went with @drewbanin's flatten() suggestion.