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.

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

Batch Insert Optimization for MSSQL #3590

Open
jpgilchrist opened this issue Dec 19, 2019 · 1 comment
Open

Batch Insert Optimization for MSSQL #3590

jpgilchrist opened this issue Dec 19, 2019 · 1 comment

Comments

@jpgilchrist
Copy link

The batchInsert functionality is incredibly inefficient. Shouldn't it utilize the underlying driver's utilities for batch processing? I.e., for mssql it should use

https://github.com/tediousjs/node-mssql#bulk-table-options-callback

const sql = db.client.driver;

  let connection = await db.client.acquireConnection();

  const table = new sql.Table("table");

  table.columns.add("col1", sql.Int, { nullable: false });
  table.columns.add("col2", sql.VarChar(255), {
    nullable: true
  });
  items.forEach(item => {
    table.rows.add(item.col1, item.col2 );
  });
  await new Promise((resolve, reject) => {
    const request = new sql.Request(connection);
    request.bulk(table, (err, result) => {
      console.log("bulk table", err, result);
      if (err) {
        return reject(err);
      }
      return resolve(result);
    });
  });
@elhigu
Copy link
Member

elhigu commented Dec 21, 2019

IMO batch insert should not even exist in knex level, since it iis more of a helper than a single query that is built.

Anyways if this kind of optimization is implemented to mssql dialect with good integration tests that it works the same way with other drivers I don't have any reason why not to accept PR for this (if it doesn't complicate any other parts in knex).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants