Rename view #320
Rename view #320
Conversation
gagalago
force-pushed
the
rename
branch
7 times, most recently
from
9635633 to
ae377ce
Compare
|
Liking this so far! |
|
A thought for versioning: can we take the old view version as either a required argument for what file to copy or an optional argument and calculate the current version of it is not given? Should a renamed view maintain the version number of the original name or start fresh? |
we cannot copy the file from scenic gem because if we rename 2 views (ex: A and B) in the same migration and that one of them use the other (ex: A use B), then the resulting SQL file need also that the in the SQL of the depending view, the other is renamed. As most of the time that will not be an issue, I think it's better to copy the file beforehand thanks to a rake generator task.
if we want to allow flow as described in #318 we are not able to maintain the original version number (he can be already taken), so we will have to start fresh (or use the next version available for the new name). |
gagalago
force-pushed
the
rename
branch
14 times, most recently
from
a7d91c8 to
4ca42b9
Compare
|
in case of a used column type change, we are not able to replace the view. I will have work again on this pull request to improve it. |
gagalago
force-pushed
the
rename
branch
14 times, most recently
from
ba73269 to
1529bce
Compare
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
|
|
||
| successfully "rails generate scenic:view greeting --materialized --rename greeting_next" | ||
| verify_identical_view_definitions "greeting_nexts_v01", "greetings_v02" | ||
| replace_into_migration "update_greetings_to_version_2", "rename_view", "replace_view" |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [89/80]
|
|
||
| successfully "rails runner 'Scenic.database.refresh_materialized_view(:greeting_nexts)'" | ||
|
|
||
| successfully "rails generate scenic:view greeting --materialized --rename greeting_next" |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [92/80]
| successfully "rake db:migrate" | ||
| verify_result "Greeting.take.hello", "hi" | ||
|
|
||
| successfully "rails runner 'Scenic.database.refresh_materialized_view(:greeting_nexts)'" |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [92/80]
gagalago
force-pushed
the
rename
branch
2 times, most recently
from
838385e to
168d1c3
Compare
|
I used this branch in production and it worked as expected. @calebthompson @derekprior do you have some others remarks? |
|
Are there plans to merge this branch? |
|
After a brief meeting of the Scenic core team over text, no I don’t believe we’ll be merging this. Rather than renaming, we recommend dropping the old view and creating a new one. However, we do see the value in a “zero downtime” materialize view update and would entertain a PR that explored on that more specifically. We do appreciate the work that went into writing and verifying this, so thank you! |
|
Credit to @gagalago, I just noticed this branch while searching for a “zero downtime” materialized view migration solution and was curious about its future. Thanks for the update! |
allow to rename views managed by Scenic