From aad60aab389b357dc732e36b55e7517e48267813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Tue, 24 Jan 2023 17:29:15 +0100 Subject: [PATCH 1/5] change SQL request logic --- .../lib/entity-manager/regular-relations.js | 144 +++++++----------- 1 file changed, 59 insertions(+), 85 deletions(-) diff --git a/packages/core/database/lib/entity-manager/regular-relations.js b/packages/core/database/lib/entity-manager/regular-relations.js index 0443ce9330..a9f841b314 100644 --- a/packages/core/database/lib/entity-manager/regular-relations.js +++ b/packages/core/database/lib/entity-manager/regular-relations.js @@ -279,96 +279,70 @@ const cleanOrderColumnsForInnoDB = async ({ const { joinTable } = attribute; const { joinColumn, inverseJoinColumn, orderColumnName, inverseOrderColumnName } = joinTable; - const now = new Date().valueOf(); - const randomHex = randomBytes(16).toString('hex'); + const randomSuffix = `${new Date().valueOf()}_${randomBytes(16).toString('hex')}`; if (hasOrderColumn(attribute) && id) { - const tempOrderTableName = `orderTable_${now}_${randomHex}`; - try { - await db.connection - .raw( - ` - CREATE TABLE :tempOrderTableName: - SELECT - id, - ( - SELECT count(*) - FROM :joinTableName: b - WHERE a.:orderColumnName: >= b.:orderColumnName: AND a.:joinColumnName: = b.:joinColumnName: AND a.:joinColumnName: = :id - ) AS src_order - FROM :joinTableName: a - WHERE a.:joinColumnName: = :id - `, - { - tempOrderTableName, - joinTableName: joinTable.name, - orderColumnName, - joinColumnName: joinColumn.name, - id, - } - ) - .transacting(trx); - - // raw query as knex doesn't allow updating from a subquery - // https://github.com/knex/knex/issues/2504 - await db.connection - .raw( - `UPDATE ?? as a, (SELECT * FROM ??) AS b - SET ?? = b.src_order - WHERE a.id = b.id`, - [joinTable.name, tempOrderTableName, orderColumnName] - ) - .transacting(trx); - } finally { - await db.connection.raw(`DROP TABLE IF EXISTS ??`, [tempOrderTableName]).transacting(trx); - } + // raw query as knex doesn't allow updating from a subquery + // https://github.com/knex/knex/issues/2504 + const orderVar = `order_${randomSuffix}`; + await db.connection.raw(`SET @${orderVar} = 0;`).transacting(trx); + await db.connection + .raw( + `UPDATE :joinTableName: as a, ( + SELECT id, (@${orderVar}:=@${orderVar} + 1) AS src_order + FROM :joinTableName: + WHERE :joinColumnName: = :id + ORDER BY :orderColumnName: + ) AS b + SET :orderColumnName: = b.src_order + WHERE a.id = b.id`, + { + joinTableName: joinTable.name, + orderColumnName, + joinColumnName: joinColumn.name, + id, + } + ) + .transacting(trx); } if (hasInverseOrderColumn(attribute) && !isEmpty(inverseRelIds)) { - const tempInvOrderTableName = `invOrderTable_${now}_${randomHex}`; - try { - await db.connection - .raw( - ` - CREATE TABLE ?? - SELECT - id, - ( - SELECT count(*) - FROM ?? b - WHERE a.?? >= b.?? AND a.?? = b.?? AND a.?? IN (${inverseRelIds - .map(() => '?') - .join(', ')}) - ) AS inv_order - FROM ?? a - WHERE a.?? IN (${inverseRelIds.map(() => '?').join(', ')}) - `, - [ - tempInvOrderTableName, - joinTable.name, - inverseOrderColumnName, - inverseOrderColumnName, - inverseJoinColumn.name, - inverseJoinColumn.name, - inverseJoinColumn.name, - ...inverseRelIds, - joinTable.name, - inverseJoinColumn.name, - ...inverseRelIds, - ] - ) - .transacting(trx); - await db.connection - .raw( - `UPDATE ?? as a, (SELECT * FROM ??) AS b - SET ?? = b.inv_order - WHERE a.id = b.id`, - [joinTable.name, tempInvOrderTableName, inverseOrderColumnName] - ) - .transacting(trx); - } finally { - await db.connection.raw(`DROP TABLE IF EXISTS ??`, [tempInvOrderTableName]).transacting(trx); - } + const orderVar = `order_${randomSuffix}`; + const columnVar = `col_${randomSuffix}`; + await db.connection.raw(`SET @${orderVar} = 0;`).transacting(trx); + await db.connection + .raw( + `UPDATE ?? as a, ( + SELECT + id, + @${orderVar}:=CASE + WHEN @${columnVar} = ?? + THEN @${orderVar} + 1 + ELSE 1 + END AS inv_order, + @${columnVar}:=?? ?? + FROM + ?? a + WHERE + ?? IN(${inverseRelIds.map(() => '?').join(', ')}) + ORDER BY ??, ?? + ) AS b + SET ?? = b.inv_order + WHERE a.id = b.id`, + [ + joinTable.name, + inverseJoinColumn.name, + inverseJoinColumn.name, + inverseJoinColumn.name, + joinTable.name, + inverseJoinColumn.name, + ...inverseRelIds, + inverseJoinColumn.name, + joinColumn.name, + inverseOrderColumnName, + ] + ) + .transacting(trx); } }; From c505a62f7e924e9670dd1d32825b61adc5eb0fa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Tue, 24 Jan 2023 17:44:31 +0100 Subject: [PATCH 2/5] change WHERE clause to avoid deadlock --- .../lib/entity-manager/regular-relations.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/core/database/lib/entity-manager/regular-relations.js b/packages/core/database/lib/entity-manager/regular-relations.js index a9f841b314..0e93db2ba4 100644 --- a/packages/core/database/lib/entity-manager/regular-relations.js +++ b/packages/core/database/lib/entity-manager/regular-relations.js @@ -295,7 +295,8 @@ const cleanOrderColumnsForInnoDB = async ({ ORDER BY :orderColumnName: ) AS b SET :orderColumnName: = b.src_order - WHERE a.id = b.id`, + WHERE a.id = b.id + AND a.:joinColumnName: = :id`, { joinTableName: joinTable.name, orderColumnName, @@ -315,11 +316,7 @@ const cleanOrderColumnsForInnoDB = async ({ `UPDATE ?? as a, ( SELECT id, - @${orderVar}:=CASE - WHEN @${columnVar} = ?? - THEN @${orderVar} + 1 - ELSE 1 - END AS inv_order, + @${orderVar}:=CASE WHEN @${columnVar} = ?? THEN @${orderVar} + 1 ELSE 1 END AS inv_order, @${columnVar}:=?? ?? FROM ?? a @@ -328,7 +325,8 @@ const cleanOrderColumnsForInnoDB = async ({ ORDER BY ??, ?? ) AS b SET ?? = b.inv_order - WHERE a.id = b.id`, + WHERE a.id = b.id + AND ?? IN(${inverseRelIds.map(() => '?').join(', ')})`, [ joinTable.name, inverseJoinColumn.name, @@ -340,6 +338,8 @@ const cleanOrderColumnsForInnoDB = async ({ inverseJoinColumn.name, joinColumn.name, inverseOrderColumnName, + inverseJoinColumn.name, + ...inverseRelIds, ] ) .transacting(trx); From c9df9b7497d2e713968a6e58e5dab834758bdba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Tue, 24 Jan 2023 18:03:36 +0100 Subject: [PATCH 3/5] fix ambiguous column name --- .../core/database/lib/entity-manager/regular-relations.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/database/lib/entity-manager/regular-relations.js b/packages/core/database/lib/entity-manager/regular-relations.js index 0e93db2ba4..85c4211d68 100644 --- a/packages/core/database/lib/entity-manager/regular-relations.js +++ b/packages/core/database/lib/entity-manager/regular-relations.js @@ -318,15 +318,13 @@ const cleanOrderColumnsForInnoDB = async ({ id, @${orderVar}:=CASE WHEN @${columnVar} = ?? THEN @${orderVar} + 1 ELSE 1 END AS inv_order, @${columnVar}:=?? ?? - FROM - ?? a - WHERE - ?? IN(${inverseRelIds.map(() => '?').join(', ')}) + FROM ?? a + WHERE ?? IN(${inverseRelIds.map(() => '?').join(', ')}) ORDER BY ??, ?? ) AS b SET ?? = b.inv_order WHERE a.id = b.id - AND ?? IN(${inverseRelIds.map(() => '?').join(', ')})`, + AND a.?? IN(${inverseRelIds.map(() => '?').join(', ')})`, [ joinTable.name, inverseJoinColumn.name, From 6d92bbca8a97c8721eae185a84790939930a41d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Wed, 25 Jan 2023 11:31:23 +0100 Subject: [PATCH 4/5] use window function for mariaDB and MySQL 8 --- .../lib/entity-manager/regular-relations.js | 81 ++++++++++++------- 1 file changed, 51 insertions(+), 30 deletions(-) diff --git a/packages/core/database/lib/entity-manager/regular-relations.js b/packages/core/database/lib/entity-manager/regular-relations.js index 85c4211d68..2c64bd2841 100644 --- a/packages/core/database/lib/entity-manager/regular-relations.js +++ b/packages/core/database/lib/entity-manager/regular-relations.js @@ -198,38 +198,59 @@ const cleanOrderColumns = async ({ id, attribute, db, inverseRelIds, transaction return; } + // Handle databases that don't support window function ROW_NUMBER (here it's MySQL 5) + if (!strapi.db.dialect.supportsWindowFunctions()) { + await cleanOrderColumnsForOldDatabases({ id, attribute, db, inverseRelIds, transaction: trx }); + return; + } + + const { joinTable } = attribute; + const { joinColumn, inverseJoinColumn, orderColumnName, inverseOrderColumnName } = joinTable; + const update = []; + const updateBinding = []; + const select = ['??']; + const selectBinding = ['id']; + const where = []; + const whereBinding = []; + + if (hasOrderColumn(attribute) && id) { + update.push('?? = b.src_order'); + updateBinding.push(orderColumnName); + select.push('ROW_NUMBER() OVER (PARTITION BY ?? ORDER BY ??) AS src_order'); + selectBinding.push(joinColumn.name, orderColumnName); + where.push('?? = ?'); + whereBinding.push(joinColumn.name, id); + } + + if (hasInverseOrderColumn(attribute) && !isEmpty(inverseRelIds)) { + update.push('?? = b.inv_order'); + updateBinding.push(inverseOrderColumnName); + select.push('ROW_NUMBER() OVER (PARTITION BY ?? ORDER BY ??) AS inv_order'); + selectBinding.push(inverseJoinColumn.name, inverseOrderColumnName); + where.push(`?? IN (${inverseRelIds.map(() => '?').join(', ')})`); + whereBinding.push(inverseJoinColumn.name, ...inverseRelIds); + } + switch (strapi.db.dialect.client) { case 'mysql': - await cleanOrderColumnsForInnoDB({ id, attribute, db, inverseRelIds, transaction: trx }); + // Here it's MariaDB and MySQL 8 + await db + .getConnection() + .raw( + `UPDATE + ?? as a, + ( + SELECT ${select.join(', ')} + FROM ?? + WHERE ${where.join(' OR ')} + ) AS b + SET ${update.join(', ')} + WHERE b.id = a.id`, + [joinTable.name, ...selectBinding, joinTable.name, ...whereBinding, ...updateBinding] + ) + .transacting(trx); break; default: { - const { joinTable } = attribute; - const { joinColumn, inverseJoinColumn, orderColumnName, inverseOrderColumnName } = joinTable; - const update = []; - const updateBinding = []; - const select = ['??']; - const selectBinding = ['id']; - const where = []; - const whereBinding = []; - - if (hasOrderColumn(attribute) && id) { - update.push('?? = b.src_order'); - updateBinding.push(orderColumnName); - select.push('ROW_NUMBER() OVER (PARTITION BY ?? ORDER BY ??) AS src_order'); - selectBinding.push(joinColumn.name, orderColumnName); - where.push('?? = ?'); - whereBinding.push(joinColumn.name, id); - } - - if (hasInverseOrderColumn(attribute) && !isEmpty(inverseRelIds)) { - update.push('?? = b.inv_order'); - updateBinding.push(inverseOrderColumnName); - select.push('ROW_NUMBER() OVER (PARTITION BY ?? ORDER BY ??) AS inv_order'); - selectBinding.push(inverseJoinColumn.name, inverseOrderColumnName); - where.push(`?? IN (${inverseRelIds.map(() => '?').join(', ')})`); - whereBinding.push(inverseJoinColumn.name, ...inverseRelIds); - } - const joinTableName = addSchema(joinTable.name); // raw query as knex doesn't allow updating from a subquery @@ -267,9 +288,9 @@ const cleanOrderColumns = async ({ id, attribute, db, inverseRelIds, transaction /* * Ensure that orders are following a 1, 2, 3 sequence, without gap. - * The use of a temporary table instead of a window function makes the query compatible with MySQL 5 and prevents some deadlocks to happen in innoDB databases + * The use of a session variable instead of a window function makes the query compatible with MySQL 5 */ -const cleanOrderColumnsForInnoDB = async ({ +const cleanOrderColumnsForOldDatabases = async ({ id, attribute, db, From 64cf2c26c8401ccae5ff936583a3c0a0458dca7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Wed, 25 Jan 2023 14:12:31 +0100 Subject: [PATCH 5/5] add SQL request as a comment --- .../lib/entity-manager/regular-relations.js | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/core/database/lib/entity-manager/regular-relations.js b/packages/core/database/lib/entity-manager/regular-relations.js index 2c64bd2841..33f780330d 100644 --- a/packages/core/database/lib/entity-manager/regular-relations.js +++ b/packages/core/database/lib/entity-manager/regular-relations.js @@ -250,6 +250,20 @@ const cleanOrderColumns = async ({ id, attribute, db, inverseRelIds, transaction ) .transacting(trx); break; + /* + UPDATE + :joinTable: as a, + ( + SELECT + id, + ROW_NUMBER() OVER ( PARTITION BY :joinColumn: ORDER BY :orderColumn:) AS src_order, + ROW_NUMBER() OVER ( PARTITION BY :inverseJoinColumn: ORDER BY :inverseOrderColumn:) AS inv_order + FROM :joinTable: + WHERE :joinColumn: = :id OR :inverseJoinColumn: IN (:inverseRelIds) + ) AS b + SET :orderColumn: = b.src_order, :inverseOrderColumn: = b.inv_order + WHERE b.id = a.id; + */ default: { const joinTableName = addSchema(joinTable.name); @@ -270,17 +284,17 @@ const cleanOrderColumns = async ({ id, attribute, db, inverseRelIds, transaction .transacting(trx); /* - `UPDATE :joinTable: as a - SET :orderColumn: = b.src_order, :inverseOrderColumn: = b.inv_order - FROM ( - SELECT - id, - ROW_NUMBER() OVER ( PARTITION BY :joinColumn: ORDER BY :orderColumn:) AS src_order, - ROW_NUMBER() OVER ( PARTITION BY :inverseJoinColumn: ORDER BY :inverseOrderColumn:) AS inv_order - FROM :joinTable: - WHERE :joinColumn: = :id OR :inverseJoinColumn: IN (:inverseRelIds) - ) AS b - WHERE b.id = a.id`, + UPDATE :joinTable: as a + SET :orderColumn: = b.src_order, :inverseOrderColumn: = b.inv_order + FROM ( + SELECT + id, + ROW_NUMBER() OVER ( PARTITION BY :joinColumn: ORDER BY :orderColumn:) AS src_order, + ROW_NUMBER() OVER ( PARTITION BY :inverseJoinColumn: ORDER BY :inverseOrderColumn:) AS inv_order + FROM :joinTable: + WHERE :joinColumn: = :id OR :inverseJoinColumn: IN (:inverseRelIds) + ) AS b + WHERE b.id = a.id; */ } }