fix: one rowcount per result, each its own, in the server's order

lib/reader.js

@@ -65,7 +65,6 @@ class Query extends BasePromises {
     // for it: what is to run once done is raised
     this.doneRaised = false
     this.afterDoneSteps = []
-    this.infoFromNextResult = false
     this.rowBatchSize = 50 /* ignored for prepared statements */
     this.currentQueryResult = null
     this.cancelled = false
@@ -185,29 +184,35 @@ class Query extends BasePromises {
     return []
   }
 
+  // what comes with the next result - info, e.g. from a PRINT, or the errors of a failed
+  // statement, whether or not the batch goes on after it - is held on it: the server sent it
+  // after the result before, which moveToNextResult completes first
   async nativeNextResult (queryId) {
     return new Promise((resolve, reject) => {
-      this.infoFromNextResult = false
       logger.debugLazy(() => `native::nextResultSet ${this.queryId}`, this.context)
       this.native.nextResultSet(this.queryId, this.notify.getHandle(), (e, res) => {
         setImmediate(() => {
-          // may contain info messages e.g. raised by PRINT statements - do not want to reject these
-          const withErrors = e ? this.dispatchInfoReturnErrors(e) : []
-          if (withErrors.length > 0 && res && !res.endOfResults) {
-            // a statement in the batch failed but the batch goes on: report and carry on
-            this.routeStatementError(withErrors, this.callback, this.notify, true)
-            resolve(res)
-          } else if (withErrors.length > 0) {
-            reject(withErrors)
+          const diagnostics = !e ? [] : Array.isArray(e) ? e : [e]
+          if (!res && diagnostics.length > 0) {
+            reject(diagnostics)
           } else {
-            this.infoFromNextResult = e != null && Array.isArray(e) && e.length > 0
+            if (diagnostics.length > 0) {
+              res.diagnostics = diagnostics
+            }
             resolve(res)
           }
         })
       })
     })
   }
 
+  // diagnostics held on a result: info is routed as info, errors with more set when the
+  // batch goes on after them
+  routeHeld (diagnostics, moreResults) {
+    const withErrors = diagnostics.some(m => !this.isInfo(m))
+    this.routeStatementError(diagnostics, this.callback, this.notify, withErrors && moreResults)
+  }
+
   async beginQuery (queryId) {
     return new Promise((resolve, reject) => {
       logger.debugLazy(() => `call query handler begin ${this.queryId}`, this.context)
@@ -312,9 +317,10 @@ class Query extends BasePromises {
     }, results, more)
   }
 
-  rowsAffected (nextResultSetInfo) {
-    const rowCount = this.currentQueryResult.rowCount
-    const moreResults = !nextResultSetInfo.endOfResults || this.infoFromNextResult
+  // the rows the result just read affected. a result with no count (-1), e.g. a PRINT or
+  // a DDL statement on its own, reports 0
+  rowsAffected (moreResults) {
+    const rowCount = Math.max(this.currentQueryResult.rowCount || 0, 0)
     this.notify.emit('rowcount', rowCount)
 
     const state = {
@@ -383,46 +389,62 @@ class Query extends BasePromises {
       }
 
       logger.debugLazy(() => `[${JSON.stringify(this.notify.getHandle())}] query ${this.queryId} moveToNextResult`, this.context)
-      const nextMeta = nextResultSetInfo.meta
-      const nextNullEmptyMeta = nextMeta == null || nextMeta.length === 0
-      const thisEmptyMeta = this.meta && this.meta.length === 0
-      if (!this.meta) {
-        this.rowsCompleted(this.metaRows(),
-          !nextResultSetInfo.endOfResults)
-      } else if (this.infoFromNextResult && nextNullEmptyMeta) {
-        this.rowsAffected(nextResultSetInfo)
-        this.nextResult()
+      const held = nextResultSetInfo.diagnostics
+      nextResultSetInfo.diagnostics = null
+      const ending = nextResultSetInfo.endOfResults
+      // a statement that failed and ended the batch: the query ends with its errors
+      const failed = ending && held != null && held.some(m => !this.isInfo(m))
+      if (failed && this.notify.getCancelSent()) {
+        // a cancel ends the query at once: cancel-cb, HY008, done, free
+        this.end(held)
         return
-      } else if (thisEmptyMeta) {
-        // handle the just finished result reading
-        // if there was no metadata, then pass the row count (rows affected)
-        this.rowsAffected(nextResultSetInfo)
+      }
+      // info that comes with the end of the batch closes the result just read
+      if (held && ending && !failed) this.routeHeld(held, false)
+      // the result just read is complete: its rows, or the rows it affected
+      const more = !ending || failed
+      if (this.meta && this.meta.length === 0) {
+        this.rowsAffected(more)
       } else {
-        this.rowsCompleted(this.metaRows(),
-          !nextResultSetInfo.endOfResults)
+        this.rowsCompleted(this.metaRows(), more)
       }
-
-      // reset for the next resultset
-      this.meta = nextResultSetInfo.meta
-      if (!this.meta) {
-        this.nextResult()
+      const next = failed
+        ? () => this.end(held)
+        : () => this.enterResult(nextResultSetInfo, held)
+      if (!more || !this.callback) {
+        next()
         return
       }
-      this.rows = []
-      if (nextResultSetInfo.endOfResults && nextResultSetInfo.endOfRows) {
-        this.close()
+      // the callback for the result just read is a tick on (the connection defers it): what
+      // the next result brings waits for it, as the server sent it after
+      setImmediate(() => {
+        if (!this.park(next)) next()
+      })
+    })
+  }
+
+  // move on to the next result: what the server sent with it, then its rows
+  enterResult (nextResultSetInfo, held) {
+    if (held && !nextResultSetInfo.endOfResults) this.routeHeld(held, true)
+    this.meta = nextResultSetInfo.meta
+    if (!this.meta) {
+      this.nextResult()
+      return
+    }
+    this.rows = []
+    if (nextResultSetInfo.endOfResults && nextResultSetInfo.endOfRows) {
+      this.close()
+    } else {
+      this.currentQueryResult = nextResultSetInfo
+      // if this is just a set of rows
+      if (this.meta.length > 0) {
+        this.notify.emit('meta', this.meta)
+        // kick off reading next set of rows
+        this.dispatch()
       } else {
-        this.currentQueryResult = nextResultSetInfo
-        // if this is just a set of rows
-        if (this.meta.length > 0) {
-          this.notify.emit('meta', this.meta)
-          // kick off reading next set of rows
-          this.dispatch()
-        } else {
-          this.nextResult()
-        }
+        this.nextResult()
       }
-    })
+    }
   }
 
   // what a native call brings back once a cancel has been sent waits for the cancel's own
@@ -511,7 +533,6 @@ class Query extends BasePromises {
 
   nextResult () {
     if (this.park(() => this.nextResult())) return
-    this.infoFromNextResult = false
     this.nativeNextResult(this.queryId)
       .then(nextResultSetInfo => this.afterCancel(() => {
         this.moveToNextResult(nextResultSetInfo)

test/compoundqueries.test.js

@@ -373,4 +373,85 @@ describe('compoundqueries', function () {
       done()
     }) // end of async.series()
   }) // end of it()
+  // one rowcount per result the server reports, in the server's order, each its own count:
+  // a result with no count (a PRINT, a lone DDL statement) reports 0. DDL, declare and set
+  // within a batch are not results at all.
+  describe('rowcount per result', function () {
+    function events (batch) {
+      return new Promise(resolve => {
+        const seen = []
+        const q = env.theConnection.query(batch)
+        q.on('rowcount', n => seen.push(`rowcount ${n}`))
+        q.on('info', i => seen.push(`info ${i.message.replace(/^.*\]/, '')}`))
+        q.on('meta', m => seen.push(`meta ${m.map(c => c.name).join(',')}`))
+        q.on('error', e => seen.push(`error ${e.message.replace(/^.*\]/, '')}`))
+        q.on('free', () => resolve(seen))
+      })
+    }
+
+    function callbacks (batch) {
+      return new Promise(resolve => {
+        const seen = []
+        env.theConnection.query(batch, (err, rows, more) => {
+          seen.push(err ? `error ${err.message.replace(/^.*\]/, '')}` : `ok ${rows.length}`)
+          if (!more) resolve(seen)
+        })
+      })
+    }
+
+    // temp tables are dropped here, as the connection may be pooled (unixODBC) and keep them
+    const dropTemp = "drop table if exists #rc; drop table if exists #t2"
+
+    this.beforeEach(async function () {
+      await env.theConnection.promises.query(`${dropTemp}; create table #rc (id int)`)
+    })
+
+    this.afterEach(async function () {
+      await env.theConnection.promises.query(dropTemp)
+    })
+
+    it('a print after an insert has its own count, after the insert count', async function () {
+      assert.deepStrictEqual(await events("insert #rc values (1); print 'p1'"),
+        ['rowcount 1', 'info p1', 'rowcount 0'])
+    })
+
+    it('a print on its own reports 0', async function () {
+      assert.deepStrictEqual(await events("print 'p1'"), ['info p1', 'rowcount 0'])
+    })
+
+    it('a print after a select follows its rows', async function () {
+      assert.deepStrictEqual(await events("select 1 as n; print 'p1'"),
+        ['meta n', 'info p1', 'rowcount 0'])
+    })
+
+    it('ddl, declare and set within a batch report nothing', async function () {
+      const batch = "insert #rc values (1), (2); create table #t (x int); drop table #t; declare @x int = 1; set @x = 2; update #rc set id = id; print 'p'; delete #rc"
+      assert.deepStrictEqual(await events(batch),
+        ['rowcount 2', 'rowcount 2', 'info p', 'rowcount 0', 'rowcount 2'])
+    })
+
+    it('a ddl statement on its own reports 0', async function () {
+      assert.deepStrictEqual(await events('create table #t2 (x int)'), ['rowcount 0'])
+    })
+
+    it('a print between selects comes between them', async function () {
+      assert.deepStrictEqual(await events("select 1 as a; print 'p1'; select 2 as b"),
+        ['meta a', 'info p1', 'rowcount 0', 'meta b'])
+    })
+
+    it('a callback for a result comes before an error that follows it', async function () {
+      assert.deepStrictEqual(await callbacks("select 1 as n; raiserror('e16', 16, 1); select 2 as m"),
+        ['ok 1', 'error e16', 'ok 0', 'ok 1'])
+    })
+
+    it('a callback for a result comes before the error that ends the batch', async function () {
+      assert.deepStrictEqual(await callbacks("select 1 as n; raiserror('e16', 16, 1)"),
+        ['ok 1', 'error e16'])
+    })
+
+    it('an insert callback comes before a print callback', async function () {
+      assert.deepStrictEqual(await callbacks("insert #rc values (1); print 'p1'; select 1 as n"),
+        ['ok 0', 'ok 0', 'ok 1'])
+    })
+  })
 })

test/contract/HANDOVER.md

@@ -32,6 +32,42 @@ the contract below. Workarounds for a missing or doubled `done` (node-mssql 12.7
 break. Then decide the node-mssql guard: an RC on `next` for dhensby, node-mssql-shaped
 scenarios, or their suite as a canary job.
 
+## Progress — 2026-10-25: one rowcount per result, in the server's order
+
+The owner's contract (2026-10-25): events follow the order the server sent them in, and each
+result the server reports raises exactly one `rowcount`, its own. A result with no count gives 0.
+
+What the server reports (raw ODBC, `SQLRowCount` after each `SQLMoreResults`, via pyodbc):
+- a PRINT, or a raiserror below severity 11, is a result of its own with no count (-1);
+- DDL, `declare` and `set` inside a batch are not results at all. The driver never stops on
+  them, so they leave no trace. A DDL statement on its own is the batch's one result, with no count.
+
+What `reader.js` did, and does now:
+- It emitted a result's rowcount as it moved on to the next. For a PRINT result it reused the
+  previous result's count, so `insert; print` gave `rowcount 1` twice. Now each result has its own
+  (`insert; print` gives `rowcount 1`, `info`, `rowcount 0`).
+- It emitted the next result's info, and a mid-batch error, before the previous result's rowcount
+  and callback. Now what comes with a result is held on it until the previous result is complete
+  (`nativeNextResult`, `moveToNextResult`, `enterResult`). In callback mode the next result
+  waits a tick for the previous result's callback, which the connection defers.
+- When an error ended the batch after a result (`select 1; throw ...`), that result's callback
+  was never called. It is now called first, with `more=true`, then the error with `more=false`.
+  The same goes for `print; raiserror(16)`: `rowcount 0` and its callback come before the error.
+  A cancel still ends the query at once: `cancel-cb`, HY008, `done`, `free`.
+
+37 transcripts changed (6 agreed among them: `errors/dup-key-b/c/d`, `print-then-raiserror`,
+`raiserror-after-result`, `throw-ends-batch`, `queue/error-after-result-*`, `scripts/issue-481`).
+The Linux pins need no change, since none of them has a result before its error. Unit tests:
+`compoundqueries.test.js` "rowcount per result". The #396 tests in `multiple-errors.test.js` now
+read to the last callback.
+
+Behaviour change for the release notes: a callback query whose batch fails after a result
+gets a callback for that result first. node-mssql pushes every `rowcount` into `rowsAffected`,
+so `insert; print` is `[1, 0]` there now (was `[1, 1]`).
+
+Still to settle: a failed statement in the middle of a batch is a result too (`rowcount 0`, and
+in callback mode an `ok rows=0` after its error). One that ends the batch is not.
+
 ## Progress — 2026-10-24: #497 (sql_variant) and #498 (numeric strings)
 
 Both reported by dhensby against 6.0.0; both reproduced here (ODBC 18), the crash included.

test/contract/corpus/errors/dup-key-b.callback.txt

@@ -6,6 +6,7 @@ submitted
 meta n
 row
 column 0 1
+cb ok rows=1 more=true
 cb error 23000 2627 Violation of PRIMARY KEY constraint 'pk_t'. Cannot insert duplicate key in object 'dbo.#t'. The duplicate key value is (1). more=false
 info 01000 3621 The statement has been terminated.
 done

test/contract/corpus/errors/dup-key-c.callback.txt

@@ -6,12 +6,12 @@ submitted
 meta n
 row
 column 0 1
+cb ok rows=1 more=true
 cb error 23000 2627 Violation of PRIMARY KEY constraint 'pk_t'. Cannot insert duplicate key in object 'dbo.#t'. The duplicate key value is (1). more=true
 info 01000 3621 The statement has been terminated.
-cb ok rows=1 more=true
 rowcount 0
-meta m
 cb ok rows=0 more=true
+meta m
 row
 column 0 2
 cb ok rows=1 more=false

test/contract/corpus/errors/dup-key-d.callback.txt

@@ -6,8 +6,8 @@ submitted
 cb error 23000 2627 Violation of PRIMARY KEY constraint 'pk_t'. Cannot insert duplicate key in object 'dbo.#t'. The duplicate key value is (1). more=true
 info 01000 3621 The statement has been terminated.
 rowcount 0
-meta m
 cb ok rows=0 more=true
+meta m
 row
 column 0 2
 cb ok rows=1 more=false

test/contract/corpus/errors/print-then-raiserror.callback.txt

@@ -3,6 +3,8 @@
 # sql: print 'p1'; raiserror('boom', 16, 1)
 submitted
 info 01000 0 p1
+rowcount 0
+cb ok rows=0 more=true
 cb error 42000 50000 boom more=false
 done
 free

test/contract/corpus/errors/print-then-raiserror.events.txt

@@ -3,6 +3,7 @@
 # sql: print 'p1'; raiserror('boom', 16, 1)
 submitted
 info 01000 0 p1
+rowcount 0
 error 42000 50000 boom more=false
 done
 free

test/contract/corpus/errors/raiserror-after-result.callback.txt

@@ -5,6 +5,7 @@ submitted
 meta n
 row
 column 0 1
+cb ok rows=1 more=true
 cb error 42000 50000 boom more=false
 done
 free

test/contract/corpus/errors/raiserror-then-result.callback.txt

@@ -4,8 +4,8 @@
 submitted
 cb error 42000 50000 boom more=true
 rowcount 0
-meta m
 cb ok rows=0 more=true
+meta m
 row
 column 0 2
 cb ok rows=1 more=false

test/contract/corpus/errors/throw-ends-batch.callback.txt

@@ -5,6 +5,7 @@ submitted
 meta n
 row
 column 0 1
+cb ok rows=1 more=true
 cb error 42000 50001 thrown more=false
 done
 free

test/contract/corpus/errors/two-raiserrors-then-result.callback.txt

@@ -3,6 +3,8 @@
 # sql: print 'p1'; raiserror('e1', 16, 1); print 'p2'; raiserror('e2', 16, 1); select 1 as n
 submitted
 info 01000 0 p1
+rowcount 0
+cb ok rows=0 more=true
 cb error 42000 50000 e1 more=true
 rowcount 0
 cb ok rows=0 more=true
@@ -12,9 +14,7 @@ cb ok rows=0 more=true
 cb error 42000 50000 e2 more=true
 rowcount 0
 cb ok rows=0 more=true
-rowcount 0
 meta n
-cb ok rows=0 more=true
 row
 column 0 1
 cb ok rows=1 more=false