streamline changes, better descriptions

Vincit · Apr 22, 2023 · 63230f6 · 63230f6
1 parent 4ddbc24
commit 63230f6
Showing 1 changed file with 35 additions and 62 deletions.
diff --git a/lib/relations/manyToMany/ManyToManyModifyMixin.js b/lib/relations/manyToMany/ManyToManyModifyMixin.js
@@ -14,6 +14,8 @@ const RelateUnrelateSelector = /relate$/;
 // that we are able to `innerJoin` the join table to the query. Most SQL engines don't allow
 // joins in updates or deletes. Join table is joined so that queries can reference the join
 // table columns.
+//
+// If the subquery is not needed at all (e.g. the query has only a findById(s) operation - usually coming from graph upsert) - skip it
 const ManyToManyModifyMixin = (Operation) => {
   return class extends Operation {
     constructor(...args) {
@@ -42,8 +44,9 @@ const ManyToManyModifyMixin = (Operation) => {
     }
 
     createModifyFilterSubquery(builder) {
-      // Check if the subquery is needed (it may be not if there are no operations other than findById(s) on the main query)
-      // and only if passed builder belongs to joinTableModelClass
+      // Check if the subquery is needed
+      // - it may not be, if there are no operations other than findById(s) on the main query
+      // and proceed only if passed builder operates on the joinTable
       if (builder.modelClass() === this.relation.joinTableModelClass) {
         const checkQuery = builder
           .clone()
@@ -90,74 +93,44 @@ const ManyToManyModifyMixin = (Operation) => {
     }
 
     applyModifyFilterForJoinTable(builder) {
-      const builderWithTriggerFix = this.applyManyToManyRelationTriggerFix(builder);
-      // null here means fix is not applicable
-      if (builderWithTriggerFix !== null) {
-        return builderWithTriggerFix;
-      }
-
-      const joinTableOwnerRefs = this.relation.joinTableOwnerProp.refs(builder);
-      const joinTableRelatedRefs = this.relation.joinTableRelatedProp.refs(builder);
-
-      const relatedRefs = this.relation.relatedProp.refs(builder);
-      const ownerValues = this.owner.getProps(this.relation);
-
-      const subquery = this.modifyFilterSubquery.clone().select(relatedRefs);
-
-      return builder
-        .whereInComposite(joinTableRelatedRefs, subquery)
-        .whereInComposite(joinTableOwnerRefs, ownerValues);
-    }
-
-    /**
-     * Workaround for ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG mysql error
-     * when a trigger on join table was operating on the related table
-     * - targeting mysql only
-     * - we return null if this fix is not applicable!
-     * - filters/modify on join table of m2m relations
-     * - if subquery is not needed at all (e.g. a query with just a findById(s) operation - usually coming from graph upsert) - skip it
-     * - otherwise extract a subquery reading related ids to separate query run before the delete query for m2m unrelate operation
-     *
-     * This is an upgraded (to objection v3) version of:
-     * - https://github.com/ovos/objection.js/pull/3
-     * - https://github.com/ovos/objection.js/pull/1
-     * Originally based on:
-     * - https://github.com/ovos/objection.js/pull/2
-     */
-    applyManyToManyRelationTriggerFix(builder) {
-      // this workaround is only needed for MySQL
-      if (!isMySql(builder.knex())) {
-        return null;
-      }
-
-      if (this.modifyFilterSubquery && builder.isFind()) {
-        return null;
-      }
-
       const joinTableOwnerRefs = this.relation.joinTableOwnerProp.refs(builder);
       const joinTableRelatedRefs = this.relation.joinTableRelatedProp.refs(builder);
       const ownerValues = this.owner.getProps(this.relation);
 
       if (this.modifyFilterSubquery) {
-        // if subquery is used (in a non-find query):
-        // extract the subquery selecting related ids to separate query run before the main query
-        // to avoid ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG mysql error
-        // when executing a db trigger on a join table which updates related table
+        // if subquery is used (in a non-find query)
         const relatedRefs = this.relation.relatedProp.refs(builder);
         const subquery = this.modifyFilterSubquery.clone().select(relatedRefs);
 
-        builder
-          .runBefore(() => subquery.execute())
-          .runBefore((related, builder) => {
-            if (!related.length) {
-              builder.resolve([]);
-              return;
-            }
-            builder.whereInComposite(
-              joinTableRelatedRefs,
-              related.map((m) => m.$values(this.relation.relatedProp.props))
-            );
-          });
+        if (isMySql(builder.knex())) {
+          // and only for mysql:
+          // extract the subquery selecting related ids to separate query run before the main query
+          // to avoid ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG mysql error
+          // when executing a db trigger on a join table which updates related table
+          //
+          // This workaround is only needed for MySQL.
+          // It could possibly be applied to all DBMS, if proven necessary,
+          // but others seem to handle such cases just fine.
+          //
+          // https://stackoverflow.com/a/2314264/3729316
+          // "MySQL triggers can't manipulate the table they are assigned to.
+          // All other major DBMS support this feature so hopefully MySQL will add this support soon."
+          // ~ Cory House, 2010
+          builder
+            .runBefore(() => subquery.execute())
+            .runBefore((related, builder) => {
+              if (!related.length) {
+                builder.resolve([]);
+                return;
+              }
+              builder.whereInComposite(
+                joinTableRelatedRefs,
+                related.map((m) => m.$values(this.relation.relatedProp.props))
+              );
+            });
+        } else {
+          builder.whereInComposite(joinTableRelatedRefs, subquery);
+        }
       } else if (builder.parentQuery()) {
         // if subquery is not used:
         // rewrite findById(s) from related table to join table