fix(postgres, sqlite): map conflictFields to column names in Model.upsert #14199
Conversation
fixes an issue where the attributes provided in conflictFields wouldn't be mapped to column names.
wbourne0
force-pushed
the
fix-upsert-conflictFields
branch
from
20f1acc
to
7e79750
Compare
src/model.js
Outdated
| options.conflictFields = options.conflictFields | ||
| && options.conflictFields.map( | ||
| attr => (this.rawAttributes[attr] && this.rawAttributes[attr].field) || attr, | ||
| ); | ||
|
|
There was a problem hiding this comment.
just a small code style change
| options.conflictFields = options.conflictFields | |
| && options.conflictFields.map( | |
| attr => (this.rawAttributes[attr] && this.rawAttributes[attr].field) || attr, | |
| ); | |
| options.conflictFields = options.conflictFields?.map( | |
| attr => (this.rawAttributes[attr]?.field) || attr, | |
| ); | |
This piece of code is used a lot in sequelize. We could add a Model.getColumnName(attributeName: string): string util which does this
There was a problem hiding this comment.
Do we want to add Model.getColumnName(attributeName: string): string in this PR already?
…x-upsert-conflictFields
…ize/sequelize into fix-upsert-conflictFields
wbourne0
changed the title
wbourne0
changed the title
…x-upsert-conflictFields
…ize/sequelize into fix-upsert-conflictFields
| @@ -2544,6 +2544,10 @@ Specify a different name for either index to resolve this issue.`); | |||
| const updateValues = Utils.mapValueFieldNames(updatedDataValues, options.fields, this); | |||
| const now = Utils.now(this.sequelize.dialect); | |||
|
|
|||
| options.conflictFields = options.conflictFields?.map( | |||
| attr => (this.rawAttributes[attr]?.field) || attr, | |||
There was a problem hiding this comment.
Since this is a breaking change, should we rename conflictFields to conflictAttributes in this PR? (see comment in #13419)
|
This pull request has been marked as maybe-abandoned and has not seen any activity for 14 days. It will be closed if no further activity occurs within the next 30 days. If you would still like to work on this, please comment to let us know and the label will be removed. |
|
Can we re-open this PR and merge? This fix can land in v7. |
|
@muraliprajapati Some further updates might need to be done in order to merge this since it's been a year since the last update. We've done quite some work internally since then. Feel free to open a new PR that adds this feature and new tests to the current codebase |
Pull Request Checklist
Please make sure to review and check all of these items:
yarn testoryarn test-DIALECTpass with this change (including linting)?Description Of Change
fixes an issue where the attributes provided in conflictFields wouldn't be mapped to column names.
Closes #14150