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

Add MaterializedView relation type, update Snowflake adapter #1431

Conversation

adrianisk
Copy link
Contributor

Fix for issue #1430

-- Adds MaterializedView to the list of RelationTypes in the BaseRelation class
-- Updates the snowflake__list_relations_without_caching macro to handle 'MATERIALIZED VIEW' table_type

I didn't find any unit tests for the BaseRelation parsing, so I just tested manually. Let me know if there's a good way to add a test for this.

Prior to the fix running a model in this project returned:

* Deprecation Warning: The adapter function `adapter.already_exists` is deprecated and will be removed in
 a future release of dbt. Please use `adapter.get_relation` instead.
 Documentation for get_relation can be found here:
 https://docs.getdbt.com/reference#adapter

Encountered an error:
Runtime Error
  Invalid arguments passed to "SnowflakeRelation" instance: type.'MATERIALIZED VIEW' is not one of ['table', 'view', 'cte', None]  

After the fix the model runs successfully:

Running with dbt=0.13.0
* Deprecation Warning: The adapter function `adapter.already_exists` is deprecated and will be removed in
 a future release of dbt. Please use `adapter.get_relation` instead.
 Documentation for get_relation can be found here:
 https://docs.getdbt.com/reference#adapter

Found 140 models, 364 tests, 0 archives, 0 analyses, 103 macros, 0 operations, 0 seed files, 0 sources

09:31:09 | Concurrency: 3 threads (target='production_bi')
09:31:09 |
09:31:09 | 1 of 1 START table model bi.the_model................... [RUN]
09:31:17 | 1 of 1 OK created table model bi.the_model.............. [SUCCESS 1 in 8.01s]
09:31:17 |
09:31:17 | Finished running 1 table models in 14.96s.

Completed successfully

Done. PASS=1 ERROR=0 SKIP=0 TOTAL=1

@drewbanin
Copy link
Contributor

Thanks @adriank-convoy - this looks great!

We're going to cut an 0.13.1 bugfix release relatively soon (within the next ~week) whereas 0.14.0 is going to take a little bit longer (5-6 weeks). If you're interested in getting this live for 0.13.1, you could apply your changed on top of dev/0.13.1 and then change the destination branch for this PR to dev/0.13.1 accordingly. If you're happy waiting for 0.14.0 to use this, then I think we can merge as-is once the automated tests pass

@drewbanin drewbanin self-requested a review April 30, 2019 16:51
@adrianisk adrianisk changed the base branch from dev/wilt-chamberlain to dev/0.13.1 April 30, 2019 16:55
@adrianisk adrianisk changed the base branch from dev/0.13.1 to dev/wilt-chamberlain April 30, 2019 16:56
@adrianisk
Copy link
Contributor Author

Not sure if I can change to source branch so I'm just going to re-do the pr from dev/0.13.1

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.

2 participants