fix: bulk load created tables and Xml columns on tedious

README.md

@@ -1213,6 +1213,8 @@ request.bulk(table, (err, result) => {
 
 **TIP**: You can also create Table variable from any recordset with `recordset.toTable()`. You can optionally specify table type name in the first argument.
 
+**TIP**: A `VarChar`, `NVarChar` or `VarBinary` column declared without a length (e.g. `sql.NVarChar`) is created as `max` when `table.create` is set, and the tedious driver loads it as `max`. It does so whether or not the table already existed, so when `table.create` is set for an existing table, give such a column its length if the table's column is sized (it loads faster) or a `sql_variant` (which cannot hold a `max` value). Without `table.create`, the tedious driver needs a column declared as `max` (e.g. `sql.NVarChar(sql.MAX)`) to bulk load into an existing `varchar(max)`, `nvarchar(max)` or `varbinary(max)` column. `Xml`, `UDT`, `Geography` and `Geometry` columns are loaded as `max` unless you give them a length.
+
 __Errors__
 - ENAME (`RequestError`) - Table name must be specified for bulk insert.
 - ETIMEOUT (`RequestError`) - Request timeout.

lib/table.js

@@ -80,14 +80,18 @@ Table.prototype._makeBulk = function _makeBulk () {
         }
         break
 
+      // these are sent as `max` values, so a column without a length is given `max`, which
+      // is how Table#declare creates it anyway
       case TYPES.Xml:
         col.type = TYPES.NVarChar(MAX).type
+        if (col.length == null) col.length = MAX
         break
 
       case TYPES.UDT:
       case TYPES.Geography:
       case TYPES.Geometry:
         col.type = TYPES.VarBinary(MAX).type
+        if (col.length == null) col.length = MAX
         break
 
       default:

lib/tedious/request.js

@@ -378,10 +378,17 @@ class Request extends BaseRequest {
           }
           bulk = connection.newBulkLoad(table.path, bulkOptions, done)
           for (const col of table.columns) {
+            const type = getTediousType(col.type)
+            // tedious resolves a missing length to 4000 or 8000, but Table#declare creates
+            // such a column as `max`, and the server will not load a sized column into a
+            // `max` one, so with table.create the column is sent as `max`, even when the
+            // table turns out to exist already. Without it the target's column types are
+            // not known here, so the column keeps the length tedious gives it.
+            const length = assertSafeTypeSize(col.length)
             // tedious emits this name as a quoted identifier of its own when it builds
             // the `insert bulk` statement, so it is checked here as well as in
             // Table#declare, in case the name changed since it was checked above.
-            bulk.addColumn(assertSafeColumnName(col.name), getTediousType(col.type), { nullable: col.nullable, length: tediousLength(assertSafeTypeSize(col.length)), scale: assertSafeTypeSize(col.scale), precision: assertSafeTypeSize(col.precision) })
+            bulk.addColumn(assertSafeColumnName(col.name), type, { nullable: col.nullable, length: length == null && table.create && MAX_LENGTH_TYPES.has(type) ? MAX : tediousLength(length), scale: assertSafeTypeSize(col.scale), precision: assertSafeTypeSize(col.precision) })
           }
 
           if (table.create) {

test/cleanup.sql

@@ -61,6 +61,24 @@ if exists (select * from sys.tables where name = 'bulk_table4')
 if exists (select * from sys.tables where name = 'bulk_table5')
 	exec('drop table [dbo].[bulk_table5]')
 
+if exists (select * from sys.tables where name = 'bulk_table_unsized')
+	exec('drop table [dbo].[bulk_table_unsized]')
+
+if exists (select * from sys.tables where name = 'bulk_table_xml')
+	exec('drop table [dbo].[bulk_table_xml]')
+
+if exists (select * from sys.tables where name = 'bulk_table_sized')
+	exec('drop table [dbo].[bulk_table_sized]')
+
+if exists (select * from sys.tables where name = 'bulk_table_variant')
+	exec('drop table [dbo].[bulk_table_variant]')
+
+if exists (select * from sys.tables where name = 'bulk_table_spatial')
+	exec('drop table [dbo].[bulk_table_spatial]')
+
+if exists (select * from sys.tables where name = 'bulk_table_hierarchy')
+	exec('drop table [dbo].[bulk_table_hierarchy]')
+
 if exists (select * from sys.tables where name = 'rowsaffected_test')
 	exec('drop table [dbo].[rowsaffected_test]')
 

test/common/tests.js

@@ -1385,6 +1385,118 @@ module.exports = (sql, driver) => {
       }).catch(done)
     },
 
+    'bulk load with columns declared without a length' (name, done) {
+      // Table#declare creates each of these columns as max (an Xml column is bulk loaded
+      // as an NVarChar(MAX))
+      const t = new sql.Table(name)
+      t.create = true
+      t.columns.add('a', sql.NVarChar, { nullable: true })
+      t.columns.add('b', sql.VarChar, { nullable: true })
+      t.columns.add('c', sql.VarBinary, { nullable: true })
+      t.columns.add('d', sql.Xml, { nullable: true })
+      t.rows.add('JP1016', 'JP1016', Buffer.from('JP1016'), '<a>JP1016</a>')
+
+      new TestRequest().bulk(t).then(result => {
+        assert.strictEqual(result.rowsAffected, 1, 'the bulk load should insert the row')
+        return new sql.Request().batch(`select * from ${name}`)
+      }).then(result => {
+        assert.strictEqual(result.recordset[0].a, 'JP1016', 'the NVarChar column should hold the loaded value')
+        assert.strictEqual(result.recordset[0].b, 'JP1016', 'the VarChar column should hold the loaded value')
+        assert.deepStrictEqual(result.recordset[0].c, Buffer.from('JP1016'), 'the VarBinary column should hold the loaded value')
+        assert.strictEqual(result.recordset[0].d, '<a>JP1016</a>', 'the Xml column should hold the loaded value')
+        done()
+      }).catch(done)
+    },
+
+    'bulk load of an Xml column into an existing xml column' (name, done) {
+      const t = new sql.Table(name)
+      t.create = false
+      t.columns.add('a', sql.Xml, { nullable: true })
+      t.rows.add('<a>JP1016</a>')
+
+      new TestRequest().bulk(t).then(result => {
+        assert.strictEqual(result.rowsAffected, 1, 'the bulk load should insert the row')
+        return new sql.Request().batch(`select * from ${name}`)
+      }).then(result => {
+        assert.strictEqual(result.recordset[0].a, '<a>JP1016</a>', 'the xml column should hold the loaded value')
+        done()
+      }).catch(done)
+    },
+
+    'bulk load of columns without a length into existing sized columns' (name, done) {
+      // without table.create the target's column types are not known, so these keep the
+      // length the driver gives them, which a sized column accepts
+      const t = new sql.Table(name)
+      t.create = false
+      t.columns.add('a', sql.NVarChar, { nullable: true })
+      t.columns.add('b', sql.VarChar, { nullable: true })
+      t.columns.add('c', sql.VarBinary, { nullable: true })
+      t.rows.add('JP1016', 'JP1016', Buffer.from('JP1016'))
+
+      new TestRequest().bulk(t).then(result => {
+        assert.strictEqual(result.rowsAffected, 1, 'the bulk load should insert the row')
+        return new sql.Request().batch(`select * from ${name}`)
+      }).then(result => {
+        assert.strictEqual(result.recordset[0].a, 'JP1016', 'the nvarchar(50) column should hold the loaded value')
+        assert.strictEqual(result.recordset[0].b, 'JP1016', 'the varchar(50) column should hold the loaded value')
+        assert.deepStrictEqual(result.recordset[0].c, Buffer.from('JP1016'), 'the varbinary(50) column should hold the loaded value')
+        done()
+      }).catch(done)
+    },
+
+    'bulk load of a column without a length into an existing sql_variant column' (name, done) {
+      // a sql_variant column cannot hold a max value, so this relies on the column keeping
+      // the driver's length too
+      const t = new sql.Table(name)
+      t.create = false
+      t.columns.add('a', sql.NVarChar, { nullable: true })
+      t.rows.add('JP1016')
+
+      new TestRequest().bulk(t).then(result => {
+        assert.strictEqual(result.rowsAffected, 1, 'the bulk load should insert the row')
+        return new sql.Request().batch(`select cast(a as nvarchar(50)) as a, cast(sql_variant_property(a, 'BaseType') as nvarchar(128)) as type from ${name}`)
+      }).then(result => {
+        assert.strictEqual(result.recordset[0].a, 'JP1016', 'the sql_variant column should hold the loaded value')
+        assert.strictEqual(result.recordset[0].type, 'nvarchar', 'the value should be stored as an nvarchar')
+        done()
+      }).catch(done)
+    },
+
+    'bulk load of spatial columns into existing geography and geometry columns' (name, done) {
+      // the values are the bytes the server serializes these types to
+      new sql.Request().query("select cast(geography::STGeomFromText('POINT(-122.36 47.656)', 4326) as varbinary(max)) as g, cast(geometry::STGeomFromText('POINT(1 2)', 0) as varbinary(max)) as m").then(result => {
+        const t = new sql.Table(name)
+        t.create = false
+        t.columns.add('g', sql.Geography, { nullable: true })
+        t.columns.add('m', sql.Geometry, { nullable: true })
+        t.rows.add(result.recordset[0].g, result.recordset[0].m)
+        return new TestRequest().bulk(t)
+      }).then(result => {
+        assert.strictEqual(result.rowsAffected, 1, 'the bulk load should insert the row')
+        return new sql.Request().batch(`select g.STAsText() as g, m.STAsText() as m from ${name}`)
+      }).then(result => {
+        assert.strictEqual(result.recordset[0].g, 'POINT (-122.36 47.656)', 'the geography column should hold the loaded point')
+        assert.strictEqual(result.recordset[0].m, 'POINT (1 2)', 'the geometry column should hold the loaded point')
+        done()
+      }).catch(done)
+    },
+
+    'bulk load of a UDT column into an existing hierarchyid column' (name, done) {
+      new sql.Request().query("select cast(hierarchyid::Parse('/1/') as varbinary(max)) as h").then(result => {
+        const t = new sql.Table(name)
+        t.create = false
+        t.columns.add('h', sql.UDT, { nullable: true })
+        t.rows.add(result.recordset[0].h)
+        return new TestRequest().bulk(t)
+      }).then(result => {
+        assert.strictEqual(result.rowsAffected, 1, 'the bulk load should insert the row')
+        return new sql.Request().batch(`select h.ToString() as h from ${name}`)
+      }).then(result => {
+        assert.strictEqual(result.recordset[0].h, '/1/', 'the hierarchyid column should hold the loaded node')
+        done()
+      }).catch(done)
+    },
+
     'parameters with length as max' (done) {
       const req = new TestRequest()
       req.input('nvarchar', sql.NVarChar('max'), 'JP1016')

test/common/unit.js

@@ -106,6 +106,25 @@ describe('Unit', () => {
     return done()
   })
 
+  it('bulk loads converted types as max', () => {
+    const t = new sql.Table('MyTable')
+    t.columns.add('x', sql.Xml, { nullable: true })
+    t.columns.add('u', sql.UDT, { nullable: true })
+    t.columns.add('g', sql.Geography, { nullable: true })
+    t.columns.add('m', sql.Geometry, { nullable: true })
+    t.columns.add('n', sql.UDT, { nullable: true, length: 50 })
+    t._makeBulk()
+
+    assert.strictEqual(t.columns[0].type, sql.NVarChar, 'an Xml column should be bulk loaded as an NVarChar')
+    assert.strictEqual(t.columns[0].length, sql.MAX, 'an Xml column should be bulk loaded as max')
+    for (const col of t.columns.slice(1, 4)) {
+      assert.strictEqual(col.type, sql.VarBinary, `the ${col.name} column should be bulk loaded as a VarBinary`)
+      assert.strictEqual(col.length, sql.MAX, `the ${col.name} column should be bulk loaded as max`)
+    }
+    assert.strictEqual(t.columns[4].length, 50, 'a length that was given should be kept')
+    assert.strictEqual(t.declare(), 'create table [MyTable] ([x] nvarchar (MAX) null, [u] varbinary (MAX) null, [g] varbinary (MAX) null, [m] varbinary (MAX) null, [n] varbinary (50) null)', 'the created columns should be the same as before')
+  })
+
   it('custom promise library', done => {
     let resolved = false
 

test/msnodesqlv8/msnodesqlv8.js

@@ -20,6 +20,10 @@ const reportsErrorsAfterAResult = major > 5 || (major === 5 && (minor > 1 || (mi
 // or polling, and lets any other run to the end
 const cancelsRunningQueries = major > 5 || (major === 5 && minor >= 6)
 
+// from 3.0 msnodesqlv8 binds a bulk column by the type of the column it loads into, and has no
+// binding for xml, sql_variant or the spatial types, which it refuses as an invalid parameter
+const itBulkLoadsAnyColumnType = major < 3 ? it : (title, fn) => it.skip(`${title} (not supported by msnodesqlv8 3 and later)`, fn)
+
 const config = function () {
   const cfg = JSON.parse(readFileSync(join(__dirname, '../.mssql.json')))
   cfg.driver = 'msnodesqlv8'
@@ -198,6 +202,12 @@ describe('msnodesqlv8', function () {
     it.skip('bulk insert of a value that is not a string into a string column throws (table) (not supported by msnodesqlv8)', done => TESTS['bulk insert of a value that is not a string into a string column throws']('bulk_table3', done))
     it('bulk insert with length option as string other than max throws (table)', done => TESTS['bulk insert with length option as string other than max throws']('bulk_table4', done))
     it('bulk insert with length as max (table)', done => TESTS['bulk insert with length as max']('bulk_table5', done))
+    it('bulk load with columns declared without a length', done => TESTS['bulk load with columns declared without a length']('bulk_table_unsized', done))
+    itBulkLoadsAnyColumnType('bulk load of an Xml column into an existing xml column', done => TESTS['bulk load of an Xml column into an existing xml column']('bulk_table_xml', done))
+    it('bulk load of columns without a length into existing sized columns', done => TESTS['bulk load of columns without a length into existing sized columns']('bulk_table_sized', done))
+    itBulkLoadsAnyColumnType('bulk load of a column without a length into an existing sql_variant column', done => TESTS['bulk load of a column without a length into an existing sql_variant column']('bulk_table_variant', done))
+    itBulkLoadsAnyColumnType('bulk load of spatial columns into existing geography and geometry columns', done => TESTS['bulk load of spatial columns into existing geography and geometry columns']('bulk_table_spatial', done))
+    it('bulk load of a UDT column into an existing hierarchyid column', done => TESTS['bulk load of a UDT column into an existing hierarchyid column']('bulk_table_hierarchy', done))
 
     after(done => sql.close(done))
   })

test/prepare.sql

@@ -165,6 +165,29 @@ exec('create table [dbo].[bulk_table] (
 	c image null
 )')
 
+exec('create table [dbo].[bulk_table_xml] (
+	a xml null
+)')
+
+exec('create table [dbo].[bulk_table_sized] (
+	a nvarchar (50) null,
+	b varchar (50) null,
+	c varbinary (50) null
+)')
+
+exec('create table [dbo].[bulk_table_variant] (
+	a sql_variant null
+)')
+
+exec('create table [dbo].[bulk_table_spatial] (
+	g geography null,
+	m geometry null
+)')
+
+exec('create table [dbo].[bulk_table_hierarchy] (
+	h hierarchyid null
+)')
+
 exec('create table [dbo].[rowsaffected_test] (
 	a int not null
 )')

test/tedious/tedious.js

@@ -194,6 +194,12 @@ describe('tedious', () => {
     it('bulk insert of a value that is not a string into a string column throws (table)', done => TESTS['bulk insert of a value that is not a string into a string column throws']('bulk_table3', done))
     it('bulk insert with length option as string other than max throws (table)', done => TESTS['bulk insert with length option as string other than max throws']('bulk_table4', done))
     it('bulk insert with length as max (table)', done => TESTS['bulk insert with length as max']('bulk_table5', done))
+    it('bulk load with columns declared without a length', done => TESTS['bulk load with columns declared without a length']('bulk_table_unsized', done))
+    it('bulk load of an Xml column into an existing xml column', done => TESTS['bulk load of an Xml column into an existing xml column']('bulk_table_xml', done))
+    it('bulk load of columns without a length into existing sized columns', done => TESTS['bulk load of columns without a length into existing sized columns']('bulk_table_sized', done))
+    it('bulk load of a column without a length into an existing sql_variant column', done => TESTS['bulk load of a column without a length into an existing sql_variant column']('bulk_table_variant', done))
+    it('bulk load of spatial columns into existing geography and geometry columns', done => TESTS['bulk load of spatial columns into existing geography and geometry columns']('bulk_table_spatial', done))
+    it('bulk load of a UDT column into an existing hierarchyid column', done => TESTS['bulk load of a UDT column into an existing hierarchyid column']('bulk_table_hierarchy', done))
     after(done => sql.close(done))
   })