-
Notifications
You must be signed in to change notification settings - Fork 321
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
[view] Support CREATE OR REPLACE #710
Conversation
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.
@mgorven Thanks for submitting this!
Would you add some tests that exercise this feature? It looks like these tests will be worthwhile:
ValueError
is raised whenmaterialized
andreplace
are both true- A view is created when
replace
is true and it doesn't exist - A view is replaced when
replace
is true and it already exists
Sure. Are there any docs on how to run tests locally? It seems like they require a real database. |
That's definitely an area that could be improved. For now, I've kicked off CI for this PR. You should be able to commit additional changes and allow CI to run here. It appears that CI has started failing, so for the purposes of this PR please disregard the existing failures while working on new tests. Thanks! |
6816617
to
c852176
Compare
b7c1db9
to
1d6e0d7
Compare
As was discovered in #621, the tests don't actually create views (they create a table). I've changed the tests to run |
f10afbe
to
c6fd76e
Compare
Only test failures are |
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.
One last thing that I finally realized: the replace parameter needs a bit of documentation in the create_view()
docstring.
It'll be rendered automatically the next time the docs are built, and will appear like the other parameters in the create_view()
documentation.
Thanks for your work on this, @mgorven! |
No description provided.