Upgrade failture due to unreliable 'drop/add constraint' in oracle-scripts

[elygre]

The oracle scripts use ALTER TABLE ADD CONSTRAINT to add certain indexes, then ALTER TABLE DROP CONSTRAINT to drop it if it needs to change. This does not work if the database schema has been through an export/import cycle between the CREATE and DROP constraint statements, making upgrades fail:

DbSqlSession - upgrading activiti engine schema from 5.14 to 5.16.1
DbSqlSession - Upgrade needed: 514 -> 515. Looking for schema update resource for component 'engine'
DbSqlSession - performing upgrade on engine with resource org/activiti/db/upgrade/activiti.oracle.upgradestep.514.to.515.engine.sql
DbSqlSession - problem during schema upgrade, statement alter table ACT_RE_PROCDEF
add constraint ACT_UNIQ_PROCDEF
unique (KEY_,VERSION_, TENANT_ID_)
java.sql.SQLSyntaxErrorException: ORA-00955: name is already used by an existing object


1) Actual upgrade code (compresssed for space)

a) From activiti.oracle.upgradestep.59.to.510.engine.sql, creating a unique constraint and index
alter table ACT_RE_PROCDEF add constraint ACT_UNIQ_PROCDEF unique (KEY_,VERSION_);

b)  activiti.postgres.upgradestep.514.to.515.engine.sql, dropping and recrating the unique constraint and index
alter table ACT_RE_PROCDEF drop constraint ACT_UNIQ_PROCDEF;
alter table ACT_RE_PROCDEF add constraint ACT_UNIQ_PROCDEF unique (KEY_,VERSION_, TENANT_ID_);


2) Diagnosis

The issue is discussed in Oracle support document 370633.1, where the following can be read under the headline "Cause":

"The problem is that the index is not dependent on the constraint after Import. The index is not dropped automatically when the constraint is dropped. You need to drop the index manually to drop the constraint totally. This is a restriction when using export/import."

What really happens is that ADD CONSTRAINT can either reuse an existing index, or create an implicit index. If it creates an implicit index, DROP CONSTRAINT will also drop the index. This is the scenario assumed by the scripts, and it works in the simple case.

However, when a table with a constraint and an implicit index is first exported and then imported, the import process will in fact first create the table, then an explicit index, and finally a constraint. After being imported, DROP CONSTRAINT no longer drops the index, and the following CREATE CONSTRAINT fails.

3) Solutions

In order for the upgrade to work, the sequence needs to be rewritten to drop the index manually:

alter table ACT_RE_PROCDEF drop constraint ACT_UNIQ_PROCDEF;
drop index ACT_UNIQ_PROCDEF;
alter table ACT_RE_PROCDEF add constraint ACT_UNIQ_PROCDEF unique (KEY_,VERSION_, TENANT_ID_);

The problem with this, of course, is that if the schema has not been through export/import, we trade one error for another (ORA-01418: specified index does not exist).

-> This is a good time for the activiti developers to suggest a suitable solution 🙂

In case there are no immediate ideas, I have a suggestion: Our product contains a database upgrade mechanism somewhat like DbSqlSession.executeSchemaResource(). There, we have utilized a syntax similar to the Oracle hint syntax that indicates that errors are to be ignored. In the example below, the "–+ IGNOREERROR" statement means that any error from the next sql-statement should be ignored

alter table ACT_RE_PROCDEF drop constraint ACT_UNIQ_PROCDEF;
–+ IGNOREERROR
drop index ACT_UNIQ_PROCDEF;
alter table ACT_RE_PROCDEF add constraint ACT_UNIQ_PROCDEF unique (KEY_,VERSION_, TENANT_ID_);

-> Is this an interesting solution?
-> If implemented reasonably, would a pull request be of interest?

[jbarrez]

I'm not completely following. We've got QA tests which upgrade from all previous version of Activiti to the latest ome.

So what do you mean with export/import cycle? And why is it different from regular upgrade?

[elygre]

To clarify the scenario:

1) As user "a", make these preparations:
* Create table
* Alter table … add constraint
* Verify that there is an index
* Now export user "a" using either "exp" or "expdp" (the two export mechanisms provided by oracle)

2) As user "a"
* Alter table … drop constraint
* Verify that the index is gone

3) As user "b"
* Import the export made by user "a" as part of #1 above, using either imp or impdp
* Alter table … drop constraint
* Verify that the index IS NOT gone


The reason for what we see as user "a" is the following:

* When using "add constraint", oracle will create the required index
* When using "drop constraint", oracle will drop the underlying index, if it was created from "add constraint"

However, when we import into user b, the following happens during import:

* First, the import program executes "create table"
* Then the import program executes "create index" for the underlying index
* Then the import program executes "add constraint", which will reuse the index from the previous step
* Later, when the activiti upgrade script runs "drop constrain", oracle will not drop the index (it was not created from "add constraint")
* Then, when the new "add constraint" runs, the error occurs

I would consider this a bug in Oracle, but seeing the history of this behavior, I don't expect it to change. Thus, applications must adapt to it:

* After using "alter table … drop constraint", execute a "drop index" statement.
* However, since this "drop index" statement may fail ("ora-01418: specified index does not exist"), either the drop index must be made conditional, or it must ignore the return code.

[elygre]

Here is a linux script which creates two users, configures ACT1, runs export/import, upgrades ACT1, and then upgrades ACT2. Run the script using username/password, or just "/" if you are the oracle user. The script will use the users ACT1 and ACT2, so be careful if these are in use!

The script should fail with this output towards the end:

<code>
SQL> alter table t add constraint i unique (a,b)
*
ERROR at line 1:
ORA-00955: name is already used by an existing object
</code>

Script:

<code>
#!/bin/bash

# ———————————————————
# Setup users
# ———————————————————
sqlplus $1 << EOF
create user act1 identified by act1;
create user act2 identified by act2;
grant connect, resource to act1;
grant connect, resource to act2;
EOF

# ———————————————————
# Setup table and constraint for user ACT1
# ———————————————————
sqlplus act1/act1 << EOF
create table t (a number, b number);
alter table t add constraint i unique (a);
insert into t (a) values (1);
select index_name from user_indexes;
EOF

# ———————————————————
# Export/import from ACT1 to ACT2
# ———————————————————
exp userid=act1/act1 tables=t
imp userid=act2/act2 fromuser=act1 touser=act2

# ———————————————————
# Verify and upgrade users ACT1 (ok) and ACT2 (not ok)
# ———————————————————
sqlplus /nolog << EOF

connect act1/act1
select index_name as "Indexes before drop constraint" from user_indexes;
alter table t drop constraint i;
select index_name as "Indexes after drop constraint" from user_indexes;
alter table t add constraint i unique (a,b);
select index_name as "Indexes after add constraint" from user_indexes;

connect act2/act2
select index_name as "Indexes before drop constraint" from user_indexes;
alter table t drop constraint i;
select index_name as "Indexes after drop constraint" from user_indexes;
alter table t add constraint i unique (a,b);
select index_name as "Indexes after add constraint" from user_indexes;

EOF
</code>

[jbarrez]

Ok, I now better understand why it's a problem. However, we can't retroactively fix the past upgrade scripts (we can change them in the current release).

Since you seem to be quite knowledgeable on the topic, would you be able to create a pull request on the current master with these changes.

[elygre]

Just returned from vacation, entering a release sprint that will run for a couple of weeks. After that, I'll get to it!

Is there a particular deadline for the next release?

[jbarrez]

No - not really. But since the Alfresco Summit is happening next week and the first week of October, it's likely to be after that.

[elygre]

OK, so I'm back at work, and will start looking at this. There are two main approaches:

First, we can add functionality to the Activiti DbSqlSession.executeSchemaResource() to specify that a given SQL-statement can fail. In a similar project, we use a special comment to specify that the next statement should run without error checking, as shown below. The advantage of this approach is that the SQL-code is very easy to read, and also very easy to write :-), and the DbSqlSession-modifications will be very small.

<code>
alter table ACT_RE_PROCDEF
    drop constraint ACT_UNIQ_PROCDEF;

–+ IGNOREERROR
drop index ACT_UNIQ_PROCDEF;
</code>

Alternatively, we can use a standard PL/SQL-block. Unlike the example above, there is no activiti-specific semantics to the sql-script, since the error handling is basically done by Oracle. However, the DbSqlSession changes are bigger, since the semicolon would not always be a reliable end-of-statement indicator.

<code>
alter table ACT_RE_PROCDEF
    drop constraint ACT_UNIQ_PROCDEF;

begin
  execute immediate 'drop index ACT_UNIQ_PROCDEF';
exception
  when others then
    null; 
end;
/
</code>

I suggest we go with the first one, and will start implementing that. If there is disagreement, however, let's talk!

[elygre]

There is now a pull request on github, https://github.com/Activiti/Activiti/pull/383.

[elygre]

There is now a pull request on github, at https://github.com/Activiti/Activiti/pull/383, along with a question from Tijs and a response from me related to how the implementation works.

We are unable to upgrade beyond 5.14 because of this, so it would be nice if it could be part of the next release.