Sequences for entities

[wivern]

for #324

[chrisknoll]

This looks very impressive, and I like how you worked through all the 'initial values for sequences'. The oracle code looks pretty complicated and I don't actually have any way of testing this out.

It appears that you put in the various checks for when max(id) might return null, which is good, this migration needs to work when the database is empty (such as in a completely new install). Did you manage to test this migration on the different flavors of RDBMS? We can test the migrations internally on MSSQL and Postgresql (@anthonysena : you have a local install of Postgresql with some cohort defs?) but oracle has always been a challenge. If you can confirm that this works for Oracle, we'll work out testing it on the other 2 platforms.

Note to everyone: before running webapi with this PR in place, I strongly suggest making a backup of your database before launching. The changes here are not easily rolled back, so your best option is a backup-restore if something doesn't work right.

[anthonysena]

I can test this out on PostgreSQL.

+1 for backing up your DB!

[chrisknoll]

Ok, I'll check it on our internal SQL server. @chen-regen : do you have an oracle instance you could test this with?

[chrisknoll]

Error when running on MS SQL:

Migration V2.2.0.20180215143000__remove_password.sql failed
-----------------------------------------------------------
SQL State  : S0001
Error Code : 156
Message    : Incorrect syntax near the keyword 'IF'.
Location   : db/migration/sqlserver/V2.2.0.20180215143000__remove_password.sql (C:\Git\WebAPI\target\classes\db\migration\sqlserver\V2.2.0.20180215143000__remove_password.sql)
Line       : 1
Statement  : ALTER TABLE ohdsi.sec_user DROP COLUMN IF EXISTS password;
ALTER TABLE ohdsi.sec_user DROP COLUMN IF EXISTS salt;

it appears that this syntax was introduced in MSSQL 2016, I'm currently running on 2014...do you think we could make this compatable with sql 2014?

I've located an example of how the old code would get implemented in 2016, but this can be used for the 2014 environment: https://www.mssqltips.com/sqlservertip/4402/new-drop-if-exists-syntax-in-sql-server-2016/

IF EXISTS(SELECT * FROM SYS.columns WHERE name='{column}' AND  
          OBJECT_ID = OBJECT_ID('[{schema}].[{table}]'))
ALTER TABLE [{schema}].[{table}] DROP COLUMN [{column}]
GO

[anthonysena]

@chrisknoll - This is my bug - sorry for that. I contributed that script as part of another PR. Let's change it since we should assume that a previous script created those columns and therefore the existence check is not required.

Also, for all: I'm also finding some bugs with the PostgreSQL script for sequencing. I'm thinking it might be easier to merge this working into a branch on WebAPI so that we can all contribute fixes on it? I'll plan to do this unless I hear from folks not to do that.

[chen-regen]

@chrisknoll @anthonysena Hi Anthony and Chris, sorry I just noticed this as Anthony brought it to my attention. I am actually working on a solution for the sec_role_permission table by sequence. Regenstrief will be working on creating a second instance for our OHDSI suite. Now we only have one instance so it would be challenging and dangerous for me to test this big changes. When do we plan on getting this released? I will try to talk to our folks here and see if we can get a testing environment sooner...

[anthonysena]

Hi @chen-regen - we aim to have the 2.3 release ready by 2/28 per #301. I'd agree that it is important to have a secondary environment for testing these changes. Just let us know the timing for your secondary environment and worst case, we can apply some patches as part of a 2.3.x release to address any issues found with Oracle.

[chen-regen]

@anthonysena wow this is pretty intense in terms of timing. I doubt our infrustracture team will get the environment ready by then considering they are working on our new ARACHNE servers now. Well if I won't be able to test, we will have to apply patches as you said. Internally, we really would like to get the cohort analysis changes and security improvements deployed with 2.3 release...