Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

operations on ManyToMany relations - do not use subquery when not necessary + avoid ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG error on mysql #2406

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

operations on ManyToMany relations - do not use subquery when not necessary + avoid ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG error on mysql #2406

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 69 additions & 7 deletions lib/relations/manyToMany/ManyToManyModifyMixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
'use strict';

const { ManyToManyFindOperation } = require('./find/ManyToManyFindOperation');
const { isMySql } = require('../../utils/knexUtils');

const FindByIdSelector = /^findByIds?$/;
const RelateUnrelateSelector = /relate$/;

// This mixin contains the shared code for all modify operations (update, delete, relate, unrelate)
// for ManyToManyRelation operations.
Expand All @@ -10,6 +14,8 @@ const { ManyToManyFindOperation } = require('./find/ManyToManyFindOperation');
// 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) {
Expand All @@ -26,7 +32,7 @@ const ManyToManyModifyMixin = (Operation) => {
onBuild(builder) {
this.modifyFilterSubquery = this.createModifyFilterSubquery(builder);

if (this.modifyMainQuery) {
if (this.modifyMainQuery && this.modifyFilterSubquery) {
// We can now remove the where and join statements from the main query.
this.removeFiltersFromMainQuery(builder);

Expand All @@ -38,6 +44,22 @@ const ManyToManyModifyMixin = (Operation) => {
}

createModifyFilterSubquery(builder) {
// 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()
.toFindQuery()
.modify(this.relation.modify)
.clear(RelateUnrelateSelector)
.clear(FindByIdSelector)
.clearOrder();
if (checkQuery.isSelectAll()) {
return null;
}
}

const relatedModelClass = this.relation.relatedModelClass;
const builderClass = builder.constructor;

Expand Down Expand Up @@ -73,15 +95,55 @@ const ManyToManyModifyMixin = (Operation) => {
applyModifyFilterForJoinTable(builder) {
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);
if (this.modifyFilterSubquery) {
// if subquery is used (in a non-find query)
const relatedRefs = this.relation.relatedProp.refs(builder);
const subquery = this.modifyFilterSubquery.clone().select(relatedRefs);

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
builder.parentQuery().forEachOperation(FindByIdSelector, (op) => {
if (op.name === 'findByIds') {
builder.whereInComposite(joinTableRelatedRefs, op.ids);
} else {
builder.whereComposite(joinTableRelatedRefs, op.id);
}
});
}

return builder
.whereInComposite(joinTableRelatedRefs, subquery)
.whereInComposite(joinTableOwnerRefs, ownerValues);
return builder.whereInComposite(joinTableOwnerRefs, ownerValues);
}

toFindOperation() {
Expand Down