feat: add upsert methods for the drivers that support onUpdate #8104

joeflateau · 2021-08-20T21:31:16Z

Description of change

This adds EntityManager#upsert, Repository#upsert and BaseEntity.upsert

Closes: #2363

Only implements upsert for drivers that InsertQueryBuilder supports onUpdate for.

Pull-Request Checklist

Code is up-to-date with the master branch
npm run lint passes with this change
npm run test passes with this change
This pull request links relevant issues as Fixes #0000
There are new or updated unit tests validating the change
Documentation has been updated to reflect this change
The new commits follow conventions explained in CONTRIBUTING.md

This adds EntityManager#upsert, BaseEntity#upsert and EntityManager#upsert Closes: typeorm#2363

jspizziri · 2021-09-21T18:03:02Z

@imnotjames @pleerock ,

Could someone take a look at this? Would be super useful to have upsert built in.

pleerock · 2021-11-05T16:26:41Z

docs/entity-manager-api.md

@@ -147,6 +147,29 @@ await manager.update(User, 1, { firstName: "Rizzrak" });
 // executes UPDATE user SET firstName = Rizzrak WHERE id = 1
 ```

+* `upsert` - Inserts a new entity or array of entities unless they already exist in which case they are updated instead.


I think we need to left a note what databases support this method

@pleerock I've added that

pleerock · 2021-11-06T14:14:16Z

docs/entity-manager-api.md

+* `upsert` - Inserts a new entity or array of entities unless they already exist in which case they are updated instead. Supported by AuroraDataApi, Cockroach, Mysql, Postgres, and Sqlite database drivers.
+
+```typescript
+await manager.upsert(User, { externalId: "abc123" }, { firstName: "Rizzrak" });


TBH I find this signature confusing. I prefer to have only second signature:

await manager.update(User, ["externalId"], [ { externalId:"abc123", firstName: "Rizzrak" }, { externalId:"bca321", firstName: "Karzzir" }, ]);

Also I would swap parameters order, it makes more logically sense to have:

await manager.update( User, [ { externalId:"abc123", firstName: "Rizzrak" }, { externalId:"bca321", firstName: "Karzzir" }, ], ["externalId"] );

I put a lot of thought into the signature and think it is best to match the style used by https://docs.mongodb.com/manual/reference/method/db.collection.findOneAndUpdate/. In addition, since the definition is essentially upsert(Entity, ${things to match/conflict on}, ${things to update}) I felt that parameter order made more sense for the second signature. I feel strongly about keeping the first signature, but a little less strongly about the order of the second signature.

${things to match/conflict on} - but when there is no conflict, this value will be used for insertion, right? So it's actually ${things to match/conflict on/insert if no conflict merged with next parameter values} - and this is what it makes this signature complicated in understanding.

this is where i'm hung up:

await postRepository.upsert({ externalId }, { title: "Post title initial" });

vs

await postRepository.upsert({ externalId, title: "Post title initial" }, ["externalId"]);

I feel that the first signature flows significantly better than the second. The second feels unnecessarily redundant. MongoRepository#updateOne has the same basic signature and I don't believe it's complicated to understand: await mongoRepository.updateOne({ externalId }, { title: "Post title initial" }, { upsert: true });

Perhaps there's a middle ground here? Maybe rather than 1 upsert method with 2 signatures I should introduce 2 upsert methods? Repository#upsertOne(conditions, partial), a signature similar to MongoRepository#updateOne(conditions, partial, { upsert: true }) and also Repository#upsertMany(conflictPaths: string[], partialEntities: DeepPartial<Entity>[])

I really don't want to lose the MongoDb style signature.

I'm looking for both simplicity and obviousness. Adding new method won't help. Following MongoDB convention doesn't have to be there, maybe some people can appreciate it, but most prefer simplicity and obviousness. And I don't find ${things to match/conflict on/insert if no conflict merged with next parameter values} parameter obvious. Maybe because I'm not MongoDB user. I would prefer to type extra code.

Well, I don't like it, but you're in charge here. I'll update the PR with your preferred style.

sorry about that =( If you want we can wait for other people who are interested in this feature to provide their feedback.

no worries, I'm going to make the change. we can always reintroduce the second signature if feedback warrants it. thanks for everything you do for typeorm.

thank you 🤞

pleerock · 2021-11-06T14:16:03Z

docs/entity-manager-api.md

+ *  ON CONFLICT (externalId) DO UPDATE firstName = EXCLUDED.firstName
+ **/
+
+await manager.update(User, ["externalId"], [


typo, update must be upsert

all set here

…flict paths

pleerock · 2021-11-09T17:57:53Z

docs/repository-api.md

@@ -154,6 +154,22 @@ await repository.update(1, { firstName: "Rizzrak" });
 // executes UPDATE user SET firstName = Rizzrak WHERE id = 1
 ```

+* `upsert` - Inserts a new entity or array of entities unless they already exist in which case they are updated instead.


We also need to specify list of supported drivers here.

…sert

pleerock · 2021-11-09T18:03:00Z

src/util/PropertyPath.ts

@@ -0,0 +1,35 @@
+export type MaxPropertyPathRecursionDepth = 5;


This is going to be a breaking change for users since we support TypeScript versions far behind 4 where this syntax is supported. We have to use string for now, but in the future we have a strategy to use this syntax everywhere.

oh, didn't realize pre-4 was still supported :(

yeah 0.2.x is quite old and we are trying to avoid breaking changes. I hope when we finally release 0.3.0 it will use the latest TypeScript version.

let me know if I can be of assistance with 0.3.0 👍

pleerock · 2021-11-09T18:07:06Z

test/functional/repository/basic-methods/repository-basic-methods.ts

+                );
+            };
+        }
+        it("should first create then update an entity", asyncMapper(upsertableConnections(true), async (connection) => {


we need to simplify code. Instead of quite complex for understanding asyncMapper and upsertableConnections we can have a simple checks like:

if (!connection.driver.supportedUpsertType) return

…k typescript pre-4

pleerock · 2021-11-09T19:09:57Z

Thank you for your hard work!

…rm#8104) * feat: add upsert methods for the drivers that support it This adds EntityManager#upsert, BaseEntity#upsert and EntityManager#upsert Closes: typeorm#2363 * docs: Document which drivers support upsert operations * docs: fix typo in entity manager upsert many example * refactor: remove mongodb style upsert signature, enforce types of conflict paths * docs: add note to repository docs specifying which drivers support upsert * refactor: cannot staticly type conflict paths because that would break typescript pre-4 * refactor: remove test utility methods in favor of some repeated checks

AndrewKiri · 2021-11-30T20:55:33Z

Is upsert only added to the docs but not to the source code of the library? upsert is reported to have not been found in the library.

package version - "typeorm": "^0.2.34"

joeflateau · 2021-11-30T20:57:41Z

@AndrewKiri it was added in 0.2.40

AndrewKiri · 2021-11-30T21:18:27Z

@AndrewKiri it was added in 0.2.40

Ah sorry, thanks 🙏
Thanks for this amazing addition & hard work 👍

joeflateau · 2021-12-01T00:46:20Z

no prob and you're welcome! I'm happy to be able to give back to the project

feat: add upsert methods for the drivers that support it

5f37460

This adds EntityManager#upsert, BaseEntity#upsert and EntityManager#upsert Closes: typeorm#2363

joeflateau force-pushed the squash-for-upsert-pr branch from ab02d6a to 5f37460 Compare August 23, 2021 17:00

joeflateau mentioned this pull request Nov 5, 2021

chore: add a script to produce a packed tgz of the module #8110

Merged

7 tasks

pleerock reviewed Nov 5, 2021

View reviewed changes

docs: Document which drivers support upsert operations

3c0a5d9

pleerock reviewed Nov 6, 2021

View reviewed changes

joeflateau added 2 commits November 6, 2021 18:22

docs: fix typo in entity manager upsert many example

9703549

refactor: remove mongodb style upsert signature, enforce types of con…

343d0f0

…flict paths

pleerock reviewed Nov 9, 2021

View reviewed changes

docs: add note to repository docs specifying which drivers support up…

c31d0e1

…sert

pleerock reviewed Nov 9, 2021

View reviewed changes

pleerock requested changes Nov 9, 2021

View reviewed changes

joeflateau added 2 commits November 9, 2021 13:08

refactor: cannot staticly type conflict paths because that would brea…

c30408d

…k typescript pre-4

refactor: remove test utility methods in favor of some repeated checks

9e4f50f

pleerock merged commit 3f98197 into typeorm:master Nov 9, 2021

joeflateau deleted the squash-for-upsert-pr branch November 9, 2021 19:53

conoremclaughlin mentioned this pull request Jan 24, 2022

upsert with return functionality #1090

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add upsert methods for the drivers that support onUpdate #8104

feat: add upsert methods for the drivers that support onUpdate #8104

joeflateau commented Aug 20, 2021

jspizziri commented Sep 21, 2021

pleerock Nov 5, 2021

joeflateau Nov 5, 2021

pleerock Nov 6, 2021

pleerock Nov 6, 2021

joeflateau Nov 6, 2021 •

edited

pleerock Nov 8, 2021

joeflateau Nov 9, 2021

pleerock Nov 9, 2021

joeflateau Nov 9, 2021

pleerock Nov 9, 2021

joeflateau Nov 9, 2021

pleerock Nov 9, 2021

pleerock Nov 6, 2021

joeflateau Nov 6, 2021

pleerock Nov 9, 2021

joeflateau Nov 9, 2021

pleerock Nov 9, 2021

joeflateau Nov 9, 2021

pleerock Nov 9, 2021

joeflateau Nov 9, 2021

pleerock Nov 9, 2021

joeflateau Nov 9, 2021

pleerock commented Nov 9, 2021

AndrewKiri commented Nov 30, 2021

joeflateau commented Nov 30, 2021

AndrewKiri commented Nov 30, 2021 •

edited

joeflateau commented Dec 1, 2021

		@@ -0,0 +1,35 @@
		export type MaxPropertyPathRecursionDepth = 5;

feat: add upsert methods for the drivers that support onUpdate #8104

feat: add upsert methods for the drivers that support onUpdate #8104

Conversation

joeflateau commented Aug 20, 2021

Description of change

Pull-Request Checklist

jspizziri commented Sep 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeflateau Nov 6, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pleerock commented Nov 9, 2021

AndrewKiri commented Nov 30, 2021

joeflateau commented Nov 30, 2021

AndrewKiri commented Nov 30, 2021 • edited

joeflateau commented Dec 1, 2021

joeflateau Nov 6, 2021 •

edited

AndrewKiri commented Nov 30, 2021 •

edited