[IMP] _change_foreign_key_refs: Add extra_where params#246
Conversation
|
Being used in OCA/OpenUpgrade#2379 |
This comment has been minimized.
This comment has been minimized.
pedrobaeza
force-pushed
the
_change_foreign_key_refs-extra_where
branch
2 times, most recently
from
daba54f to
b2712d3
Compare
|
@StefanRijnhart do you oppose to this change, as you were the original author of the method? |
| env.cr.execute('ROLLBACK TO SAVEPOINT sp1') | ||
| if error.pgcode != UNIQUE_VIOLATION: | ||
| raise | ||
| elif error.pgcode == UNDEFINED_COLUMN: |
There was a problem hiding this comment.
Isn't the exception raised already at this point?
Also, I would prefer that you check if extra_where is actually set before you assume it's causing the error here.
There was a problem hiding this comment.
I don't get your first sentence, but yes, we can check if extra_where is set. Does this looks good to you?:
| elif error.pgcode == UNDEFINED_COLUMN: | |
| elif error.pgcode == UNDEFINED_COLUMN: | |
| if not extra_where: | |
| raise |
There was a problem hiding this comment.
Yes, agree with that.
What I mean with my first sentence, is that if error.pgcode == UNDEFINED_COLUMN is true, then error.pgcode != UNIQUE_VIOLATION is also true in which case line 60 raises before your code can take effect.
There was a problem hiding this comment.
I get you. What about now?
Adding the new argument `extra_where`, we are able to change a FK reference depending on an extra condition, so not all ocurrences are replaced. This is useful for example when we have a unique record for all companies, and on new version, there should be one record per company. We perform the split, and we need to replace references for only a specific company (extra_where="AND company_id = X"). Retro-compatibility is kept as the argument is optional, and a safeguard has been introduced for skipping models where the condition can't be fulfilled if the field doesn't exist (so we are not able to say if the reference should be replaced or not). TT28115
pedrobaeza
force-pushed
the
_change_foreign_key_refs-extra_where
branch
from
b2712d3 to
86e621f
Compare
|
This PR has the |
5 participants
Adding the new argument
extra_where, we are able to change a FK reference depending on an extra condition, so not all ocurrences are replaced.This is useful for example when we have a unique record for all companies, and on new version, there should be one record per company. We perform the split, and we need to replace references for only a specific company (extra_where="AND company_id = X").
Retro-compatibility is kept as the argument is optional, and a safeguard has been introduced for skipping models where the condition can't be fulfilled if the field doesn't exist (so we are not able to say if the reference should be replaced or not).
@Tecnativa TT28115