Snowflake-Labs / schemachange

A Database Change Management tool for Snowflake
Apache License 2.0
482 stars 219 forks source link

Script not executed based on version control #114

Open drgsec1234 opened 2 years ago

drgsec1234 commented 2 years ago

When multiple developers work we get to a problem that one of them merge a script with version control for instance 50 The other developer is still working on a version 40, even if he merges that deploy will be skipped since: Max applied change script version: 50 We use Snowflake sequence to get unique version number not to mess up and have same versions, but in this case this doesn't work. Is this expected and we also need to keep track to use larger value compared to last deployment or not?

bnelsontfs commented 1 year ago

Bumping this one because I just ran into the same problem.

  1. The latest deployed change is version 10.
  2. Developer A creates version 11 and starts working on it.
  3. Developer B creates version 12 and starts working on it.
  4. Developer B's changes are ready and get deployed to production. Now the latest version in production is 12.
  5. Developer A's changes are ready for production, but schemachange won't deploy them because 12 > 11.

How are we supposed to manage this? Nobody can guarantee the sequencing of changes going into any particular environment, and I don't want to have to rename a bunch of files as prep for entering any environment.

satyabehara commented 1 year ago

We too have the same issue. @sfc-gh-jhansen can you please just confirm this is the expected behavior or if we're probably overlooking something ?

sfc-gh-jhansen commented 1 year ago

This is the expected behavior, and is the default behavior for Flyway as well. Flyway does offer an "out of order" setting to allow for situations like this. The reason it's like this is to help ensure dependency order is met, but customers do run into the challenge you described. Need to think through the risks of allowing out of order executions.

The manual solution, although a bit of a pain, is to rename the script with the next version number in these situations.

Again, need to think through the implications/risks of running out of order. In that case, the version number really looses a bunch of its meaning. It no longer guarantees that all scripts run in a specific order.

sfc-gh-tmathew commented 9 months ago

Hello @drgsec1234, @bnelsontfs and @satyabehara

Could you check the issue #110 and see if using the combination of -f and -m will help you control the order of execution and wrap the versioning of a batch of scripts with single version number?

Does this help?

bnelsontfs commented 9 months ago

@sfc-gh-tmathew It does not solve the problem, and in fact, I believe it introduces more.

There's still the issue of not knowing the specific order that changes will be implemented into any environment. I think, in order to use this method, one would have to create a new "master" migration file for each change in each environment, because there's still no guarantee of the order of application of various features to those different environments. It takes away the convenience of "run everything that hasn't already been run" and changes it to "create a new script to tell me what to run," which might not even be able to be used in a later environment. So, there's more work on each migration than there was before. At that point, you might as well just create a master script for each environment and run it directly - schemachange isn't providing a ton of value at that point beyond logging the master script execution in change_history and offering templating.

The larger issue is that nonidempotent changes that may fail are going to be a problem. For example, if I have scripts in my list of scripts that are all %included that do alter table <table> add column <column>, and somehow one alter fails (perhaps the column already exists for some odd reason), now that master script must be edited to proceed, because you can't rerun the alters that ran prior to the failure.

FWIW, we're limping around this issue by having an out-of-order flag that I've implemented in a fork. While this generally works, there is one situation with repeatable scripts that is still a huge problem, and I think it's an issue even without out-of-order execution: If a repeatable script exists in the main branch, two developers branch from there, and one changes the script and it gets pushed back to main and executed (presumably in production), the other developer will still have an older version of that script that schemechange will want to execute in other environments. I suppose we could merge from main before executing in lower environments, but I haven't thought that all of the way through.

sfc-gh-tmathew commented 8 months ago

Thank you @bnelsontfs for a detailed response. I understand the #110 is not helpful.

You rightly call out the out of order execution not working when an older version of an object such as View, Stored procedure overwrites a newer version of the object because of the order in which the commits were pushed. Honestly, I believe this scenario is more of a release management, code review than a schemachange feature.

Do you have the PR for the out of sequence flag you have implemented? Lets review the PR and tie it to this issue.

FWIW, Snowflake just released this gem: https://docs.snowflake.com/en/release-notes/2023/7_38#alter-table-support-for-if-not-exists-with-add-column-and-drop-column

which will be useful for our create table if not exists syntax and alter statements that follow.

We would also have to update the documentation to ensure that the out-of-sequence flag is not used with other combinations to lead to unintended consequences.