fix(mssql): allow bulkCreate to insert more than 1000 rows in mssql (#…

…14689)
sequelize · Dec 9, 2022 · ed5f63d · ed5f63d
1 parent 151a458
commit ed5f63d
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 15 deletions.
diff --git a/src/dialects/mssql/query-generator.js b/src/dialects/mssql/query-generator.js
@@ -425,24 +425,25 @@ export class MsSqlQueryGenerator extends MsSqlQueryGeneratorTypeScript {
       }
 
       const quotedAttributes = allAttributes.map(attr => this.quoteIdentifier(attr)).join(',');
-      allQueries.push(tupleStr => `INSERT INTO ${quotedTable} (${quotedAttributes})${outputFragment} VALUES ${tupleStr};`);
+      allQueries.push(tupleStr => `INSERT INTO ${quotedTable} (${quotedAttributes})${outputFragment} VALUES ${tupleStr}`);
     }
 
     const commands = [];
     let offset = 0;
-    const batch = Math.floor(250 / (allAttributes.length + 1)) + 1;
     while (offset < Math.max(tuples.length, 1)) {
-      const tupleStr = tuples.slice(offset, Math.min(tuples.length, offset + batch));
+      // SQL Server can insert a maximum of 1000 rows at a time,
+      // This splits the insert in multiple statements to respect that limit
+      const tupleStr = tuples.slice(offset, Math.min(tuples.length, offset + 1000));
       let generatedQuery = allQueries.map(v => (typeof v === 'string' ? v : v(tupleStr))).join(';');
       if (needIdentityInsertWrapper) {
-        generatedQuery = `SET IDENTITY_INSERT ${quotedTable} ON; ${generatedQuery}; SET IDENTITY_INSERT ${quotedTable} OFF;`;
+        generatedQuery = `SET IDENTITY_INSERT ${quotedTable} ON; ${generatedQuery}; SET IDENTITY_INSERT ${quotedTable} OFF`;
       }
 
       commands.push(generatedQuery);
-      offset += batch;
+      offset += 1000;
     }
 
-    return commands.join(';');
+    return `${commands.join(';')};`;
   }
 
   updateQuery(tableName, attrValueHash, where, options = {}, attributes) {

diff --git a/test/support.ts b/test/support.ts
@@ -361,6 +361,20 @@ export function toMatchSql(sql: string) {
   return new SqlExpectation(sql);
 }
 
+class RegexExpectation extends Expectation<string> {
+  constructor(private readonly regex: RegExp) {
+    super();
+  }
+
+  assert(value: string) {
+    expect(value).to.match(this.regex);
+  }
+}
+
+export function toMatchRegex(regex: RegExp) {
+  return new RegexExpectation(regex);
+}
+
 type HasPropertiesInput<Obj extends Record<string, unknown>> = {
   [K in keyof Obj]?: any | Expectation<Obj[K]> | Error;
 };
@@ -385,7 +399,10 @@ type MaybeLazy<T> = T | (() => T);
 
 export function expectsql(
   query: MaybeLazy<{ query: string, bind: unknown } | Error>,
-  assertions: { query: PartialRecord<ExpectationKey, string | Error>, bind: PartialRecord<ExpectationKey, unknown> },
+  assertions: {
+    query: PartialRecord<ExpectationKey, string | Error>,
+    bind: PartialRecord<ExpectationKey, unknown>,
+   },
 ): void;
 export function expectsql(
   query: MaybeLazy<string | Error>,

diff --git a/test/unit/dialects/mssql/query-generator.test.js b/test/unit/dialects/mssql/query-generator.test.js
@@ -110,7 +110,7 @@ if (current.dialect.name === 'mssql') {
       // Bulk Insert With autogenerated primary key
       const attributes = { id: { autoIncrement: true } };
       expectsql(this.queryGenerator.bulkInsertQuery('myTable', [{ id: null }], {}, attributes), {
-        mssql: 'INSERT INTO [myTable] DEFAULT VALUES',
+        mssql: 'INSERT INTO [myTable] DEFAULT VALUES;',
       });
     });
 

diff --git a/test/unit/query-interface/bulk-insert.test.ts b/test/unit/query-interface/bulk-insert.test.ts
@@ -1,7 +1,8 @@
 import { expect } from 'chai';
+import range from 'lodash/range';
 import sinon from 'sinon';
 import { DataTypes } from '@sequelize/core';
-import { expectsql, sequelize } from '../../support';
+import { expectPerDialect, sequelize, toMatchRegex, toMatchSql } from '../../support';
 
 describe('QueryInterface#bulkInsert', () => {
   const User = sequelize.define('User', {
@@ -12,6 +13,38 @@ describe('QueryInterface#bulkInsert', () => {
     sinon.restore();
   });
 
+  it('uses minimal insert queries when rows <=1000', async () => {
+    const stub = sinon.stub(sequelize, 'queryRaw').resolves([[], 0]);
+
+    const users = range(1000).map(i => ({ firstName: `user${i}` }));
+    await sequelize.getQueryInterface().bulkInsert(User.tableName, users);
+
+    expect(stub.callCount).to.eq(1);
+    const firstCall = stub.getCall(0).args[0];
+
+    expectPerDialect(() => firstCall, {
+      default: toMatchRegex(/^INSERT INTO (?:`|")Users(?:`|") \((?:`|")firstName(?:`|")\) VALUES (?:\('\w+'\),){999}\('\w+'\);$/),
+      ibmi: toMatchRegex(/^SELECT \* FROM FINAL TABLE \(INSERT INTO "Users" \("firstName"\) VALUES (?:\('\w+'\),){999}\('\w+'\)\)$/),
+      mssql: toMatchRegex(/^INSERT INTO \[Users\] \(\[firstName\]\) VALUES (?:\(N'\w+'\),){999}\(N'\w+'\);$/),
+    });
+  });
+
+  it('uses minimal insert queries when rows >1000', async () => {
+    const stub = sinon.stub(sequelize, 'queryRaw').resolves([[], 0]);
+
+    const users = range(2000).map(i => ({ firstName: `user${i}` }));
+    await sequelize.getQueryInterface().bulkInsert(User.tableName, users);
+
+    expect(stub.callCount).to.eq(1);
+    const firstCall = stub.getCall(0).args[0];
+
+    expectPerDialect(() => firstCall, {
+      default: toMatchRegex(/^INSERT INTO (?:`|")Users(?:`|") \((?:`|")firstName(?:`|")\) VALUES (?:\('\w+'\),){1999}\('\w+'\);$/),
+      ibmi: toMatchRegex(/^SELECT \* FROM FINAL TABLE \(INSERT INTO "Users" \("firstName"\) VALUES (?:\('\w+'\),){1999}\('\w+'\)\)$/),
+      mssql: toMatchRegex(/^(?:INSERT INTO \[Users\] \(\[firstName\]\) VALUES (?:\(N'\w+'\),){999}\(N'\w+'\);){2}$/),
+    });
+  });
+
   // you'll find more replacement tests in query-generator tests
   it('does not parse replacements outside of raw sql', async () => {
     const stub = sinon.stub(sequelize, 'queryRaw').resolves([[], 0]);
@@ -25,13 +58,14 @@ describe('QueryInterface#bulkInsert', () => {
     });
 
     expect(stub.callCount).to.eq(1);
-    const firstCall = stub.getCall(0);
+    const firstCall = stub.getCall(0).args[0];
 
-    expectsql(firstCall.args[0] as string, {
-      default: `INSERT INTO [Users] ([firstName]) VALUES (':injection');`,
-      mssql: `INSERT INTO [Users] ([firstName]) VALUES (N':injection');`,
+    expectPerDialect(() => firstCall, {
+      default: toMatchSql('INSERT INTO "Users" ("firstName") VALUES (\':injection\');'),
+      'mysql mariadb sqlite': toMatchSql('INSERT INTO `Users` (`firstName`) VALUES (\':injection\');'),
+      mssql: toMatchSql(`INSERT INTO [Users] ([firstName]) VALUES (N':injection');`),
       // TODO: db2 should use the same system as ibmi
-      ibmi: `SELECT * FROM FINAL TABLE (INSERT INTO "Users" ("firstName") VALUES (':injection'))`,
+      ibmi: toMatchSql(`SELECT * FROM FINAL TABLE (INSERT INTO "Users" ("firstName") VALUES (':injection'))`),
     });
   });
 });
diff --git a/test/unit/sql/insert.test.js b/test/unit/sql/insert.test.js
@@ -252,7 +252,7 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
       expectsql(sql.bulkInsertQuery(M.tableName, [{ id: 0 }, { id: null }], {}, M.fieldRawAttributesMap),
         {
           query: {
-            mssql: 'SET IDENTITY_INSERT [ms] ON; INSERT INTO [ms] DEFAULT VALUES;INSERT INTO [ms] ([id]) VALUES (0),(NULL);; SET IDENTITY_INSERT [ms] OFF;',
+            mssql: 'SET IDENTITY_INSERT [ms] ON; INSERT INTO [ms] DEFAULT VALUES;INSERT INTO [ms] ([id]) VALUES (0),(NULL); SET IDENTITY_INSERT [ms] OFF;',
             postgres: 'INSERT INTO "ms" ("id") VALUES (0),(DEFAULT);',
             db2: 'INSERT INTO "ms" VALUES (1);INSERT INTO "ms" ("id") VALUES (0),(NULL);',
             ibmi: 'SELECT * FROM FINAL TABLE (INSERT INTO "ms" ("id") VALUES (0),(DEFAULT))',