From a9270a70a457fdd960759d04d2d7daf4bc39a71c Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Thu, 28 May 2026 21:48:41 -0700 Subject: [PATCH] fix: NUMERIC sign loss in Display + Node bindings decode (#84) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes two related but distinct bugs that surface together as wrong NUMERIC values in MCP query results and Node bindings: 1. **Core: `Numeric::Display` dropped the sign for values in (-1, 0).** The previous impl derived the sign from `int_part = value / divisor`, which is `0` for sub-unit magnitudes (and `0` prints without a sign); the fractional part was `(value % divisor).abs()` (sign stripped). So `Numeric::new(-5000, 4).to_string()` returned `"0.5000"` instead of `"-0.5000"`. The fix computes the sign explicitly and formats the magnitude via `unsigned_abs()`. This silently flipped the sign of any correlation, 0-1 index, or regression residual that crossed the stringify path. `unsigned_abs()` also avoids the `i128::MIN` overflow panic that `.abs()` would have hit. The MCP `query` tool (`row_value_to_json`) serializes NUMERIC via `to_string()` then `parse::()`, so `CAST(-0.5 AS numeric(10,4))` was returning JSON `0.5` instead of `-0.5`. 2. **Node bindings: `getF64` was decoding NUMERIC as garbage / NaN.** This is a separate root cause from #84's Display bug, fixed here for the same test surface. `extract_row` and `extract_chunk_columnar` were calling `row.get_f64(i)` for `SqlType::Numeric` columns. `get_f64` only handles native f32/f64 wire types; for NUMERIC it reinterpreted the unscaled-integer bytes as IEEE-754 → garbage values or NaN, regardless of sign. Switched to schema-aware `row.get_numeric(i)`, which honors the column scale and decodes the 8/16-byte / Arrow Decimal wire forms correctly. Added a `CellValue::Numeric(Numeric)` variant; `getString` returns the exact decimal text (preserving scale and sign), `getFloat64` returns the lossy-but-correct double, and `getInt32`/`getInt64` return the truncated integer. `getBigInt` on a `NUMERIC(p, 0)` column now preserves the full unscaled `i128` value (use it instead of `getInt64` for integer NUMERIC values above `Number.MAX_SAFE_INTEGER`); on `NUMERIC(p, scale>0)` it returns `null` rather than silently truncating. The columnar fast path surfaces NUMERIC as correct `f64` values. ## Test coverage - Core: `test_numeric_display_negative_sign_preserved` — covers -0.5, -0.999, -0.0001, -1.5, 0, +0.5, scale-0 negative, and `i128::MIN` (no panic). - MCP: `engine_negative_sub_unit_numeric_keeps_sign` — exercises the exact reproduction from the issue body via `execute_query_to_json`. - Node smoke (section 18b): row-wise `getString`/`getFloat64`, AVG aggregate, columnar fast path, `getBigInt` precision preservation for `NUMERIC(38,0)` values above `Number.MAX_SAFE_INTEGER`, `getBigInt` returning null on `NUMERIC(p, scale>0)`. Adversarial review (line-level + architect in parallel) caught the missing `getBigInt` Numeric arm and the smoke-test gap; both addressed before commit. The Cargo.lock regen in `hyperdb-api-node/package-lock.json` removes stale `version: "0.1.3"` and `optionalDependencies` entries that violate the version-consistency CI guard (npm package versions are injected at publish time, not tracked in source). Closes #84. --- hyperdb-api-core/CHANGELOG.md | 9 ++ hyperdb-api-core/src/types/special.rs | 41 +++++++-- hyperdb-api-node/CHANGELOG.md | 16 ++++ hyperdb-api-node/__test__/smoke.mjs | 116 ++++++++++++++++++++++++++ hyperdb-api-node/package-lock.json | 62 -------------- hyperdb-api-node/src/columnar.rs | 12 ++- hyperdb-api-node/src/result.rs | 37 +++++++- hyperdb-mcp/CHANGELOG.md | 9 ++ hyperdb-mcp/tests/engine_tests.rs | 37 ++++++++ 9 files changed, 267 insertions(+), 72 deletions(-) diff --git a/hyperdb-api-core/CHANGELOG.md b/hyperdb-api-core/CHANGELOG.md index 10449b0..a180814 100644 --- a/hyperdb-api-core/CHANGELOG.md +++ b/hyperdb-api-core/CHANGELOG.md @@ -13,6 +13,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed + +- `Numeric`'s `Display` implementation no longer drops the sign of negative + values with magnitude less than 1 (the open interval `(-1, 0)`). Values such + as `-0.5` previously rendered as `0.5000` because the sign was derived from + the integer part, which is `0` for sub-unit magnitudes. The sign is now + computed explicitly and the magnitude formatted via `unsigned_abs`, which + also removes a latent `i128::MIN` overflow panic. + ## [0.1.1] - 2026-05-13 ### Added diff --git a/hyperdb-api-core/src/types/special.rs b/hyperdb-api-core/src/types/special.rs index c5d7374..7355bf9 100644 --- a/hyperdb-api-core/src/types/special.rs +++ b/hyperdb-api-core/src/types/special.rs @@ -1225,14 +1225,19 @@ impl fmt::Display for Numeric { if self.scale == 0 { write!(f, "{}", self.value) } else { - let divisor = 10i128.pow(u32::from(self.scale)); - let int_part = self.value / divisor; - let frac_part = (self.value % divisor).abs(); + // Compute the sign explicitly and format the magnitude. Deriving + // the sign from `int_part` alone loses it whenever `|value| < 1` + // (the integer part is `0`, which prints without a sign), so + // values like -0.5 would render as "0.5000". `unsigned_abs` also + // avoids the `i128::MIN` overflow that `.abs()` would hit. + let divisor = 10u128.pow(u32::from(self.scale)); + let sign = if self.value < 0 { "-" } else { "" }; + let abs = self.value.unsigned_abs(); + let int_part = abs / divisor; + let frac_part = abs % divisor; write!( f, - "{}.{:0width$}", - int_part, - frac_part, + "{sign}{int_part}.{frac_part:0width$}", width = self.scale as usize ) } @@ -2221,6 +2226,30 @@ mod tests { assert_eq!(num.to_string(), "123.45"); } + #[test] + fn test_numeric_display_negative_sign_preserved() { + // Regression: values in (-1, 0) must keep their sign. The integer + // part is 0 for these, which previously dropped the minus sign and + // rendered -0.5000 as "0.5000". + assert_eq!(Numeric::new(-5000, 4).to_string(), "-0.5000"); + assert_eq!(Numeric::new(-9990, 4).to_string(), "-0.9990"); + assert_eq!(Numeric::new(-1, 4).to_string(), "-0.0001"); + + // |value| >= 1 already worked; guard against regressions. + assert_eq!(Numeric::new(-15000, 4).to_string(), "-1.5000"); + assert_eq!(Numeric::new(-10000, 4).to_string(), "-1.0000"); + + // Zero and positive sub-unit values must not gain a spurious sign. + assert_eq!(Numeric::new(0, 4).to_string(), "0.0000"); + assert_eq!(Numeric::new(5000, 4).to_string(), "0.5000"); + + // scale == 0 path keeps negative integers intact. + assert_eq!(Numeric::new(-1, 0).to_string(), "-1"); + + // i128::MIN must not panic (unsigned_abs avoids the .abs() overflow). + let _ = Numeric::new(i128::MIN, 4).to_string(); + } + #[test] fn test_numeric_from_binary_with_scale() { // Unscaled value 123 with scale 2 = 1.23 diff --git a/hyperdb-api-node/CHANGELOG.md b/hyperdb-api-node/CHANGELOG.md index d7aab98..f7dfa26 100644 --- a/hyperdb-api-node/CHANGELOG.md +++ b/hyperdb-api-node/CHANGELOG.md @@ -14,6 +14,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed + +- `NUMERIC` columns are now decoded correctly. Previously the bindings read + numerics with `getF64`, which reinterpreted the raw unscaled-integer bytes as + an IEEE-754 double and returned garbage values or `NaN`. Numerics are now + decoded schema-aware (honoring the column scale): `getString` returns the + exact decimal text (preserving scale and sign, including sub-unit negatives + such as `-0.5000`), `getFloat64` returns the correct (possibly lossy) value, + and `getInt32`/`getInt64` return the truncated integer. `getBigInt` on a + `NUMERIC(p, 0)` column now preserves the full unscaled value (use it instead + of `getInt64` for integer NUMERIC values above `Number.MAX_SAFE_INTEGER`); on + a `NUMERIC(p, scale>0)` column it returns `null` (use `getString` for exact + text or `getFloat64` for a lossy value). The columnar fast path + (`executeQueryColumnar`) surfaces numerics as correct `f64` values instead of + garbage. Relates to [#84](https://github.com/tableau/hyper-api-rust/issues/84). + ## [0.1.1] - 2026-05-13 ### Added diff --git a/hyperdb-api-node/__test__/smoke.mjs b/hyperdb-api-node/__test__/smoke.mjs index 9135470..44a669b 100644 --- a/hyperdb-api-node/__test__/smoke.mjs +++ b/hyperdb-api-node/__test__/smoke.mjs @@ -338,6 +338,122 @@ async function main() { const verify = await conn.executeQuery('SELECT COUNT(*) FROM arrow_t'); console.log(` Verified COUNT(*): ${verify[0].getBigInt(0)}`); + // 18b. NUMERIC decoding — sign, scale, precision (row-wise + columnar) + console.log('\n18b. NUMERIC types...'); + await conn.executeCommand('DROP TABLE IF EXISTS numeric_test'); + await conn.executeCommand( + 'CREATE TABLE numeric_test (id INT NOT NULL, n NUMERIC(20,4), hp NUMERIC(38,12))' + ); + await conn.executeCommand( + `INSERT INTO numeric_test VALUES + (1, 123.4500, 0), + (2, -67.8900, 0), + (3, -0.5000, 0), + (4, -0.9990, 0), + (5, 0.0000, 123456789.123456789012)` + ); + + const numRows = await conn.executeQuery( + 'SELECT id, n, hp FROM numeric_test ORDER BY id' + ); + + // Exact decimal text via getString (preserves scale + sign). + const nStrings = numRows.map((r) => r.getString(1)); + console.log(` getString(n): ${JSON.stringify(nStrings)}`); + assert.equal(nStrings[0], '123.4500', 'positive numeric exact text'); + assert.equal(nStrings[1], '-67.8900', 'negative numeric exact text'); + // Regression: sub-unit negatives must keep their sign (issue #84). + assert.equal(nStrings[2], '-0.5000', 'sub-unit negative keeps sign'); + assert.equal(nStrings[3], '-0.9990', 'sub-unit negative keeps sign'); + + // getFloat64 must be the real value, not a reinterpreted-bytes garbage/NaN. + const nFloats = numRows.map((r) => r.getFloat64(1)); + console.log(` getFloat64(n): ${JSON.stringify(nFloats)}`); + assert.ok(Math.abs(nFloats[0] - 123.45) < 1e-9, 'positive f64'); + assert.ok(Math.abs(nFloats[1] - -67.89) < 1e-9, 'negative f64'); + assert.ok(Math.abs(nFloats[2] - -0.5) < 1e-12, 'sub-unit negative f64 keeps sign'); + assert.ok(nFloats[2] < 0, 'sub-unit negative f64 is negative'); + assert.ok(nFloats.every((v) => !Number.isNaN(v)), 'no NaN from NUMERIC decode'); + + // High precision (>15 significant digits): exact via string, lossy via f64. + const hp = numRows[4].getString(2); + console.log(` getString(hp): ${hp}`); + assert.equal(hp, '123456789.123456789012', 'high-precision exact text'); + + // AVG over a numeric column returns a numeric — decode it correctly. + const avgRow = await conn.executeQuery( + 'SELECT AVG(n) FROM numeric_test WHERE id <= 4' + ); + const avg = avgRow[0].getFloat64(0); + console.log(` AVG(n) where id<=4: ${avg}`); + // (123.45 - 67.89 - 0.5 - 0.999) / 4 = 13.51525 + assert.ok(Math.abs(avg - 13.51525) < 1e-6, 'AVG numeric decodes correctly'); + + // Columnar path surfaces NUMERIC as f64 — must match row-wise, not garbage. + const numColStream = conn.executeQueryColumnar( + 'SELECT id, n FROM numeric_test ORDER BY id' + ); + const numChunk = await numColStream.nextChunk(); + const nCol = numChunk.getFloat64Column(1); + console.log(` columnar getFloat64Column(n): [${Array.from(nCol).join(', ')}]`); + assert.ok(Math.abs(nCol[0] - 123.45) < 1e-9, 'columnar positive'); + assert.ok(Math.abs(nCol[2] - -0.5) < 1e-12, 'columnar sub-unit negative keeps sign'); + assert.ok(Array.from(nCol).every((v) => !Number.isNaN(v)), 'columnar no NaN'); + + // NUMERIC(p, 0) integer-shaped paths: getInt32 / getInt64 / getBigInt. + // Use a separate table so we can exercise scale=0 specifically. + await conn.executeCommand('DROP TABLE IF EXISTS numeric_int_test'); + await conn.executeCommand( + 'CREATE TABLE numeric_int_test (id INT NOT NULL, big NUMERIC(38,0))' + ); + // Includes a value > 2^53 to exercise the BigInt path's precision + // preservation (i64::MAX = 9223372036854775807 > Number.MAX_SAFE_INTEGER). + await conn.executeCommand( + `INSERT INTO numeric_int_test VALUES + (1, 42), + (2, -42), + (3, 9223372036854775807)` + ); + + const intRows = await conn.executeQuery( + 'SELECT id, big FROM numeric_int_test ORDER BY id' + ); + + // getInt32 / getInt64 narrow through f64 — fine for small values. + assert.equal(intRows[0].getInt32(1), 42, 'NUMERIC(p,0) -> Int32 positive'); + assert.equal(intRows[1].getInt32(1), -42, 'NUMERIC(p,0) -> Int32 negative'); + assert.equal(intRows[0].getInt64(1), 42, 'NUMERIC(p,0) -> Int64 positive'); + assert.equal(intRows[1].getInt64(1), -42, 'NUMERIC(p,0) -> Int64 negative'); + + // getBigInt preserves full precision for NUMERIC(p, 0); a value above + // Number.MAX_SAFE_INTEGER must round-trip exactly. + const bigSmall = intRows[0].getBigInt(1); + const bigNeg = intRows[1].getBigInt(1); + const bigLarge = intRows[2].getBigInt(1); + console.log( + ` getBigInt(NUMERIC(38,0)): ${bigSmall}, ${bigNeg}, ${bigLarge}` + ); + assert.equal(bigSmall, 42n, 'BigInt small positive'); + assert.equal(bigNeg, -42n, 'BigInt small negative'); + assert.equal( + bigLarge, + 9223372036854775807n, + 'BigInt preserves precision above 2^53' + ); + + // getBigInt on a non-zero-scale NUMERIC must return null (the cell is + // not an integer; callers should use getString or getFloat64). + const decBigInt = numRows[0].getBigInt(1); + assert.equal( + decBigInt, + null, + 'getBigInt on NUMERIC(p, scale>0) returns null' + ); + + await conn.executeCommand('DROP TABLE numeric_int_test'); + await conn.executeCommand('DROP TABLE numeric_test'); + console.log(' All NUMERIC tests passed ✓'); + // 19. Clean up console.log('\n19. Cleaning up...'); await catalog.dropTable('test_users'); diff --git a/hyperdb-api-node/package-lock.json b/hyperdb-api-node/package-lock.json index 34244ff..340b79b 100644 --- a/hyperdb-api-node/package-lock.json +++ b/hyperdb-api-node/package-lock.json @@ -6,7 +6,6 @@ "packages": { "": { "name": "hyperdb-api-node", - "version": "0.1.3", "license": "MIT OR Apache-2.0", "devDependencies": { "@napi-rs/cli": "^3.6.2", @@ -16,13 +15,6 @@ "engines": { "node": ">= 21" }, - "optionalDependencies": { - "hyperdb-api-node-darwin-arm64": "0.1.3", - "hyperdb-api-node-linux-arm64-gnu": "0.1.3", - "hyperdb-api-node-linux-x64-gnu": "0.1.3", - "hyperdb-api-node-linux-x64-musl": "0.1.3", - "hyperdb-api-node-win32-x64-msvc": "0.1.3" - }, "peerDependencies": { "apache-arrow": ">=14.0.0" }, @@ -2429,60 +2421,6 @@ "node": ">=8" } }, - "node_modules/hyperdb-api-node-darwin-arm64": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/hyperdb-api-node-darwin-arm64/-/hyperdb-api-node-darwin-arm64-0.1.3.tgz", - "integrity": "sha512-XV5B15T1iC6FyRMEBXRTzzvRzuRmDza6/JjiB+SkhcI0RME++yAzFOuewirFnCp5T5vLt1dGdgEhfdE45zOsLA==", - "cpu": [ - "arm64" - ], - "license": "MIT OR Apache-2.0", - "optional": true, - "os": [ - "darwin" - ], - "engines": { - "node": ">= 21" - } - }, - "node_modules/hyperdb-api-node-linux-arm64-gnu": { - "optional": true - }, - "node_modules/hyperdb-api-node-linux-x64-gnu": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/hyperdb-api-node-linux-x64-gnu/-/hyperdb-api-node-linux-x64-gnu-0.1.3.tgz", - "integrity": "sha512-olez3M2g4V2bNFAK6nHi9v7P6WAC6QL9An2D7FhR1jx5350zNhNmy95Os/RDUobO5UAMPcR6sl9wXdWt0sx/gg==", - "cpu": [ - "x64" - ], - "license": "MIT OR Apache-2.0", - "optional": true, - "os": [ - "linux" - ], - "engines": { - "node": ">= 21" - } - }, - "node_modules/hyperdb-api-node-linux-x64-musl": { - "optional": true - }, - "node_modules/hyperdb-api-node-win32-x64-msvc": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/hyperdb-api-node-win32-x64-msvc/-/hyperdb-api-node-win32-x64-msvc-0.1.3.tgz", - "integrity": "sha512-X8k0Lxbwq6vtmWutE5JPfRhRfPIW1vBziRjxHCalKayPsDKmVCzbVz157GcS/o5Z7JNdlkCYHLRqLPmTtaZ6aA==", - "cpu": [ - "x64" - ], - "license": "MIT OR Apache-2.0", - "optional": true, - "os": [ - "win32" - ], - "engines": { - "node": ">= 21" - } - }, "node_modules/iconv-lite": { "version": "0.7.2", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.7.2.tgz", diff --git a/hyperdb-api-node/src/columnar.rs b/hyperdb-api-node/src/columnar.rs index f0d5eb1..68821db 100644 --- a/hyperdb-api-node/src/columnar.rs +++ b/hyperdb-api-node/src/columnar.rs @@ -252,11 +252,21 @@ pub(crate) fn extract_chunk_columnar( v[row_idx] = f64::from(row.get_f32(col_idx).unwrap_or(0.0)); } } - hyperdb_api::SqlType::Double | hyperdb_api::SqlType::Numeric { .. } => { + hyperdb_api::SqlType::Double => { if let ColumnData::Float64(ref mut v) = columns[col_idx] { v[row_idx] = row.get_f64(col_idx).unwrap_or(0.0); } } + hyperdb_api::SqlType::Numeric { .. } => { + // Schema-aware decode then narrow to f64. `get_f64` must NOT + // be used: it reinterprets the unscaled-integer bytes as an + // IEEE-754 double (garbage/NaN). The columnar fast path + // surfaces numerics as f64 (lossy for >15 sig digits); the + // row-wise path preserves exact text via `getString`. + if let ColumnData::Float64(ref mut v) = columns[col_idx] { + v[row_idx] = row.get_numeric(col_idx).map_or(0.0, |n| n.to_f64()); + } + } hyperdb_api::SqlType::Date => { if let ColumnData::Strings(ref mut v) = columns[col_idx] { v[row_idx] = row diff --git a/hyperdb-api-node/src/result.rs b/hyperdb-api-node/src/result.rs index 9b13752..dcb68ab 100644 --- a/hyperdb-api-node/src/result.rs +++ b/hyperdb-api-node/src/result.rs @@ -26,6 +26,10 @@ pub(crate) enum CellValue { F64(f64), String(String), Bytes(Vec), + /// Arbitrary-precision decimal, decoded with the column's scale. Stored + /// as the schema-aware `Numeric` so `getString` can return the exact + /// decimal text while `getFloat64` returns the (possibly lossy) `f64`. + Numeric(hyperdb_api::Numeric), /// Date as Unix milliseconds (for JS Date interop) DateMs(f64), /// Timestamp as Unix milliseconds (for JS Date interop) @@ -62,8 +66,12 @@ pub(crate) fn extract_row( hyperdb_api::SqlType::Float => CellValue::F32(row.get_f32(i).unwrap_or(0.0)), hyperdb_api::SqlType::Double => CellValue::F64(row.get_f64(i).unwrap_or(0.0)), hyperdb_api::SqlType::Numeric { .. } => { - // Numeric doesn't have a direct get method; use f64 or string - CellValue::F64(row.get_f64(i).unwrap_or(0.0)) + // Schema-aware decode: `get_numeric` reads the column's scale + // and dispatches on the wire form (8/16-byte, Arrow Decimal). + // `get_f64` must NOT be used here — it reinterprets the raw + // unscaled-integer bytes as an IEEE-754 double (garbage/NaN). + row.get_numeric(i) + .map_or(CellValue::Null, CellValue::Numeric) } hyperdb_api::SqlType::ByteA => CellValue::Bytes(row.get_bytes(i).unwrap_or_default()), hyperdb_api::SqlType::Date => { @@ -179,6 +187,14 @@ impl RowData { let x = *v as i32; Some(x) } + CellValue::Numeric(n) => { + #[expect( + clippy::cast_possible_truncation, + reason = "caller-selected column coercion: Numeric cell narrowed to i32 per get_int32 contract" + )] + let x = n.to_f64() as i32; + Some(x) + } _ => None, } } @@ -200,6 +216,14 @@ impl RowData { let x = *v as i64; Some(x) } + CellValue::Numeric(n) => { + #[expect( + clippy::cast_possible_truncation, + reason = "caller-selected column coercion: Numeric cell narrowed to i64 per get_int64 contract" + )] + let x = n.to_f64() as i64; + Some(x) + } _ => None, } } @@ -250,13 +274,17 @@ impl RowData { /// Gets a 64-bit integer as a `BigInt` (no precision loss). /// /// Unlike `getInt64()` which returns a JS `number` (lossy above 2^53), - /// this returns a native `BigInt`. + /// this returns a native `BigInt`. For `NUMERIC(p, 0)` columns this + /// preserves the full 128-bit unscaled value; for `NUMERIC(p, scale>0)` + /// the cell is not an integer and `null` is returned (use `getString` + /// for exact decimal text, or `getFloat64` for a lossy numeric value). #[napi] pub fn get_big_int(&self, index: u32) -> Option { match self.values.get(index as usize)? { CellValue::I16(v) => Some(BigInt::from(i64::from(*v))), CellValue::I32(v) => Some(BigInt::from(i64::from(*v))), CellValue::I64(v) => Some(BigInt::from(*v)), + CellValue::Numeric(n) if n.scale() == 0 => Some(BigInt::from(n.unscaled_value())), _ => None, } } @@ -270,6 +298,7 @@ impl RowData { CellValue::I16(v) => Some(f64::from(*v)), CellValue::I32(v) => Some(f64::from(*v)), CellValue::I64(v) => Some(*v as f64), + CellValue::Numeric(n) => Some(n.to_f64()), _ => None, } } @@ -287,6 +316,8 @@ impl RowData { CellValue::I64(v) => Some(v.to_string()), CellValue::F32(v) => Some(v.to_string()), CellValue::F64(v) => Some(v.to_string()), + // Exact decimal text (preserves scale and sign); never lossy. + CellValue::Numeric(n) => Some(n.to_string()), CellValue::Bytes(v) => Some(hex::encode(v)), CellValue::DateMs(ms) => { // Convert back to ISO date string for getString. A Date is diff --git a/hyperdb-mcp/CHANGELOG.md b/hyperdb-mcp/CHANGELOG.md index 1198788..d5dea10 100644 --- a/hyperdb-mcp/CHANGELOG.md +++ b/hyperdb-mcp/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed + +- Query results now preserve the sign of negative `NUMERIC`/`DECIMAL` values + with magnitude less than 1. Previously a value like `CAST(-0.5 AS + numeric(10,4))` was serialized to JSON as `0.5` because `row_value_to_json` + stringifies NUMERIC via `Numeric::to_string()`, whose `Display` impl dropped + the sign for sub-unit magnitudes (fixed in `hyperdb-api-core`). This silently + flipped the sign of correlations, 0–1 indices, and regression residuals. + ### Added - **Single-instance `hyperd` daemon** — by default, all MCP clients now diff --git a/hyperdb-mcp/tests/engine_tests.rs b/hyperdb-mcp/tests/engine_tests.rs index bc1c383..2c6d606 100644 --- a/hyperdb-mcp/tests/engine_tests.rs +++ b/hyperdb-mcp/tests/engine_tests.rs @@ -336,6 +336,43 @@ fn engine_numeric_columns_and_aggregates_render_as_json_numbers() { assert!((avg - 2.0).abs() < 1e-9, "got {avg}"); } +/// Regression test: negative NUMERIC values with magnitude < 1 (the open +/// interval `(-1, 0)`) must keep their sign through `execute_query_to_json`. +/// +/// `row_value_to_json` serializes NUMERIC via `Numeric::to_string()`, whose +/// `Display` impl previously derived the sign from the integer part alone. +/// For `-0.5` the integer part is `0` (which prints without a sign), so the +/// value rendered as `0.5` — silently flipping the sign of correlations, +/// 0–1 indices, and regression residuals. Values with `|x| >= 1` and the +/// `DOUBLE PRECISION` path were unaffected, which this test also guards. +#[test] +fn engine_negative_sub_unit_numeric_keeps_sign() { + let te = TestEngine::new_ephemeral(); + + let rows = te + .engine + .execute_query_to_json( + "SELECT \ + CAST(-0.5 AS numeric(10,4)) AS a, \ + CAST(-0.999 AS numeric(10,4)) AS b, \ + CAST(-1.5 AS numeric(10,4)) AS c, \ + CAST(-0.5 AS double precision) AS d", + ) + .unwrap(); + assert_eq!(rows.len(), 1); + let row = &rows[0]; + + let val = |k: &str| { + row[k] + .as_f64() + .unwrap_or_else(|| panic!("{k} rendered as non-number: {:?}", row[k])) + }; + assert!((val("a") - (-0.5)).abs() < 1e-9, "got {}", val("a")); + assert!((val("b") - (-0.999)).abs() < 1e-9, "got {}", val("b")); + assert!((val("c") - (-1.5)).abs() < 1e-9, "got {}", val("c")); + assert!((val("d") - (-0.5)).abs() < 1e-9, "got {}", val("d")); +} + /// `resolve_log_dir` helper: persistent mode uses the workspace's parent, /// ephemeral mode uses the per-PID temp dir. #[test]