fix: numeric strings read decimal, money and smallmoney as exact text (#498)

cpp/include/odbc/odbc_statement_legacy.h

@@ -151,6 +151,7 @@ class OdbcStatementLegacy : public IOdbcStatement {
   SQLRETURN poll_check(SQLRETURN ret, shared_ptr<vector<uint16_t>> vec, const bool direct);
   bool get_data_binary(size_t row_id, size_t column);
   bool get_data_decimal(size_t row_id, size_t column);
+  bool get_data_numeric_string(size_t row_id, size_t column);
   bool get_data_numeric(size_t row_id, size_t column);
   bool get_data_bit(size_t row_id, size_t column);
   bool get_data_timestamp(size_t row_id, size_t column);

cpp/src/odbc/odbc_statement_legacy.cpp

@@ -1458,15 +1458,17 @@ bool OdbcStatementLegacy::dispatch(
       }
       break;
 
+    // decimal (and money, smallmoney) is numeric under another name: under numeric strings
+    // both are the driver's exact text, never a double (#498)
     case SQL_NUMERIC:
+    case SQL_DECIMAL:
       if (_numericStringEnabled) {
-        res = try_read_string(false, row_id, column);
+        res = get_data_numeric_string(row_id, column);
       } else {
         res = get_data_decimal(row_id, column);
       }
       break;
 
-    case SQL_DECIMAL:
     case SQL_REAL:
     case SQL_FLOAT:
     case SQL_DOUBLE:
@@ -1976,6 +1978,43 @@ bool OdbcStatementLegacy::get_data_decimal(const size_t row_id, const size_t col
   return true;
 }
 
+// a numeric or decimal as the driver's text, which keeps every digit and the scale ("5.00").
+// the driver leaves out the zero before the point ("-.50"); it is put back ("-0.50").
+bool OdbcStatementLegacy::get_data_numeric_string(const size_t row_id, const size_t column) {
+  // 38 digits, a sign and a point
+  constexpr SQLLEN capacity = 64;
+  constexpr auto unit = static_cast<SQLLEN>(sizeof(uint16_t));
+  std::vector<uint16_t> text(capacity, 0);
+  SQLLEN value_len = 0;
+  const auto ret = _odbcApi->SQLGetData(_statement->get_handle(),
+                                        static_cast<SQLSMALLINT>(column + 1),
+                                        SQL_C_WCHAR,
+                                        text.data(),
+                                        capacity * unit,
+                                        &value_len);
+  if (!check_odbc_error(ret)) {
+    SQL_LOG_DEBUG_STREAM("get_data_numeric_string failed to get data");
+    return false;
+  }
+  if (value_len == SQL_NULL_DATA) {
+    _resultset->add_column(row_id, make_shared<NullColumn>(column));
+    return true;
+  }
+  auto len = static_cast<size_t>(value_len / unit);
+  if (len >= static_cast<size_t>(capacity)) {
+    len = static_cast<size_t>(capacity) - 1;
+  }
+  text.resize(len);
+  const auto sign = !text.empty() && text[0] == u'-' ? 1 : 0;
+  if (text.size() > static_cast<size_t>(sign) && text[sign] == u'.') {
+    text.insert(text.begin() + sign, u'0');
+  }
+  const auto size = text.size();
+  const auto storage = make_shared<std::vector<uint16_t>>(std::move(text));
+  _resultset->add_column(row_id, make_shared<StringColumn>(column, storage, 0, size));
+  return true;
+}
+
 bool OdbcStatementLegacy::get_data_binary(const size_t row_id, const size_t column) {
   auto storage = make_shared<DatumStorageLegacy>();
 

lib/index.d.ts

@@ -754,7 +754,9 @@ declare namespace MsNodeSqlV8 {
     id: number
     /**
      * optionally return all number based columns as strings
-     * to prevent exceeding max numeric for JS
+     * to prevent exceeding max numeric for JS. numeric, decimal, money and
+     * smallmoney are the server's exact text, with their scale and a leading
+     * zero ('5.00', '-0.50'); float and real are the text String(n) gives.
      * @param numericString boolean true for numers as strings.
      */
     setUseNumericString: (numericString: boolean) => void

test/contract/HANDOVER.md

@@ -49,11 +49,13 @@ Repros in `repro/i497.js`, `repro/i498.js`; corpus `scripts/issue-497` (+ `-ns`,
 - **bigint max read as a double** came back as INT64_MIN: the int64 range check used bounds
   that round to 2^63. Bounds exact as doubles now.
 - **#498 part 1 fixed:** a double under numeric strings is `String(n)`'s text (`"1e-10"`), not `%f`.
-- **#498 part 2 open, the owner's call:** `decimal`, `money` and `smallmoney` are still read as a
-  double, so under numeric strings digits beyond a double are lost (`decimal(38,2)` with 20
-  integer digits, large `money`); a prepared statement binds `numeric` as a double too. The fix,
-  reading `SQL_DECIMAL` as text like `SQL_NUMERIC`, changes what users get: the driver's text
-  keeps the scale (`"5.00"`) and has no leading zero (`".0000001"`, as `numeric` already does).
+- **#498 part 2 fixed (owner's call: option 1, no new config):** under numeric strings `decimal`,
+  `money` and `smallmoney` are read as the driver's text, as `numeric` is - they are the same
+  type to SQL Server, and gave different text ("5" against "5.00"). Both now get the leading zero
+  the driver leaves out ("-0.50", not "-.50"). Corpus `scripts/issue-498` agreed.
+- **#498 still open: prepared statements** bind `numeric`/`decimal` result columns as a double
+  (`bound_datum.cpp` reserve, `reserved_decimal`), so they still lose digits under numeric
+  strings. Pinned `known-bug` in `scripts/issue-498-prepared`; needs a text binding.
 - The replay keeps whole rows for scripts now (`run-one.js`), so a script's column values replay.
   A transcript stops at 400 lines: split a script that needs more (as `issue-497-ns`).
 

test/contract/corpus/scripts/issue-498-prepared.txt

@@ -0,0 +1,20 @@
+# scenario: scripts/issue-498-prepared
+# status: known-bug
+# script: test/contract/scripts/issue-498-prepared.js
+# #498: a prepared statement binds numeric and decimal result columns as a double, so column 1
+# loses digits (should be "12345678901234567.89")
+setup: submitted
+setup: rowcount 1
+setup: cb ok rows=0 more=false
+setup: done
+setup: free
+== prepared, numeric strings on
+ps: submitted
+ps: meta n,d,f
+ps: row
+ps: column 0 "1.0123456789"
+ps: column 1 "12345678901234568"
+ps: column 2 "1e-10"
+ps: done
+ps: free
+check ok

test/contract/corpus/scripts/issue-498.txt

@@ -1,9 +1,6 @@
 # scenario: scripts/issue-498
-# status: known-bug
+# status: agreed
 # script: test/contract/scripts/issue-498.js
-# #498: doubles now print as String(n) does. still open: decimal and money are read as a double,
-# so digits a double cannot hold are lost (c6, c8, c10), and a prepared statement binds numeric
-# as a double too (ps column 1). numeric's text has no leading zero (c14 ".0000001")
 -- c0 cast(0.1234567890123 as float); c1 cast(1e-10 as float); c2 cast(1.5e300 as float); c3 cast(-2.5 as float); c4 cast(0.12345678 as real); c5 cast(1.0123456789 as decimal(38,10)); c6 cast(12345678901234567890.12 as decimal(38,2)); c7 cast(0.0000001 as decimal(10,7)); c8 cast(9007199254740993 as decimal(19,0)); c9 cast(709 as decimal(10,0)); c10 cast(922337203685477.5807 as money); c11 cast(-0.0001 as money); c12 cast(1.2345 as smallmoney); c13 cast(1.0123456789 as numeric(38,10)); c14 cast(0.0000001 as numeric(10,7)); c15 cast(9223372036854775807 as bigint)
 == numeric strings off
 plain: submitted
@@ -37,30 +34,16 @@ ns: column 2 "1.5e+300"
 ns: column 3 "-2.5"
 ns: column 4 "0.12345678359270096"
 ns: column 5 "1.0123456789"
-ns: column 6 "12345678901234567000"
-ns: column 7 "1e-7"
-ns: column 8 "9007199254740992"
+ns: column 6 "12345678901234567890.12"
+ns: column 7 "0.0000001"
+ns: column 8 "9007199254740993"
 ns: column 9 "709"
-ns: column 10 "922337203685477.6"
+ns: column 10 "922337203685477.5807"
 ns: column 11 "-0.0001"
 ns: column 12 "1.2345"
 ns: column 13 "1.0123456789"
-ns: column 14 ".0000001"
+ns: column 14 "0.0000001"
 ns: column 15 "9223372036854775807"
 ns: done
 ns: free
-setup: submitted
-setup: rowcount 1
-setup: cb ok rows=0 more=false
-setup: done
-setup: free
-== prepared, numeric strings on
-ps: submitted
-ps: meta n,d,f
-ps: row
-ps: column 0 "1.0123456789"
-ps: column 1 "12345678901234568"
-ps: column 2 "1e-10"
-ps: done
-ps: free
 check ok

test/contract/native/scripts/issue-498-prepared.win32.json

@@ -0,0 +1 @@
+{"scenario":"scripts/issue-498-prepared","transcript":["setup: submitted","setup: rowcount 1","setup: cb ok rows=0 more=false","setup: done","setup: free","== prepared, numeric strings on","ps: submitted","ps: meta n,d,f","ps: row","ps: column 0 \"1.0123456789\"","ps: column 1 \"12345678901234568\"","ps: column 2 \"1e-10\"","ps: done","ps: free","check ok"],"log":[{"ev":"call","id":0,"t":11.7,"name":"open","args":["<connection string>",0]},{"ev":"return","of":0,"t":31.7,"result":[null,{"success":true}]},{"ev":"call","id":2,"t":33.5,"name":"query","args":[0,{"query_str":"create table #p (n numeric(38,10), d decimal(38,2), f float); insert #p values (1.0123456789, 12345678901234567.89, 1e-10)","query_timeout":0,"query_polling":false,"query_tz_adjustment":0,"numeric_string":false,"bigint_as_native":false,"stateChangeCallback":{"$":"function"}},[]]},{"ev":"state","of":2,"t":33.7,"change":{"handle":{"connectionId":0,"statementId":0},"oldState":"STATEMENT_CREATED","newState":"STATEMENT_BINDING"}},{"ev":"state","of":2,"t":33.8,"change":{"handle":{"connectionId":0,"statementId":0},"oldState":"STATEMENT_BINDING","newState":"STATEMENT_SUBMITTED"}},{"ev":"state","of":2,"t":35,"change":{"handle":{"connectionId":0,"statementId":0},"oldState":"STATEMENT_SUBMITTED","newState":"STATEMENT_READING"}},{"ev":"return","of":2,"t":35,"result":[null,{"meta":[],"handle":{"connectionId":0,"statementId":0},"endOfRows":true,"endOfResults":false,"rowCount":1},true]},{"ev":"call","id":7,"t":35.2,"name":"nextResultSet","args":[0,{"connectionId":0,"statementId":0}]},{"ev":"return","of":7,"t":35.3,"result":[null,{"meta":[],"handle":{"connectionId":0,"statementId":0},"endOfRows":true,"endOfResults":true,"rowCount":1}]},{"ev":"call","id":9,"t":35.6,"name":"releaseStatement","args":[0,{"connectionId":0,"statementId":0}]},{"ev":"state","of":2,"t":35.7,"change":{"handle":{"connectionId":0,"statementId":0},"oldState":"STATEMENT_READING","newState":"STATEMENT_CLOSED"}},{"ev":"return","of":9,"t":35.7,"result":[null,true]},{"ev":"call","id":12,"t":187.8,"name":"prepare","args":[1,{"query_str":"select n, d, f from #p where 1 = ?","numeric_string":true,"bigint_as_native":false,"stateChangeCallback":{"$":"function"}}]},{"ev":"state","of":12,"t":189.5,"change":{"handle":{"connectionId":0,"statementId":1},"oldState":"STATEMENT_CREATED","newState":"STATEMENT_PREPARED"}},{"ev":"return","of":12,"t":189.5,"result":[null,{"meta":[{"name":"n","nullable":true,"type":"number","sqlType":"numeric","size":38},{"name":"d","nullable":true,"type":"number","sqlType":"decimal","size":38},{"name":"f","nullable":true,"type":"number","sqlType":"float","size":53}],"handle":{"connectionId":0,"statementId":1},"endOfRows":true,"endOfResults":false,"rowCount":0}]},{"ev":"call","id":15,"t":189.8,"name":"bindQuery","args":[1,[1]]},{"ev":"state","of":12,"t":189.9,"change":{"handle":{"connectionId":0,"statementId":1},"oldState":"STATEMENT_PREPARED","newState":"STATEMENT_SUBMITTED"}},{"ev":"return","of":15,"t":193.5,"result":[null,{"meta":[{"name":"n","nullable":true,"type":"number","sqlType":"numeric","size":38},{"name":"d","nullable":true,"type":"number","sqlType":"decimal","size":38},{"name":"f","nullable":true,"type":"number","sqlType":"float","size":53}],"handle":{"connectionId":0,"statementId":1},"endOfRows":false,"endOfResults":false,"rowCount":0}]},{"ev":"call","id":18,"t":193.7,"name":"fetchRows","args":[1,{"connectionId":0,"statementId":1},{"asArrays":true,"batchSize":50,"asObjects":false}]},{"ev":"return","of":18,"t":194.5,"result":[null,{"endOfRows":false,"endOfResults":false,"rowCount":1,"data":[["1.0123456789","12345678901234568","1e-10"]]}]},{"ev":"call","id":20,"t":194.7,"name":"fetchRows","args":[1,{"connectionId":0,"statementId":1},{"asArrays":true,"batchSize":50,"asObjects":false}]},{"ev":"return","of":20,"t":194.7,"result":[null,{"endOfRows":true,"endOfResults":false,"rowCount":0,"data":[]}]},{"ev":"call","id":22,"t":194.8,"name":"nextResultSet","args":[1,{"connectionId":0,"statementId":1}]},{"ev":"return","of":22,"t":194.9,"result":[null,{"meta":[{"name":"n","nullable":true,"type":"number","sqlType":"numeric","size":38},{"name":"d","nullable":true,"type":"number","sqlType":"decimal","size":38},{"name":"f","nullable":true,"type":"number","sqlType":"float","size":53}],"handle":{"connectionId":0,"statementId":1},"endOfRows":true,"endOfResults":true,"rowCount":0}]},{"ev":"call","id":24,"t":195,"name":"releaseStatement","args":[1,{"connectionId":0,"statementId":1}]},{"ev":"state","of":12,"t":195,"change":{"handle":{"connectionId":0,"statementId":1},"oldState":"STATEMENT_SUBMITTED","newState":"STATEMENT_CLOSED"}},{"ev":"return","of":24,"t":195.5,"result":[null,true]},{"ev":"call","id":27,"t":346.3,"name":"query","args":[2,{"query_str":"select 1 as n","query_timeout":0,"query_polling":false,"query_tz_adjustment":0,"numeric_string":false,"bigint_as_native":false,"stateChangeCallback":{"$":"function"}},[]]},{"ev":"state","of":27,"t":346.4,"change":{"handle":{"connectionId":0,"statementId":2},"oldState":"STATEMENT_CREATED","newState":"STATEMENT_BINDING"}},{"ev":"state","of":27,"t":346.4,"change":{"handle":{"connectionId":0,"statementId":2},"oldState":"STATEMENT_BINDING","newState":"STATEMENT_SUBMITTED"}},{"ev":"state","of":27,"t":347.4,"change":{"handle":{"connectionId":0,"statementId":2},"oldState":"STATEMENT_SUBMITTED","newState":"STATEMENT_READING"}},{"ev":"return","of":27,"t":347.5,"result":[null,{"meta":[{"name":"n","nullable":false,"type":"number","sqlType":"int","size":10}],"handle":{"connectionId":0,"statementId":2},"endOfRows":true,"endOfResults":false,"rowCount":0},true]},{"ev":"call","id":32,"t":347.6,"name":"fetchRows","args":[2,{"connectionId":0,"statementId":2},{"asArrays":true,"batchSize":50,"asObjects":false}]},{"ev":"return","of":32,"t":347.6,"result":[null,{"endOfRows":true,"endOfResults":false,"rowCount":0,"data":[[1]]}]},{"ev":"call","id":34,"t":347.7,"name":"nextResultSet","args":[2,{"connectionId":0,"statementId":2}]},{"ev":"return","of":34,"t":347.8,"result":[null,{"meta":[{"name":"n","nullable":false,"type":"number","sqlType":"int","size":10}],"handle":{"connectionId":0,"statementId":2},"endOfRows":true,"endOfResults":true,"rowCount":0}]},{"ev":"call","id":36,"t":347.9,"name":"releaseStatement","args":[2,{"connectionId":0,"statementId":2}]},{"ev":"state","of":27,"t":347.9,"change":{"handle":{"connectionId":0,"statementId":2},"oldState":"STATEMENT_READING","newState":"STATEMENT_CLOSED"}},{"ev":"return","of":36,"t":347.9,"result":[null,true]},{"ev":"call","id":39,"t":348.1,"name":"close","args":[]},{"ev":"return","of":39,"t":348.2,"result":[null,true]}]}

test/contract/scripts/issue-498-prepared.js

@@ -0,0 +1,18 @@
+'use strict'
+
+// #498 through a prepared statement: it binds numeric, decimal and float result columns as a
+// double, so under numeric strings numeric and decimal lose what a double cannot hold.
+
+module.exports = async t => {
+  t.query('setup', 'create table #p (n numeric(38,10), d decimal(38,2), f float); ' +
+    'insert #p values (1.0123456789, 12345678901234567.89, 1e-10)', null, { callback: true })
+  await t.settled('setup')
+  const ps = await new Promise((resolve, reject) => {
+    t.conn.prepare({ query_str: 'select n, d, f from #p where 1 = ?', numeric_string: true },
+      (err, p) => err ? reject(err) : resolve(p))
+  })
+  t.section('prepared, numeric strings on')
+  t.watch('ps', ps.preparedQuery([1]), { columns: true })
+  await t.next('ps', 'done')
+  await new Promise(resolve => ps.free(resolve))
+}

test/contract/scripts/issue-498.js

@@ -2,8 +2,8 @@
 
 // #498: under numeric strings, float, real, decimal and money were printed with %f (six
 // decimal places, no exponent): 1e-10 came back as "0.000000". each value is read with
-// numeric strings off and on; the strings should keep the value. then the same through a
-// prepared statement.
+// numeric strings off and on; the strings should keep the value. decimal and money are the
+// driver's exact text, as numeric is. prepared statements: issue-498-prepared.
 
 const VALUES = [
   'cast(0.1234567890123 as float)',
@@ -34,16 +34,4 @@ module.exports = async t => {
     await t.settled(label)
   }
 
-  // a prepared statement binds these columns as double, numeric included
-  t.query('setup', 'create table #p (n numeric(38,10), d decimal(38,2), f float); ' +
-    'insert #p values (1.0123456789, 12345678901234567.89, 1e-10)', null, { callback: true })
-  await t.settled('setup')
-  const ps = await new Promise((resolve, reject) => {
-    t.conn.prepare({ query_str: 'select n, d, f from #p where 1 = ?', numeric_string: true },
-      (err, p) => err ? reject(err) : resolve(p))
-  })
-  t.section('prepared, numeric strings on')
-  t.watch('ps', ps.preparedQuery([1]), { columns: true })
-  await t.next('ps', 'done')
-  await new Promise(resolve => ps.free(resolve))
 }

test/params-complete-refactored.test.js

@@ -981,13 +981,26 @@ describe('Parameter Tests', function () {
       assert.deepStrictEqual(result.first[0], { f1: '1e-10', f2: '0.1234567890123', f3: '1.5e+300', f4: '-2.5', r1: '0.5' })
     })
 
-    it('should return decimal and money as strings without %f padding', async function () {
+    // decimal is numeric under another name: both are the server's exact text
+    it('should return decimal and money as their exact text', async function () {
       const result = await env.theConnection.promises.query({
-        query_str: 'select cast(1.0123456789 as decimal(38,10)) as d1, cast(0.0000001 as decimal(10,7)) as d2, ' +
-          'cast(-0.0001 as money) as m1, cast(1.2345 as smallmoney) as m2',
+        query_str: 'select cast(1.0123456789 as decimal(38,10)) as d1, cast(12345678901234567890.12 as decimal(38,2)) as d2, ' +
+          'cast(9007199254740993 as decimal(19,0)) as d3, cast(922337203685477.5807 as money) as m1, ' +
+          'cast(-0.0001 as money) as m2, cast(1.2345 as smallmoney) as m3',
         numeric_string: true
       })
-      assert.deepStrictEqual(result.first[0], { d1: '1.0123456789', d2: '1e-7', m1: '-0.0001', m2: '1.2345' })
+      assert.deepStrictEqual(result.first[0], {
+        d1: '1.0123456789', d2: '12345678901234567890.12', d3: '9007199254740993', m1: '922337203685477.5807', m2: '-0.0001', m3: '1.2345'
+      })
+    })
+
+    it('should return decimal and numeric with their scale and a leading zero', async function () {
+      const result = await env.theConnection.promises.query({
+        query_str: 'select cast(5 as decimal(5,2)) as d1, cast(-0.5 as decimal(5,2)) as d2, cast(0 as decimal(5,2)) as d3, ' +
+          'cast(5 as numeric(5,2)) as n1, cast(-0.5 as numeric(5,2)) as n2, cast(0.0000001 as numeric(10,7)) as n3, cast(null as decimal(5,2)) as nul',
+        numeric_string: true
+      })
+      assert.deepStrictEqual(result.first[0], { d1: '5.00', d2: '-0.50', d3: '0.00', n1: '5.00', n2: '-0.50', n3: '0.0000001', nul: null })
     })
   })