Repository navigation
feat(postgres): native upsert - #12301
Conversation
1c7299d to
f0de18f
Compare
Codecov Report
@@ Coverage Diff @@
## master #12301 +/- ##
==========================================
- Coverage 96.34% 96.29% -0.05%
==========================================
Files 95 95
Lines 9127 9127
==========================================
- Hits 8793 8789 -4
- Misses 334 338 +4
Continue to review full report at Codecov.
|
SimonSchick
left a comment
There was a problem hiding this comment.
Can't say much about the query logic changes, I'm not really a postgres user, some comments on the code though.
| * @param {boolean} [options.returning=false] If true, fetches back auto generated values (Postgres only) | ||
| * @param {boolean} [options.returning=true] If true, fetches back auto generated values |
There was a problem hiding this comment.
Does this work for all 5 dialects now?
There was a problem hiding this comment.
Except for mariadb, which does not support RETURINING * at all, in current 10.1 target version. But it wont fail
There was a problem hiding this comment.
I am confused, if it is not supported by MariaDB, then why could the following code be removed:
if (options.returning === true && primaryKey) {
const record = await this.findByPk(primaryKey, options);
result = [record, created];There was a problem hiding this comment.
This works but it is buggy, see linked issues. Plus one extra read from database. So I decided to not support this for mariadb.
0c10665 to
7b174eb
Compare
|
@papb I think I have taken care of all points raised by you, I am merging this PR. Let me know if you have any questions |
Pull Request check-list
Please make sure to review and check all of these items:
npm run testornpm run test-DIALECTpass with this change (including linting)?Description of change
Closes #11246
Fixes #4132
Fixes #12051
Fixes #9629
Fixes #9216