Issue Creation Checklist
Feature Description
#17752 adds an opt-in parameterStyle: ParameterStyle.BIND option to Model.bulkCreate / QueryInterface#bulkInsert. The abstract bulkInsertQuery implements it, so postgres, mysql, mariadb, sqlite3, snowflake and ibmi support it. MsSqlQueryGenerator#bulkInsertQuery and Db2QueryGenerator#bulkInsertQuery are separate implementations that always inline values as literals, so those two dialects declare supports.inserts.bulkInsertParameterStyles[ParameterStyle.BIND] = false and reject a BIND request. The TODO comments next to those flags point here.
Describe the solution you'd like
Both overrides accept a bindParam collector in this.escape() the same way the abstract implementation does, return { query, bind }, and flip the capability flag to true. Things to take into account:
- mssql already splits inserts into batches of 1000 rows (
packages/mssql/src/query-generator.js, the offset += 1000 loop) and joins them into one multi-statement string. With bind parameters the whole string is one sp_executesql call, so the per-statement limit of 2100 parameters applies to the combined statement. Either lower the batch size when binding (2100 / number of columns), or issue one query per batch from the query interface instead of joining them.
- mssql
SET IDENTITY_INSERT ... ON/OFF wrapping and the OUTPUT INSERTED.* / temp-table returning path must keep working with the bind map.
- db2 uses
template() string replacement to assemble the statement, which would need to change so $sequelize_N tokens survive untouched, and SELECT * FROM FINAL TABLE (...) for returning must still wrap the bound query.
- The unit tests in
packages/core/test/unit/dialects/{mssql,db2}/query-generator.test.js and packages/core/test/unit/query-interface/bulk-insert.test.ts currently pin the "replacement only" behaviour for these dialects via the capability flag, so they only need the flag flipped plus bind expectations added.
Why should this be implemented in Sequelize?
Users pick BIND to keep values out of the SQL text and logs, to reuse execution plans, and to sidestep literal-escaping edge cases. It should not silently depend on the dialect. Until this is done, the capability flag makes the limitation explicit instead of ignoring the option.
Would you be willing to resolve this issue by submitting a Pull Request?
Issue Creation Checklist
Feature Description
#17752 adds an opt-in
parameterStyle: ParameterStyle.BINDoption toModel.bulkCreate/QueryInterface#bulkInsert. The abstractbulkInsertQueryimplements it, so postgres, mysql, mariadb, sqlite3, snowflake and ibmi support it.MsSqlQueryGenerator#bulkInsertQueryandDb2QueryGenerator#bulkInsertQueryare separate implementations that always inline values as literals, so those two dialects declaresupports.inserts.bulkInsertParameterStyles[ParameterStyle.BIND] = falseand reject aBINDrequest. TheTODOcomments next to those flags point here.Describe the solution you'd like
Both overrides accept a
bindParamcollector inthis.escape()the same way the abstract implementation does, return{ query, bind }, and flip the capability flag totrue. Things to take into account:packages/mssql/src/query-generator.js, theoffset += 1000loop) and joins them into one multi-statement string. With bind parameters the whole string is onesp_executesqlcall, so the per-statement limit of 2100 parameters applies to the combined statement. Either lower the batch size when binding (2100 / number of columns), or issue one query per batch from the query interface instead of joining them.SET IDENTITY_INSERT ... ON/OFFwrapping and theOUTPUT INSERTED.*/ temp-tablereturningpath must keep working with the bind map.template()string replacement to assemble the statement, which would need to change so$sequelize_Ntokens survive untouched, andSELECT * FROM FINAL TABLE (...)forreturningmust still wrap the bound query.packages/core/test/unit/dialects/{mssql,db2}/query-generator.test.jsandpackages/core/test/unit/query-interface/bulk-insert.test.tscurrently pin the "replacement only" behaviour for these dialects via the capability flag, so they only need the flag flipped plus bind expectations added.Why should this be implemented in Sequelize?
Users pick
BINDto keep values out of the SQL text and logs, to reuse execution plans, and to sidestep literal-escaping edge cases. It should not silently depend on the dialect. Until this is done, the capability flag makes the limitation explicit instead of ignoring the option.Would you be willing to resolve this issue by submitting a Pull Request?