Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cpp/include/js/columns/number_column.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ class NumberColumn : public Column {
NumberColumn(int id, shared_ptr<DatumStorageLegacy> storage)
: Column(id), value((*storage->doublevec_ptr)[0]) {}

// the same text as String(n) in JS: the shortest string that reads back as the same
// double. to_wstring is %f, six decimal places and no exponent (1e-10 gave "0.000000", #498)
inline Napi::Object ToString(Napi::Env env) override {
return AsString<double>(env, value);
return Napi::Number::New(env, value).ToString().As<Napi::Object>();
}

inline Napi::Object ToNative(Napi::Env env) override {
Expand Down
6 changes: 6 additions & 0 deletions cpp/include/js/columns/result_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ class ResultSet {
void add_column(size_t row_id, const shared_ptr<Column>& column);
Napi::Object get_entry(Napi::Env env, const ColumnDefinition& definition);
shared_ptr<Column> get_column(size_t row_id, size_t id) const;
// forget rows from row_id on: a row whose read failed part way
void drop_rows_from(size_t row_id) {
if (_rows.size() > row_id) {
_rows.resize(row_id);
}
}
size_t get_result_count() const {
return _rows.size();
}
Expand Down
3 changes: 3 additions & 0 deletions cpp/src/js/columns/result_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ shared_ptr<Column> ResultSet::get_column(const size_t row_id, const size_t colum
return nullptr;
}
const auto& row = _rows[row_id];
if (column_id >= row.size()) {
return nullptr;
}
return row[column_id];
}

Expand Down
4 changes: 3 additions & 1 deletion cpp/src/js/js_object_mapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ Napi::Object JsObjectMapper::fromQueryResult(const Napi::Env& env,
auto row_array = Napi::Array::New(env, column_count);
results_array.Set(static_cast<uint32_t>(row_id), row_array);
for (auto c = 0; c < column_count; ++c) {
row_array.Set(static_cast<uint32_t>(c), resultset->get_column(row_id, c)->ToValue(env));
// a cell never stored (its read failed) is null, not a dereference of nothing
const auto column = resultset->get_column(row_id, c);
row_array.Set(static_cast<uint32_t>(c), column ? column->ToValue(env) : env.Null());
}
}

Expand Down
38 changes: 27 additions & 11 deletions cpp/src/odbc/odbc_statement_legacy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,21 @@ bool OdbcStatementLegacy::fetch_read(const size_t number_rows) {
const auto column_count = static_cast<int>(_resultset->get_column_count());
for (auto c = 0; c < column_count; ++c) {
const auto& definition = _resultset->get_meta_data(c);
res = dispatch(definition.dataType, row_id, c);
if (!res) {
break;
const auto errors = _errors->size();
if (!dispatch(definition.dataType, row_id, c)) {
// a cell that cannot be read ends the fetch with its error: drop the part of the row
// already read, rather than go on and leave a row with holes in it (#497)
_resultset->drop_rows_from(row_id);
if (_errors->size() == errors) {
// the read failed with no diagnostic from the driver: the query must still end in
// an error, not look complete with its remaining rows missing
const auto msg = "[msnodesql] cannot read column " + std::to_string(c) + " of the result";
const auto error = make_shared<OdbcError>("HY000", msg.c_str(), 0, 0, "", "", 0);
_errors->push_back(error);
_errorHandler->AddError(error);
set_state(OdbcStatementState::STATEMENT_ERROR);
}
return false;
}
}
}
Expand Down Expand Up @@ -1505,6 +1517,12 @@ bool OdbcStatementLegacy::d_variant(
SQL_LOG_DEBUG_STREAM("[" << _handle.toString() << "] d_variant failed to get data");
return false;
}
// a NULL is known from this first read; a second SQLGetData on the column would return
// SQL_NO_DATA, and the row would be lost
if (iv == SQL_NULL_DATA) {
_resultset->add_column(row_id, make_shared<NullColumn>(column));
return true;
}
// Figure out the type
ret = _odbcApi->SQLColAttribute(statement.get_handle(),
column + 1,
Expand All @@ -1517,11 +1535,9 @@ bool OdbcStatementLegacy::d_variant(
SQL_LOG_DEBUG_STREAM("[" << _handle.toString() << "] d_variant failed to get variant type");
return false;
}
// set the definiton to actual data underlying data type.
auto& definition = _resultset->get_meta_data(static_cast<int>(column));
definition.dataType = static_cast<SQLSMALLINT>(variant_type);
const auto res = dispatch(definition.dataType, row_id, column);
return res;
// read this row by its own base type. the column stays SQL_SS_VARIANT, so every row is
// resolved again: rows of one variant column can each hold a different type (#497)
return dispatch(static_cast<SQLSMALLINT>(variant_type), row_id, column);
}

bool OdbcStatementLegacy::d_time(
Expand Down Expand Up @@ -1940,9 +1956,9 @@ bool OdbcStatementLegacy::get_data_decimal(const size_t row_id, const size_t col
}

const auto v2 = trunc(v);
if (v2 == v &&
v2 >= static_cast<long double>(numeric_limits<DatumStorageLegacy::bigint_t>::min()) &&
v2 <= static_cast<long double>(numeric_limits<DatumStorageLegacy::bigint_t>::max())) {
// the bounds are exact as doubles: INT64_MAX is not, and rounds up to 2^63, which does not
// fit in an int64 (bigint max read as a double came back as INT64_MIN)
if (v2 == v && v2 >= -9223372036854775808.0 && v2 < 9223372036854775808.0) {
auto bi = static_cast<DatumStorageLegacy::bigint_t>(v);
const auto col = make_shared<BigIntColumn>(column, bi);
if (_numericStringEnabled) {
Expand Down
27 changes: 26 additions & 1 deletion test/contract/HANDOVER.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Handover: the v6 event contract (from the 5.6.0 regressions, #478 – #481)

Started 2026-10-15 from dhensby's reports. Last updated 2026-10-18. The newest state is
Started 2026-10-15 from dhensby's reports. Last updated 2026-10-24. The newest state is
first; the dated Progress sections below have the detail, and the older sections are history.

## Where we are — end of 2026-10-18
Expand Down Expand Up @@ -32,6 +32,31 @@ 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-24: #497 (sql_variant) and #498 (numeric strings)

Both reported by dhensby against 6.0.0; both reproduced here (ODBC 18), the crash included.
Repros in `repro/i497.js`, `repro/i498.js`; corpus `scripts/issue-497` (+ `-ns`, agreed) and
`scripts/issue-498` (known-bug). Unit tests in `test/params-complete-refactored.test.js`.

- **#497 fixed.** `d_variant` wrote the first row's base type into the column's definition, so
every later row was read as that type. It now dispatches each row on its own type, and stores
a NULL from its first `SQLGetData` (a second read of a NULL is `SQL_NO_DATA`, which lost the row).
- **The 0xC0000005 crash fixed.** A failed cell left a hole in the row, and building the JS rows
dereferenced it. `get_column` and `fromQueryResult` now give null for a missing cell, and a
failed cell ends the fetch: the partial row is dropped and the query ends in an error - one of
ours (HY000 "cannot read column N") if the driver gave none, so a failed read is never silent.
Still open: the rows read before it in the same fetch batch are not delivered.
- **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).
- 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`).

## Progress — 2026-10-22: node-mssql against v6

node-mssql's own msnodesqlv8 suite (`npx mocha --exit -t 30000 test/msnodesqlv8` in a clone,
Expand Down
236 changes: 236 additions & 0 deletions test/contract/corpus/scripts/issue-497-ns.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
# scenario: scripts/issue-497-ns
# status: agreed
# script: test/contract/scripts/issue-497-ns.js
== int first, numeric strings on
int-ns: submitted
int-ns: meta i,v
int-ns: row
int-ns: column 0 "1"
int-ns: column 1 "7"
int-ns: row
int-ns: column 0 "2"
int-ns: column 1 "9223372036854775807"
int-ns: row
int-ns: column 0 "3"
int-ns: column 1 "1.50"
int-ns: row
int-ns: column 0 "4"
int-ns: column 1 "2.25e-9"
int-ns: row
int-ns: column 0 "5"
int-ns: column 1 "abc"
int-ns: row
int-ns: column 0 "6"
int-ns: column 1 2020-01-02T00:00:00.000Z
int-ns: row
int-ns: column 0 "7"
int-ns: column 1 true
int-ns: row
int-ns: column 0 "8"
int-ns: column 1 null
int-ns: done
int-ns: free
== bigint first, numeric strings on
bigint-ns: submitted
bigint-ns: meta i,v
bigint-ns: row
bigint-ns: column 0 "1"
bigint-ns: column 1 "9223372036854775807"
bigint-ns: row
bigint-ns: column 0 "2"
bigint-ns: column 1 "7"
bigint-ns: row
bigint-ns: column 0 "3"
bigint-ns: column 1 "1.50"
bigint-ns: row
bigint-ns: column 0 "4"
bigint-ns: column 1 "2.25e-9"
bigint-ns: row
bigint-ns: column 0 "5"
bigint-ns: column 1 "abc"
bigint-ns: row
bigint-ns: column 0 "6"
bigint-ns: column 1 2020-01-02T00:00:00.000Z
bigint-ns: row
bigint-ns: column 0 "7"
bigint-ns: column 1 true
bigint-ns: row
bigint-ns: column 0 "8"
bigint-ns: column 1 null
bigint-ns: done
bigint-ns: free
== decimal first, numeric strings on
decimal-ns: submitted
decimal-ns: meta i,v
decimal-ns: row
decimal-ns: column 0 "1"
decimal-ns: column 1 "1.50"
decimal-ns: row
decimal-ns: column 0 "2"
decimal-ns: column 1 "7"
decimal-ns: row
decimal-ns: column 0 "3"
decimal-ns: column 1 "9223372036854775807"
decimal-ns: row
decimal-ns: column 0 "4"
decimal-ns: column 1 "2.25e-9"
decimal-ns: row
decimal-ns: column 0 "5"
decimal-ns: column 1 "abc"
decimal-ns: row
decimal-ns: column 0 "6"
decimal-ns: column 1 2020-01-02T00:00:00.000Z
decimal-ns: row
decimal-ns: column 0 "7"
decimal-ns: column 1 true
decimal-ns: row
decimal-ns: column 0 "8"
decimal-ns: column 1 null
decimal-ns: done
decimal-ns: free
== float first, numeric strings on
float-ns: submitted
float-ns: meta i,v
float-ns: row
float-ns: column 0 "1"
float-ns: column 1 "2.25e-9"
float-ns: row
float-ns: column 0 "2"
float-ns: column 1 "7"
float-ns: row
float-ns: column 0 "3"
float-ns: column 1 "9223372036854775807"
float-ns: row
float-ns: column 0 "4"
float-ns: column 1 "1.50"
float-ns: row
float-ns: column 0 "5"
float-ns: column 1 "abc"
float-ns: row
float-ns: column 0 "6"
float-ns: column 1 2020-01-02T00:00:00.000Z
float-ns: row
float-ns: column 0 "7"
float-ns: column 1 true
float-ns: row
float-ns: column 0 "8"
float-ns: column 1 null
float-ns: done
float-ns: free
== nvarchar first, numeric strings on
nvarchar-ns: submitted
nvarchar-ns: meta i,v
nvarchar-ns: row
nvarchar-ns: column 0 "1"
nvarchar-ns: column 1 "abc"
nvarchar-ns: row
nvarchar-ns: column 0 "2"
nvarchar-ns: column 1 "7"
nvarchar-ns: row
nvarchar-ns: column 0 "3"
nvarchar-ns: column 1 "9223372036854775807"
nvarchar-ns: row
nvarchar-ns: column 0 "4"
nvarchar-ns: column 1 "1.50"
nvarchar-ns: row
nvarchar-ns: column 0 "5"
nvarchar-ns: column 1 "2.25e-9"
nvarchar-ns: row
nvarchar-ns: column 0 "6"
nvarchar-ns: column 1 2020-01-02T00:00:00.000Z
nvarchar-ns: row
nvarchar-ns: column 0 "7"
nvarchar-ns: column 1 true
nvarchar-ns: row
nvarchar-ns: column 0 "8"
nvarchar-ns: column 1 null
nvarchar-ns: done
nvarchar-ns: free
== datetime first, numeric strings on
datetime-ns: submitted
datetime-ns: meta i,v
datetime-ns: row
datetime-ns: column 0 "1"
datetime-ns: column 1 2020-01-02T00:00:00.000Z
datetime-ns: row
datetime-ns: column 0 "2"
datetime-ns: column 1 "7"
datetime-ns: row
datetime-ns: column 0 "3"
datetime-ns: column 1 "9223372036854775807"
datetime-ns: row
datetime-ns: column 0 "4"
datetime-ns: column 1 "1.50"
datetime-ns: row
datetime-ns: column 0 "5"
datetime-ns: column 1 "2.25e-9"
datetime-ns: row
datetime-ns: column 0 "6"
datetime-ns: column 1 "abc"
datetime-ns: row
datetime-ns: column 0 "7"
datetime-ns: column 1 true
datetime-ns: row
datetime-ns: column 0 "8"
datetime-ns: column 1 null
datetime-ns: done
datetime-ns: free
== bit first, numeric strings on
bit-ns: submitted
bit-ns: meta i,v
bit-ns: row
bit-ns: column 0 "1"
bit-ns: column 1 true
bit-ns: row
bit-ns: column 0 "2"
bit-ns: column 1 "7"
bit-ns: row
bit-ns: column 0 "3"
bit-ns: column 1 "9223372036854775807"
bit-ns: row
bit-ns: column 0 "4"
bit-ns: column 1 "1.50"
bit-ns: row
bit-ns: column 0 "5"
bit-ns: column 1 "2.25e-9"
bit-ns: row
bit-ns: column 0 "6"
bit-ns: column 1 "abc"
bit-ns: row
bit-ns: column 0 "7"
bit-ns: column 1 2020-01-02T00:00:00.000Z
bit-ns: row
bit-ns: column 0 "8"
bit-ns: column 1 null
bit-ns: done
bit-ns: free
== null first, numeric strings on
null-ns: submitted
null-ns: meta i,v
null-ns: row
null-ns: column 0 "1"
null-ns: column 1 null
null-ns: row
null-ns: column 0 "2"
null-ns: column 1 "7"
null-ns: row
null-ns: column 0 "3"
null-ns: column 1 "9223372036854775807"
null-ns: row
null-ns: column 0 "4"
null-ns: column 1 "1.50"
null-ns: row
null-ns: column 0 "5"
null-ns: column 1 "2.25e-9"
null-ns: row
null-ns: column 0 "6"
null-ns: column 1 "abc"
null-ns: row
null-ns: column 0 "7"
null-ns: column 1 2020-01-02T00:00:00.000Z
null-ns: row
null-ns: column 0 "8"
null-ns: column 1 true
null-ns: done
null-ns: free
check ok
Loading
Loading