search

tacitknowledge / autopatch

An automated database patching framework for Java.
41 stars 19 forks source link

mysql.properties includes a primary key that fails with new migration strategy #27

Closed jeffkole closed 12 years ago

jeffkole commented 12 years ago

The primary key on the patches table for mysql is still the system_name. From the looks of it, the patches table is now holding records for all patches run, so the primary key probably needs to change to (system_name, patch_level).

scottfromsf commented 12 years ago

Thanks for pointing this out, Jeff. We'll look into it and will get back to you shortly.

jeffkole commented 12 years ago

No worries. I meant to log the issue and then submit a patch for it. Is there anything I should know about the new design of the patches table that would prevent a primary key of (system_name, patch_level) from being appropriate? I see that the primary key on the hsqldb schema was removed entirely, but the primary key of just system_name still is in place in the Oracle, SQLServer, and Postgres versions along with MySQL.

MexicanHacker commented 12 years ago

Hey Jeff, I was thinking about the problem yesterday and I would like to get more info about this, as I see it the new migration strategy shouldn't require you to modify the primary key or change the way it works. The new strategy changes the way we decide which patch should be applied or not, but shouldn't affect the rest of the functionality or change it. If you see a problem I would like to know more about the preconditions to reproduce it.

That being said, I see we can weight the uniqueness of the patches based on a new primary key based on system and level vs the ability to switch between strategies. I'll think about it.

Lastly, the primary key missing from hsqldb.properties could be a bug, we'll take a look at it right the way.

jeffkole commented 12 years ago

I haven't run the version 1.3 autopatcher yet, so I am speculating from looking at the code, but here is what I see:

  1. The patches table gets created with a primary key of system_name.
  2. An initial row is inserted like this (corresponding to the sql for level.create): INSERT INTO patches (system_name, patch_level) VALUES (?, 0)
  3. After having run a patch, the patches table is "updated" to include that patch_level, but the update is really an insert like this (corresponding to the sql for level.update): INSERT INTO patches (patch_level, system_name, patch_date) VALUES (?, ?, CURRENT_TIMESTAMP)

The second insert will fail, because the same system_name is being used which will cause a duplicate key violation. I believe this used to be an actual update statement that just changed the patch_level and patch_date for the record associated with the system_name.

If the new design is to record each patch that is run, then the primary key needs to be a composite of system_name and patch_level.

Jeff

On Nov 17, 2011, at 5:58 AM, Oscar Gonzalez wrote:

Hey Jeff, I was thinking about the problem yesterday and I would like to get more info about this, as I see it the new migration strategy shouldn't require you to modify the primary key or change the way it works. The new strategy changes the way we decide which patch should be applied or not, but shouldn't affect the rest of the functionality or change it. If you see a problem I would like to know more about the preconditions to reproduce it.

That being said, I see we can weight the uniqueness of the patches based on a new primary key based on system and level vs the ability to switch between strategies. I'll think about it.

Lastly, the primary key missing from hsqldb.properties could be a bug, we'll take a look at it right the way.

Reply to this email directly or view it on GitHub: https://github.com/tacitknowledge/autopatch/issues/27#issuecomment-2776180

hemri commented 12 years ago

Jeff, you are right. In order to provide more information to the recently added strategy (missing patches strategy) and for future ones, the system needs to keep the information of which patches have been applied. That is why the level.update refers to an INSERT INTO , as you have pointed out. Removing the primary key from hsqldb.properties is a bug and we need to put a primary key back in place. Therefore, as you mentioned, a composite primary key is the solution: system_name and patch_level. Thanks.