fix(emx2): Check if column has refBack before deleting #4379
Conversation
There was a problem hiding this comment.
looks okay, indeed needs a test?
offtopic for future PR, the migration is a hell piece of code and might need some change soon.
It should
- first add tables and pkey (these things never change, pkey needed for refs)
- then reiterate and all other columns (might be updated/deleted)
- delete all columns last
this relates to circular error we still have.
| for (Column column : mergeTable.getColumns()) { | ||
| if (!column.isReferenceWithRefback()) continue; | ||
|
|
||
| Column refbackColumn = column.getReferenceRefback(); |
There was a problem hiding this comment.
where do you check for refbacks in existing tables that are not being dropped (might even be in other schema)? I would expect that to happen in SqlTableMetadata
| @@ -297,7 +289,7 @@ public void dropColumn(String name) { | |||
| throw new MolgenisException("Drop column " + name + " failed: column does not exist"); | |||
| } | |||
| // if changing 'ref' then check if not refBack exists | |||
| checkNotRefback(name, column); | |||
There was a problem hiding this comment.
I think you need to move this check so that it fires within transaction?
I expected this to be checked within 'executeRemoveColumn' so that when executeDropTable is executed it also happens.
| @@ -221,7 +221,7 @@ private static SqlTableMetadata alterColumnTransaction( | |||
|
|
|||
| // if changing 'ref' then check if not refBack exists | |||
| if (!oldColumn.getColumnType().equals(newColumn.getColumnType())) { | |||
| tm.checkNotRefback(columnName, oldColumn); | |||
| checkNotRefback(oldColumn); | |||
There was a problem hiding this comment.
maybe rename to 'checkNoRefbackExists'
| if (oldColumn != null && !newColumn.isDrop()) { | ||
| if (!created.contains(newColumn.getTableName() + "." + newColumn.getName())) { | ||
| oldTable.alterColumn(oldColumn.getName(), newColumn); | ||
| } | ||
| } else | ||
| // don't forget to add the refbacks | ||
| if (oldColumn == null && newColumn.isRefback()) { | ||
| if (oldColumn == null && !newColumn.isDrop() && newColumn.isRefback()) { |
There was a problem hiding this comment.
this a different issue/fix right ?
There was a problem hiding this comment.
Yes I sneaked it in here
|
Fixes #4091
What are the main changes you did:
how to test:
todo: