fix: prepared statements read numeric and decimal as exact text under numeric strings (#498)

cpp/include/core/bound_datum.h

@@ -88,6 +88,10 @@ class BoundDatum {
     _params = nullptr;
   }
 
+  // a numeric or decimal result column bound as text: 38 digits, a sign, a point, a leading
+  // zero and a terminator
+  static constexpr size_t numeric_text_width = 64;
+
   BoundDatum(shared_ptr<QueryOperationParams> params) : BoundDatum() {
     _params = params;
   }

cpp/include/odbc/odbc_statement_legacy.h

@@ -172,6 +172,7 @@ class OdbcStatementLegacy : public IOdbcStatement {
   bool reserved_int(const size_t row_count, const size_t column) const;
   bool reserved_big_int(const size_t row_count, const size_t column) const;
   bool reserved_decimal(const size_t row_count, const size_t column) const;
+  bool reserved_numeric_text(const size_t row_count, const size_t column) const;
   bool reserved_time(const size_t row_count, const size_t column) const;
   bool reserved_timestamp(const size_t row_count, const size_t column) const;
   bool reserved_timestamp_offset(const size_t row_count, const size_t column) const;

cpp/src/core/bound_datum.cpp

@@ -2408,6 +2408,15 @@ void BoundDatum::reserve_column_type(const SQLSMALLINT type, size_t& len, const
 
     case SQL_DECIMAL:
     case SQL_NUMERIC:
+      // under numeric strings, the driver's exact text: as a double it lost what a double
+      // cannot hold (#498). the reader picks its path from c_type
+      if (_params && _params->numeric_string) {
+        reserve_w_var_char_array(numeric_text_width, row_count);
+      } else {
+        reserve_double(static_cast<SQLLEN>(row_count));
+      }
+      break;
+
     case SQL_REAL:
     case SQL_FLOAT:
     case SQL_DOUBLE:

cpp/src/odbc/odbc_statement_legacy.cpp

@@ -1358,6 +1358,14 @@ bool OdbcStatementLegacy::dispatch_prepared(
 
     case SQL_DECIMAL:
     case SQL_NUMERIC:
+      // bound as text at prepare when numeric strings were on
+      if (_preparedStorage->atIndex(static_cast<int>(column))->c_type == SQL_C_WCHAR) {
+        res = reserved_numeric_text(rows_read, column);
+      } else {
+        res = reserved_decimal(rows_read, column);
+      }
+      break;
+
     case SQL_REAL:
     case SQL_FLOAT:
     case SQL_DOUBLE:
@@ -1978,6 +1986,14 @@ bool OdbcStatementLegacy::get_data_decimal(const size_t row_id, const size_t col
   return true;
 }
 
+// the driver's text for a numeric leaves out the zero before the point ("-.50"): put it back
+static void add_leading_zero(std::vector<uint16_t>& text) {
+  const size_t sign = !text.empty() && text[0] == u'-' ? 1 : 0;
+  if (text.size() > sign && text[sign] == u'.') {
+    text.insert(text.begin() + static_cast<std::ptrdiff_t>(sign), u'0');
+  }
+}
+
 // 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) {
@@ -2005,16 +2021,42 @@ bool OdbcStatementLegacy::get_data_numeric_string(const size_t row_id, const siz
     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');
-  }
+  add_leading_zero(text);
   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;
 }
 
+// a prepared numeric or decimal column bound as text (numeric strings at prepare). an
+// execution with numeric strings off reads the same text back as a number.
+bool OdbcStatementLegacy::reserved_numeric_text(const size_t row_count, const size_t column) const {
+  const auto& bound_datum = _preparedStorage->atIndex(static_cast<int>(column));
+  const auto& ind = bound_datum->get_ind_vec();
+  const auto& store = *bound_datum->get_storage()->uint16vec_ptr;
+  constexpr auto unit = sizeof(uint16_t);
+  for (size_t row_id = 0; row_id < row_count; ++row_id) {
+    if (ind[row_id] == SQL_NULL_DATA) {
+      _resultset->add_column(row_id, make_shared<NullColumn>(column));
+      continue;
+    }
+    const auto offset = BoundDatum::numeric_text_width * row_id;
+    const auto len = min(static_cast<size_t>(ind[row_id]) / unit, BoundDatum::numeric_text_width - 1);
+    std::vector<uint16_t> text(store.begin() + static_cast<std::ptrdiff_t>(offset),
+                               store.begin() + static_cast<std::ptrdiff_t>(offset + len));
+    add_leading_zero(text);
+    if (_numericStringEnabled) {
+      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));
+    } else {
+      const std::string narrow(text.begin(), text.end());
+      _resultset->add_column(row_id, make_shared<NumberColumn>(column, std::strtod(narrow.c_str(), nullptr)));
+    }
+  }
+  return true;
+}
+
 bool OdbcStatementLegacy::get_data_binary(const size_t row_id, const size_t column) {
   auto storage = make_shared<DatumStorageLegacy>();
 

test/contract/HANDOVER.md

@@ -53,9 +53,13 @@ Repros in `repro/i497.js`, `repro/i498.js`; corpus `scripts/issue-497` (+ `-ns`,
   `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.
+- **#498 prepared statements fixed:** with numeric strings on at prepare, `BoundDatum::
+  reserve_column_type` binds `numeric`/`decimal` result columns as wide text (64 units a row)
+  instead of a double; `dispatch_prepared` picks the reader from the binding's `c_type`
+  (`reserved_numeric_text`). The text is the same as a query's, row for row
+  (`scripts/issue-498-prepared`, agreed). Seen on the way: a prepared `select top (?) …` is
+  described with no columns at prepare and returns no rows, numeric strings or not - the same
+  no-meta-at-prepare bug as `select 10 / ? as n`.
 - 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

@@ -1,20 +1,98 @@
 # scenario: scripts/issue-498-prepared
-# status: known-bug
+# status: agreed
 # 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: rowcount 4
 setup: cb ok rows=0 more=false
 setup: done
 setup: free
+== query, numeric strings on
+query: submitted
+query: meta i,n,d,m,f
+query: row
+query: column 0 "1"
+query: column 1 "1.0123456789"
+query: column 2 "12345678901234567.89"
+query: column 3 "922337203685477.5807"
+query: column 4 "1e-10"
+query: row
+query: column 0 "2"
+query: column 1 "-0.5000000000"
+query: column 2 "0.00"
+query: column 3 "-0.0001"
+query: column 4 "-2.5"
+query: row
+query: column 0 "3"
+query: column 1 null
+query: column 2 null
+query: column 3 null
+query: column 4 null
+query: row
+query: column 0 "4"
+query: column 1 "0.0000001000"
+query: column 2 "5.00"
+query: column 3 "1.2345"
+query: column 4 "1.5e+300"
+query: done
+query: free
 == prepared, numeric strings on
+ps-ns: submitted
+ps-ns: info 01S02 0 Cursor type changed
+ps-ns: meta i,n,d,m,f
+ps-ns: row
+ps-ns: column 0 "1"
+ps-ns: column 1 "1.0123456789"
+ps-ns: column 2 "12345678901234567.89"
+ps-ns: column 3 "922337203685477.5807"
+ps-ns: column 4 "1e-10"
+ps-ns: row
+ps-ns: column 0 "2"
+ps-ns: column 1 "-0.5000000000"
+ps-ns: column 2 "0.00"
+ps-ns: column 3 "-0.0001"
+ps-ns: column 4 "-2.5"
+ps-ns: row
+ps-ns: column 0 "3"
+ps-ns: column 1 null
+ps-ns: column 2 null
+ps-ns: column 3 null
+ps-ns: column 4 null
+ps-ns: row
+ps-ns: column 0 "4"
+ps-ns: column 1 "0.0000001000"
+ps-ns: column 2 "5.00"
+ps-ns: column 3 "1.2345"
+ps-ns: column 4 "1.5e+300"
+ps-ns: done
+ps-ns: free
+== prepared, numeric strings off
 ps: submitted
-ps: meta n,d,f
+ps: info 01S02 0 Cursor type changed
+ps: meta i,n,d,m,f
 ps: row
-ps: column 0 "1.0123456789"
-ps: column 1 "12345678901234568"
-ps: column 2 "1e-10"
+ps: column 0 1
+ps: column 1 1.0123456789
+ps: column 2 12345678901234568
+ps: column 3 922337203685477.6
+ps: column 4 1e-10
+ps: row
+ps: column 0 2
+ps: column 1 -0.5
+ps: column 2 0
+ps: column 3 -0.0001
+ps: column 4 -2.5
+ps: row
+ps: column 0 3
+ps: column 1 null
+ps: column 2 null
+ps: column 3 null
+ps: column 4 null
+ps: row
+ps: column 0 4
+ps: column 1 1e-7
+ps: column 2 5
+ps: column 3 1.2345
+ps: column 4 1.5e+300
 ps: done
 ps: free
 check ok

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

@@ -1,18 +1,30 @@
 '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.
+// #498 through a prepared statement: with numeric strings on at prepare, numeric and decimal
+// result columns are bound as text, so they keep every digit, as a query's do. several rows,
+// so each row's text is read from its own place in the bound array; a null, values below 1,
+// money, next to a query of the same rows; and the same statement prepared with numeric strings off still reads numbers.
+
+const ROWS = "(1, 1.0123456789, 12345678901234567.89, 922337203685477.5807, 1e-10), " +
+  '(2, -0.5, 0, -0.0001, -2.5), (3, null, null, null, null), (4, 0.0000001, 5, 1.2345, 1.5e300)'
 
 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 })
+  t.query('setup', 'create table #p (i int, n numeric(38,10), d decimal(38,2), m money, f float); ' +
+    `insert #p values ${ROWS}`, 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))
+  // what a query gives for the same rows: the prepared statement should read the same
+  t.section('query, numeric strings on')
+  t.query('query', { query_str: 'select i, n, d, m, f from #p where i >= 1 order by i', numeric_string: true }, null, { columns: true })
+  await t.settled('query')
+  for (const numericString of [true, false]) {
+    const label = numericString ? 'ps-ns' : 'ps'
+    const ps = await new Promise((resolve, reject) => {
+      t.conn.prepare({ query_str: 'select i, n, d, m, f from #p where i >= ? order by i', numeric_string: numericString },
+        (err, p) => err ? reject(err) : resolve(p))
+    })
+    t.section(`prepared, numeric strings ${numericString ? 'on' : 'off'}`)
+    t.watch(label, ps.preparedQuery([1]), { columns: true })
+    await t.next(label, 'done')
+    await new Promise(resolve => ps.free(resolve))
+  }
 }

test/params-complete-refactored.test.js

@@ -994,6 +994,28 @@ describe('Parameter Tests', function () {
       })
     })
 
+    // a prepared statement binds these columns as text under numeric strings; 120 rows is three
+    // fetch blocks of 50, so each row is read from its own place in the bound array
+    it('should return prepared decimal and numeric as their exact text', async function () {
+      const q = {
+        query_str: "select cast('12345678901234567.' + right('00' + cast(i as varchar(3)), 2) as decimal(38,2)) as d, " +
+          'cast(-i / 1000.0 as numeric(10,4)) as n from (select row_number() over (order by object_id) as i from sys.all_objects) t ' +
+          'where i <= ? order by i',
+        numeric_string: true
+      }
+      const prepared = await env.theConnection.promises.prepare(q)
+      try {
+        const res = await prepared.promises.query([120])
+        const expected = Array.from({ length: 120 }, (_, k) => {
+          const i = k + 1
+          return { d: `12345678901234567.${String(i % 100).padStart(2, '0')}`, n: `-${(i / 1000).toFixed(4)}` }
+        })
+        assert.deepStrictEqual(res.first, expected)
+      } finally {
+        await prepared.promises.free()
+      }
+    })
+
     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, ' +