Issue #302: Sequenced sql files

[queryluke]

Information

Fixes #302. db_tools.php script now uses information about the installation/upgrade version to process any sql files that are > the current version and <= the target version.

Example 1: Moving from 2.1.0 => 2.4.0, the db_tools script would sequentially process any files within:
$MODULE_NAME/install/protected/2.2.0/*.sql
$MODULE_NAME/install/protected/2.3.0/*.sql
$MODULE_NAME/install/protected/2.4.0/*.sql

Example 2: On a fresh install, it will process all version files. If Coral was up to 2.4, the install script would run the following files:
$MODULE_NAME/install/protected/test_create.sql
$MODULE_NAME/install/protected/install.sql
$MODULE_NAME/install/protected/2.0.0/*.sql
$MODULE_NAME/install/protected/2.1.0/*.sql
$MODULE_NAME/install/protected/2.2.0/*.sql
$MODULE_NAME/install/protected/2.3.0/*.sql
$MODULE_NAME/install/protected/2.4.0/*.sql

Workflow Moving Forward

(Based on information from #302)

1. All DB updates should be placed in the $MODULE_NAME/install/protected/$VERSION/, with a sequence number and issue number $SEQ_NUM-$ISS_NUM
2. DO NOT update the install.sql files anymore. Those represents a < 2.0 installation. All other updates should be processed sequentially in there own files.

Testing

Test an upgrade

1. Install a fresh copy of Coral from the master branch (or use your existing one).
2. Switch to this branch and sync code
3. nav to your.coral.url/
4. Make sure database is updated with any sql files from $MODULE_NAME/install/protected/2.1.0

Test a fresh install

1. Which this code deployed to a server, run an install
2. Make sure database is updated with any sql files from $MODULE_NAME/install/protected/2.1.0

[queryluke]

Additionally, I should point out that you should not merge #351 if you choose to merge this. #351 has a lot of changes to the install.sql file which might cause errors when trying to install.

However, I did move the update_Orders.sql file into the 2.1.0 dir within this branch and gave it the same filename as is in #351.

[t4k]

@queryluke This works really well. I've tested both an upgrade from a 2.0.1 database as well as a fresh install.

One thing it doesn't address, and a bunch of us discussed it once before, though I'm not sure you were involved in this project yet, was an idea of a database or schema version that is independent of the code version.

I don't know if this PR is the best place to be bringing up this issue, but we can continue the discussion elsewhere if needed.

Where this idea becomes relevant is during development between releases when we don't want to lose database content. For example, if I am testing a PR that has a schema change (a new $SEQ_NUM-$ISS_NUM.sql file), I need to revert my db completely back to the schema defined in 2.0.0 and change the version in common/configuration.ini file to then let the updater do its work. If I don't, and I simply change the version number in common/configuration.ini I will get table already exists errors, or something else, because the SQL files have already been run.

In my ideal world we would be able to run a database update script that would be based on a schema version that indicates which SQL updates have already been applied. That way, any database content that has been added to tables that were already updated in previous schema update does not have to be lost when reverting the entire schema to the previous code version.

I do understand that we can manually run the SQL files as needed, but I do think an independent schema version could make the developer experience easier.

[jcuenod]

My understanding is that #285 should solve this problem @t4k.

[queryluke]

@t4k @jcuenod Have you ever worked with database migrations? Not sure which language started it, but my first experience with them was in Rails. The Laravel framework (in PHP) also uses them. I'm pretty partial to Laravel's setup, which I'll outline, but if it's not clear, feel free to read the docs I've linked.

Each DB change is it's own script stored in a file named [timestamp]_[descriptive_name].php. Within the script is an 'up' and 'down' function indicating how to instantiate the change and how to rollback the change. The database itself has a table called 'migrations', which contains 2 columns (migration_name, group) and stores the name of the migration script along with the group (i.e. other migration files, the script ran with).

When you proc the migrations to start, with a command line or other script (like the installer), it queries the database and then runs all migrations that have yet to be added. Once a migration is added, it stores the name of the migration file in the database as well, so it won't be run again. It also applies an incremental number to the group of migrations that were run.

The beauty is that you can then run a 'rollback' command which finds the latest group number and runs the 'down' function for each script within that group, then removes them from the database.

Using this setup also makes working with different branches a cinch. Checkout branch A, db:migrate, test, db:rollback. Checkout branch B, db:migrate, test, db:rollback, and so on. If branch A and branch B were merged, db:migrate would run the db changes for both files, since they would both be new.

The only downside, (extremely minor, IMHO), is that this takes slightly more developer time because they need to write a reverse of all db changes they make.

[queryluke]

I should also say that Laravel's migration structure uses a lot of other Laravel classes. This code base couldn't easily plug it in. But the workflow might be accomplished by putting SQL code in the up/down functions of the migration file.

[jcuenod]

I seem to remember the discussion about rollbacks but maybe I had it with @jeffnm in person. I think it's probably not a good idea because of the extra work it requires from whoever is building the sql files. I haven't used laravel but with Rails, the migrations are being constructed using black(box) magic so it just doesn't require the same kind of work from people building updates.

I speak as a non-module developer though so I am quite open to correction on this.

[t4k]

I'm happy to table this discussion for later or move it back to #285. Sorry about the confusion.

The functionality in this PR works great and as expected for me!

[jeffnm]

This seems to be working. I'd like to go ahead and merge it if there are no objections.