feat: implement SQL type inference for MySQL and MSSQL, add tests for type inference functions - #181
Conversation
… type inference functions
There was a problem hiding this comment.
Pull request overview
This PR introduces centralized SQL parameter type inference (primarily for MSSQL) and wires it into the shared Sql parameter-binding paths, while keeping MySQL behavior aligned with mysql2’s expectations. It also refactors existing decimal inference logic to reuse the new implementation and adds unit tests around the inference helpers.
Changes:
- Added
lib/sql-type-inference.jswith MSSQL-focused type inference (ASCII-aware strings, bucketed lengths, bucketed Decimal precision/scale, scalar vs batch behavior). - Updated
lib/sql.jsto use inference for default parameter binding, added optional schema-driven type resolution viaschemaTypes/schemaDrivenTypes, plus schema caching utilities. - Updated
lib/mysql.jsto override default inference so MySQL bindings usemysql.Types.*codes (ornull) instead of MSSQL type constructors; added tests for inference helpers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/sql-type-inference.test.js | Adds unit tests for new inference helpers (currently has assertion issues noted in comments). |
| lib/sql.js | Integrates inference into parameter binding; adds schema discovery/cache + schema-driven resolution options. |
| lib/sql-type-inference.js | New module implementing MSSQL type inference and bucketing logic. |
| lib/mysql.js | Overrides inference methods so MySQL continues using mysql2-appropriate type codes. |
| lib/business/business-base.mjs | Reuses shared getDecimalSqlType instead of maintaining an inline implementation. |
…QL type inference
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
lib/sql.js:448
- _parseTableName() splits on '.' and destructures the first two segments. For 3- or 4-part object names (e.g. "MyDb.dbo.Users" or "Server.MyDb.dbo.Users"), this mis-identifies schema/table (schema becomes "MyDb" and table becomes "dbo"), breaking schema cache keys and INFORMATION_SCHEMA lookups.
_parseTableName(tableName) {
const unquoted = tableName.replace(/[[\]]/g, '');
const [schema, name] = unquoted.includes('.') ? unquoted.split('.') : ['dbo', unquoted];
return { schema, name, key: `${schema}.${name}` };
…ests for large integer strings
Bigint values were not handled distinctly from other numeric types, causing incorrect type coercion in SQL parameters. Batch operations were also normalizing parameter values even when an explicit sqlType was already specified, preventing proper type resolution. This fix adds explicit bigint handling that preserves raw bigints only for mssql.BigInt type, safely coerces other numeric types to Number when within the safe integer range, and refactors batch inference to respect explicit sqlType and avoid unnecessary normalization.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
lib/sql-type-inference.js:201
- When a batch contains fractional numbers, this path does
nonNull.map(Number). If any element is a very largebigint,Number(bigint)can becomeInfinity, andgetRequiredDecimalDigits()will skip non-finite values—leading to an undersizedDecimal(p,s)that can truncate the bigint values when binding. Consider guarding for non-finite conversions and falling back to a safe max-tier Decimal in that case.
if (hasFractional) {
return getDecimalSqlType(nonNull.map(Number));
}
Numeric values like bigints can overflow to Infinity during type inference, but the code didn't account for this. The root cause was that getRequiredDecimalDigits() had no way to signal when non-finite values were encountered, leading to undercounted precision. Now we track the presence of non-finite values and size the decimal type to the maximum precision tier instead of calculating based on incomplete data. Also added discoverColumnTypes() guard to MySQL since the mssql-specific INFORMATION_SCHEMA queries are not supported.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
lib/sql.js:837
in()now throws when it encounters abigintvalue with a non-mssql.BigIntsqlType and the value is outside the JS safe-integer range. This path is reachable for MySQL:addParameters()normalizes large integer strings toBigIntfor batch inference,Mysql.inferDefaultBatchSqlType()infers an integer type code (notmssql.BigInt), and thenin()will throw for any 64-bit ID. For MySQL callers this is an unintended runtime regression (and the error message referencesutil.normalizeSqlType(sqlType), which for MySQL is just a numeric type code).
// Only mssql.BigInt should receive a raw bigint; other numeric types coerce to Number when safe.
if (sqlType !== mssql.BigInt) {
if (value < BigInt(Number.MIN_SAFE_INTEGER) || value > BigInt(Number.MAX_SAFE_INTEGER)) {
throw new Error(`Value ${value} for ${paramName} exceeds the safe range for ${util.normalizeSqlType(sqlType)}`);
}
No description provided.