Skip to content

Commit

Permalink
m2m relations - do not use subquery when not necessary
Browse files Browse the repository at this point in the history
1. performance optimization for operations on m2m relations
`ManyToManyModifyMixin` creates a subquery and moves all filters from the main query into it.
> This is done so 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.

But, the subquery is not really needed in all types of operations
- e.g. a query with just a findById(s) operation - usually coming from graph upsert
- or when query builder is not operating on the join table but e.g. the target related table

2. Additionally, 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
  • Loading branch information
marcinlee authored and falkenhawk committed Apr 22, 2023
1 parent 123efe2 commit d2218da
Showing 1 changed file with 69 additions and 7 deletions.
76 changes: 69 additions & 7 deletions lib/relations/manyToMany/ManyToManyModifyMixin.js
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

0 comments on commit d2218da

Please sign in to comment.